Re: [PATCH v4 12/13] mm/collapse: declare the collapse interface in collapse.h

[Zi Yan]

On Mon Sep 28, 2026 at 6:06 AM EDT, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@xxxxxxxxxx>
>
> A collapse takes three calls:
>
>   - collapse_control_init() - set up the control a caller carries;
>   - collapse_scan_pmd() - scan one PTE table, under mmap_lock;
>   - collapse_run_pmd() - collapse what the scan found, no mmap_lock.
>
> All three are static in khugepaged.c, as are collapse_possible_orders(),
> which says what a VMA allows, and the revalidate a caller needs once a
> collapse has given the mmap_lock up.  No other file can ask for a collapse
> without them.
>
> Declare them in collapse.h, with a comment stating the order they are
> called in and who holds the lock over each step.  Each function gets a
> kerneldoc comment where it is defined: what it takes, what it does, and
> the lock state on entry and exit.
>
> hugepage_vma_revalidate() becomes collapse_vma_revalidate(): it is part of
> what a collapse offers now, not a helper of the daemon.
>
> Preparation for implementing MADV_COLLAPSE in madvise.c.
>
> No functional change.
>
> Assisted-by: LLM
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> ---
>  mm/collapse.h   | 45 ++++++++++++++++++++++++++
>  mm/khugepaged.c | 86 ++++++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 115 insertions(+), 16 deletions(-)
>
<snip>

> + * lock it took to get there.  SCAN_SUCCEED means there is
> + * something to collapse.  SCAN_PTE_MAPPED_HUGEPAGE means the page cache
> + * already holds the PMD folio and only the PTE table is left to retract.
> + * Both are work for the run, which is handed what the scan returned; anything
> + * else is why there is nothing to do.

These are implmentation details and already documented in the
collapse_scan_pmd()'s kdoc.

> + *
> + * The run is called without the lock and returns without it, taking what it
> + * needs in between: what it does -- allocate, isolate, copy, flush -- is slow
> + * enough that a writer would wait behind it.  The caller gives the lock up
> + * first, and with it @vma and anything derived under it, so a caller carrying
> + * on has to look up again with collapse_vma_revalidate().  The run revalidates
> + * for itself rather than trusting what the scan saw.


The run does ... would be sufficient, since the above is the same
contente in collapse_run_pmd()'s kdoc.

> + *
> + * A scan that found something has to be run: the file side takes a reference on
> + * the file while it still has the VMA to take it from, and the run is what
> + * gives it back.

This is a repeated comment in collapse_control. We only need one.


Otherwise, LGTM.

Acked-by: Zi Yan <ziy@xxxxxxxxxx>

-- 
Best Regards,
Yan, Zi