Re: [PATCH mm-unstable v4 5/5] mm/khugepaged: unify khugepaged and madv_collapse with collapse_single_pmd()

David Hildenbrand (Arm)
2026-03-31T20:50:55Z

On 3/31/26 22:06, Lorenzo Stoakes (Oracle) wrote:
> On Tue, Mar 31, 2026 at 10:00:58PM +0200, David Hildenbrand (Arm) wrote:
>> On 3/31/26 16:15, Lorenzo Stoakes (Oracle) wrote:
>>>
>>> That updates mmap_unlocked which will be handled in the loop or on exit:
>>>
>>> if (mmap_unlocked) {
>>> ...
>>> mmap_read_lock(mm);
>>> mmap_unlocked = false;
>>> *lock_dropped = true;
>>> ...
>>> }
>>>
>>> result = collapse_single_pmd(addr, vma, &mmap_unlocked, cc);
>>>
>>> ...
>>>
>>> out_maybelock:
>>> /* Caller expects us to hold mmap_lock on return */
>>> if (mmap_unlocked) {
>>> *lock_dropped = true;
>>> mmap_read_lock(mm);
>>> }
>>>
>>
>> Right.
>>
>> The original code used
>>
>> bool mmap_locked = true;
>>
>> If the fix get squashed, maybe we should revert to that handling to
>> minimize the churn?
>
> Well collapse_single_pmd() and all descendents would need to be updated to
> invert the boolean, not sure it's worth it.

The code is confusing me.

In the old code, collapse_single_pmd() was called with "mmap_locked ==
true" and set "mmap_locked=false".

Now we call it with "mmap_unlocked=false" and it sets "mmap_unlocked=true".

So far so good.

However, collapse_scan_pmd() consumed "mmap_locked", now it effectively
consumes "mmap_unlocked".

I think I have to squash your fix into Nicos patch to make sense of this. :)

--
Cheers,

David