Re: [PATCH v4 09/13] mm/collapse: separate scanning a PTE table from collapsing it

[Zi Yan]

On Mon Sep 28, 2026 at 6:06 AM EDT, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@xxxxxxxxxx>
>
> A collapse is two jobs.  One reads a PTE table under mmap_lock and decides
> whether the range is worth collapsing.  The other allocates, isolates,
> copies and flushes, and wants the lock given up first.
>
> collapse_single_pmd() did both, so the boundary between them was somewhere
> in the middle of a function.
>
> Give each half its own function:
>
>   - collapse_scan_pmd() scans one table.  The anonymous scan that used to
>     carry that name keeps its body as collapse_scan_anon_pmd(), and
>     collapse_scan_pmd() is now the entry that picks the anonymous or the
>     file side.
>
>   - collapse_run_pmd() does the collapse the scan asked for, and is
>     handed what the scan returned.  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; anything else is why there is
>     nothing to do.
>
> collapse_single_pmd() is now the two of them with the mmap_lock drop in
> between, so its callers see what they saw before.  collapse_control_init()
> sets a control up before its first scan.
>
> What the scan found and the run needs travels in collapse_control.  For
> an anonymous table that is the orders and the referenced and swapped-out
> counts, which mthp_collapse() and collapse_huge_page() now read from
> there instead of taking as arguments.  For a file it is the file itself
> and the offset in it: a file collapse works on the page cache and never
> sees a VMA, so the scan takes the reference while it still has one and
> the run gives it back.
>
> The file scan moves under mmap_lock with the anonymous one, where before
> the lock was given up first.  The lock is now held over the page cache
> walk, an RCU walk over one table's worth of slots with no PTL, and taken
> fewer times.  collapse_scan_mm_slot() ends its walk whenever the lock was
> dropped, so a refused file table used to cost khugepaged an unlock, a
> trip back through khugepaged_do_scan(), a relock and a VMA lookup.  Now
> only a table that goes on to be collapsed does.
>
> Assisted-by: LLM
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> ---
>  mm/collapse.h   |  14 +++++
>  mm/khugepaged.c | 141 ++++++++++++++++++++++++++++++++----------------
>  2 files changed, 108 insertions(+), 47 deletions(-)
>
> diff --git a/mm/collapse.h b/mm/collapse.h
> index dcd117071955..ca7b367c89cb 100644
> --- a/mm/collapse.h
> +++ b/mm/collapse.h
> @@ -98,6 +98,20 @@ struct collapse_control {
>  
>  	/* Each bit marks a PTE the scan accepted as a collapse source */
>  	DECLARE_BITMAP(eligible_ptes, MAX_PTRS_PER_PTE);
> +
> +	/*
> +	 * What a scan found and the run after it needs.  Live only between the
> +	 * two, and read by nobody else.
> +	 *
> +	 * The file side takes a reference while it still has the VMA, since a
> +	 * file collapse works on the page cache and never sees one; the run is
> +	 * what gives it back.
> +	 */
> +	unsigned long scan_orders;
> +	int scan_referenced;
> +	int scan_unmapped;

They are used by anon only.

> +	struct file *scan_file;
> +	pgoff_t scan_pgoff;

They are used by file only.

I think the comments need to address them separately.

Otherwise, LGTM.

Acked-by: Zi Yan <ziy@xxxxxxxxxx>



-- 
Best Regards,
Yan, Zi