Re: [PATCH v2] mm/khugepaged: clear MMF_VM_HUGEPAGE on mm_slot_alloc() failure

[Dev Jain]

On 11/05/26 11:10 am, David Hildenbrand (Arm) wrote:
> On 5/11/26 06:00, Dev Jain wrote:
>>
>>
>> On 09/05/26 3:11 am, David Hildenbrand (Arm) wrote:
>>> On 5/6/26 12:51, Lance Yang wrote:
>>>>
>>>>
>>>> Right. A racing khugepaged_enter_vma() can see MMF_VM_HUGEPAGE is set
>>>> and return, then !slot clears it again. If there is no later
>>>> khugepaged_enter_vma(), the mm still wouldn't get registered :)
>>>
>>> So why not
>>>
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index 5f4e009593e0..78735f34250a 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -437,13 +437,16 @@ void __khugepaged_enter(struct mm_struct *mm)
>>>
>>>         /* __khugepaged_exit() must not run from under us */
>>>         VM_BUG_ON_MM(collapse_test_exit(mm), mm);
>>> -       if (unlikely(mm_flags_test_and_set(MMF_VM_HUGEPAGE, mm)))
>>> -               return;
>>>
>>>         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;
>>> +       }
>>> +
>>>         spin_lock(&khugepaged_mm_lock);
>>>         mm_slot_insert(mm_slots_hash, mm, slot);
>>>         /*
>>>
>>>
>>> Arguably, on the race described above, likely the thread seeing the
>>> MMF_VM_HUGEPAGE would likely similarly have failed the allocation.
>>>
>>> I'm fine with either, just wanted to raise the (cleaner looking?) alternative
>>> where we just properly back off?
>>
>> Yes this is also fine - I am overthinking but I wasn't going this way because ...
>> A process doing THP allocations will fail on the mm_flags_test_and_set everytime
>> after the first time.
> We should perform a mm_flags_test(MMF_VM_HUGEPAGE, vma->vm_mm) test before
> calling the function when the flag might not be set yet: in khugepaged_enter_vma()

Ah that slipped my mind, you are right.

> 
> khugepaged_fork() should only get called once per process.
> 
> Which makes sense, because mm_flags_test_and_set() is expensive.
>