Re: [PATCH 08/12] mm/collapse: separate scanning a PTE table from collapsing it

[Zi Yan]

On Fri Sep 4, 2026 at 11:10 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 and only reads.  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.
>     SCAN_SUCCEED from the scan means there is something to run; anything
>     else is why there is not.
>
> collapse_single_pmd() is now the two of them with the mmap_lock drop in
> between, so its callers see what they saw before.
>
> Scan results (beyond SCAN_SUCCEED) communicated via collapse_control
> structure: the orders, the referenced and swapped-out counts, and for a
> file the file itself and the offset in it.
>
> A file collapse works on the page cache and never sees a VMA.  The scan
> takes the file reference while it still has VMA and the run unpins it
> when it is done.
>
> Tracing changes with it.  mm_khugepaged_scan_pmd now fires before
> mm_collapse_huge_page instead of after it.  Its status field already reads
> SCAN_SUCCEED for an accepted table, so what the collapse then made of that
> table is mm_collapse_huge_page's to report, per order.
>
> The two calls to that tracepoint become one.  They differed in what the
> collapse between them changed; with the collapse no longer here, both
> carry the same arguments.  failed_pfn is set only where a PTE was refused,
> so it is -1 exactly when the result is SCAN_SUCCEED.
>
> Assisted-by: Claude-Code:claude-opus-5
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> ---
>  mm/collapse.h   |  14 ++++++
>  mm/khugepaged.c | 121 ++++++++++++++++++++++++++++++++++--------------
>  2 files changed, 100 insertions(+), 35 deletions(-)
>

<snip>

>  
> -	mmap_read_unlock(mm);
> -	*lock_dropped = true;
> +static enum scan_result collapse_run_pmd(struct mm_struct *mm,
> +		unsigned long addr, struct collapse_control *cc)
> +{
> +	struct file *file = cc->scan_file;
> +	bool triggered_wb = false;
> +	enum scan_result result;
> +	pgoff_t pgoff;
> +
> +	if (!file)
> +		return mthp_collapse(mm, addr, cc->scan_referenced,
> +				     cc->scan_unmapped, cc, cc->scan_orders);
> +
> +	cc->scan_file = NULL;
> +	pgoff = cc->scan_pgoff;
>  retry:
>  	result = collapse_scan_file(mm, addr, file, pgoff, cc);

In the commit message, collapse_run_pmd() is said to do the collapse
work, but collapse_scan_file() is scanning, right?

It seems that the code only separate anonymous scan and collapse.
Why cannot pagecache code be separated in a similar way?


-- 
Best Regards,
Yan, Zi