Re: [PATCH v3 09/12] mm/collapse: open-code collapse_single_pmd() in its two callers

[David Hildenbrand (Arm)]

On 9/16/26 11:31, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@xxxxxxxxxx>
> 
> collapse_scan_pmd() and collapse_run_pmd() each have a clear locking
> contract.  The scan is called with mmap_lock held for reading and returns
> with it still held.  The collapse is called without it.
> 
> collapse_single_pmd() kept that boundary inside itself.  It dropped the
> lock on some paths and not others, and reported which by way of a bool its
> callers had to carry along and then act on.
> 
> Open-code it in the two callers.  Each scans under the lock it already
> holds and, when the scan found work, gives the lock up before running the
> collapse.
> khugepaged's lock_dropped and madvise_collapse()'s mmap_unlocked both go:
> the code dropping the lock is now the code that wanted to know.
> 
> khugepaged's walk carries on to the next table while the scan keeps
> refusing, and ends once a collapse has taken the lock from under it.
> madvise_collapse() re-finds its VMA after a collapse, which it did before,
> and now uses a NULL vma to say that it has to.  It still reports the drop
> to its own caller, from the line that does it.
> 
> The lock is given up and taken again at the same points as before.  No
> functional change.
> 

I'm not sure I see the benefit. The code in the previous collapse_single_pmd()
callers certainly gets more messy?

Is there some other patches in this series that depend on it or what's the
motivation?

-- 
Cheers,

David