Messages in this thread |  | | Date | Tue, 5 May 2026 09:47:24 +0530 | | Subject | Re: [PATCH] mm/khugepaged: clear MMF_VM_HUGEPAGE on mm_slot_alloc() failure | | From | Dev Jain <> |
| |
On 01/05/26 6:54 pm, Lance Yang wrote: > > On Fri, May 01, 2026 at 04:10:58PM +0530, Dev Jain wrote: >> >> >> On 01/05/26 1:27 pm, Ye Liu wrote: >>> From: Ye Liu <liuye@kylinos.cn> >>> >>> __khugepaged_enter() sets MMF_VM_HUGEPAGE before allocating the >>> corresponding mm_slot. If mm_slot_alloc() fails, the function >>> returns with the flag set but without inserting the mm into the >>> khugepaged tracking structures. >>> >>> This leaves the mm in an inconsistent state: it is marked as >>> registered (MMF_VM_HUGEPAGE set), but will never be scanned by >>> khugepaged. Future attempts to register the mm are skipped since >>> khugepaged_enter_vma() checks the flag and returns early. >>> >>> Fix this by clearing MMF_VM_HUGEPAGE when mm_slot_alloc() fails, >>> restoring the ability to retry registration later. >>> >>> Signed-off-by: Ye Liu <liuye@kylinos.cn> >>> --- >>> mm/khugepaged.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c >>> index 7d48d4fbd5f3..60ab7c1b61dd 100644 >>> --- a/mm/khugepaged.c >>> +++ b/mm/khugepaged.c >>> @@ -559,8 +559,10 @@ void __khugepaged_enter(struct mm_struct *mm) >>> return; >>> >>> slot = mm_slot_alloc(mm_slot_cache); >>> - if (!slot) >>> + if (!slot) { >>> + mm_flags_clear(MMF_VM_HUGEPAGE, mm); >>> return; >>> + } >> >> You could just move the test_and_set() after this no? > > Yep, that sounds better :) > > Just one small thing: if we move the test_and_set(), after > mm_slot_alloc(), we need to free the slot when test_and_set() says that > the flag was already set. Otherwise the racing caller that loses would > leak the slot :) > > Something like: > > slot = mm_slot_alloc(mm_slot_cache); > if (!slot) > return; > > if (unlikely(mm_flags_test_and_set(MMF_VM_HUGEPAGE, mm))) { > mm_slot_free(mm_slot_cache, slot); > return; > }
My bad, I missed the fact that test-and-set may also fail. So I think Ye's patch is also fine. > >> So if slot allocation fails then MMF_VM_HUGEPAGE is never set. >> >> Fixes tag is probably required but not sure about Ccing stable. > > Yes, Fixes sounds good. khugepaged collapse is best-effort though, > anyway :) > > So I'm also not sure about Ccing stable. > >>> >>> spin_lock(&khugepaged_mm_lock); >>> mm_slot_insert(mm_slots_hash, mm, slot); > > Cheers, Lance
|  |