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

[Zi Yan]

On Thu Sep 10, 2026 at 8:02 AM EDT, 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, on SCAN_SUCCEED, 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.
>
> Assisted-by: LLM
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> ---
>  mm/khugepaged.c | 102 +++++++++++++++++++++++-------------------------
>  1 file changed, 49 insertions(+), 53 deletions(-)
>

LGTM. Thanks.

Reviewed-by: Zi Yan <ziy@xxxxxxxxxx>

One question:

What prevents us from doing:
while () {
    1. mmap_lock
    2. scan_pmd
    3. mmap_unlock, bail out if needed
    4. run_pmd
}

for both cases? It improves readability. What is the downside of
dropping the lock during multiple scans?

for madvise_collapse(), I see mmap_read_lock is held when it is called,
so it can be dropped at the entry and the code makes sure it is held at
the exit.


-- 
Best Regards,
Yan, Zi