Re: [PATCH v2 11/12] mm/collapse: declare the collapse interface in collapse.h

["Zi Yan" <ziy@nvidia.com>]

On Thu Sep 10, 2026 at 8:02 AM EDT, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
>
> A collapse takes four 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;
>   - collapse_control_release() - done with the control.
>
> All four 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.
>
> 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@kernel.org>
> ---
>  mm/collapse.h   | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  mm/khugepaged.c | 18 +++++++++---------
>  2 files changed, 53 insertions(+), 9 deletions(-)
>
> diff --git a/mm/collapse.h b/mm/collapse.h
> index 346859a2184f..1ebbbf63fb25 100644
> --- a/mm/collapse.h
> +++ b/mm/collapse.h
> @@ -106,4 +106,48 @@ struct collapse_control {
>  	bool scan_retract_only;
>  };
>  
> +/* Which orders a VMA may collapse to, zero when it may not collapse at all */
> +unsigned long collapse_possible_orders(struct vm_area_struct *vma,
> +		vm_flags_t vm_flags, enum tva_type tva_flags);
> +
> +/*
> + * A caller states what it allows in cc->policy and then hands over one PTE
> + * table's worth of a VMA at a time:
> + *
> + *     collapse_control_init(cc)         once, before the first table
> + *     collapse_scan_pmd(vma, addr, ...) per table
> + *     collapse_run_pmd(mm, addr, cc)    when a scan found work
> + *     collapse_control_release(cc)      once, when done with the control

This is a good overview of the workflow.

> + *
> + * The caller holds mmap_lock for reading over the scan and passes an address
> + * within @vma, aligned to the PTE table the scan is to judge.
> + *
> + * The scan returns with that lock still held.  It only reads, and almost every
> + * table it is offered has nothing to collapse, so a caller walks a whole VMA
> + * under the one lock it took to get there.  SCAN_SUCCEED means there is
> + * something to collapse; anything else is why there is not.
> + *
> + * 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.
> + *
> + * 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.
> + */

It might be better to document each function individually about the
requirements and what each does instead of putting everything above.

> +void collapse_control_init(struct collapse_control *cc);
> +void collapse_control_release(struct collapse_control *cc);
> +enum scan_result collapse_scan_pmd(struct vm_area_struct *vma,
> +		unsigned long addr, struct collapse_control *cc,
> +		unsigned long orders);
> +enum scan_result collapse_run_pmd(struct mm_struct *mm, unsigned long addr,
> +		struct collapse_control *cc);
> +enum scan_result collapse_vma_revalidate(struct mm_struct *mm,
> +		unsigned long address, bool expect_anon,
> +		struct vm_area_struct **vmap, struct collapse_control *cc,
> +		unsigned int order);
> +
>  #endif	/* __MM_COLLAPSE_H */


Otherwise, LGTM.

-- 
Best Regards,
Yan, Zi