Re: [PATCH v4 05/13] mm/collapse: state what a collapse may do in the policy

[David Hildenbrand (Arm)]

On 9/28/26 12:06, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@xxxxxxxxxx>
> 
> Tests scattered through the collapse path decide what a collapse is
> allowed to do by asking whether khugepaged started it.  Between them they
> settle:
> 
>  - which VMAs are eligible, and how hard to try for a folio;
>  - how many empty, swapped-out or shared PTEs a window may contain, and
>    whether a sub-PMD window is held to a stricter rule than a PMD;
>  - whether a range has to look used, and whether a MADV_FREE'd page is
>    left alone;
>  - whether the PMD is mapped as part of the request, and whether dirty
>    pages are worth writing back and retrying.
> 
> None of those is a fact about khugepaged.  Each is something the caller
> decided before asking, and the collapse code should not have to look up
> who called to find out.
> 
> Add struct collapse_policy for the caller to fill: khugepaged from its
> own settings, MADV_COLLAPSE from the fact that a user asked explicitly.
> Every test becomes a read of a field, and cc->is_khugepaged goes, having
> no reader left.
> 
> The PTE limits come as two sets, one for a PMD-sized window and one for
> anything smaller, so that the helpers pick a set for the order and read
> it.  khugepaged takes no swapped-out or shared PTE into a sub-PMD window,
> and empty PTEs only when the knob says all or nothing; the warning for a
> knob value in between moves to where khugepaged fills its policy.  The
> fields only one side reads say which: anon_ or file_.  David Hildenbrand
> asked for both.
> 
> khugepaged fills the policy once per scan pass, MADV_COLLAPSE once per
> call.  That is the one change in behaviour.  The max_ptes_* limits and the
> defrag setting behind the allocation mask are sampled once per pass rather
> than on every table.  A table scanned early in a pass and one scanned late
> are then treated alike.
> 
> collapse_file() also drops a NULL check on the collapse_control.  It has
> one call site, reached only from collapse_single_pmd(), which dereferences
> cc unconditionally, so the check was already dead.
> 
> Assisted-by: LLM
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> ---
>  mm/collapse.h   |  41 +++++++++++++-
>  mm/khugepaged.c | 147 +++++++++++++++++++++++++-----------------------
>  2 files changed, 118 insertions(+), 70 deletions(-)
> 
> diff --git a/mm/collapse.h b/mm/collapse.h
> index b115034d9018..dcd117071955 100644
> --- a/mm/collapse.h
> +++ b/mm/collapse.h
> @@ -45,8 +45,47 @@ enum scan_result {
>  	SCAN_PAGE_DIRTY_OR_WRITEBACK,
>  };
>  
> +/* How many PTEs of a window may be missing, swapped out or shared */
> +struct collapse_limits {
> +	/* Counted over a PMD-sized window; HPAGE_PMD_NR means "no limit" */
> +	unsigned int max_ptes_none;
> +	unsigned int max_ptes_swap;
> +	unsigned int max_ptes_shared;
> +};
> +
> +/* What a collapse is allowed to do, decided by the caller that asks for it */
> +struct collapse_policy {
> +	/* Limits for a PMD-sized window */
> +	struct collapse_limits pmd;
> +
> +	/*
> +	 * Limits for a smaller window.  Its max_ptes_none is either 0 or
> +	 * COLLAPSE_MAX_PTES_LIMIT, the latter meaning all but one PTE of the
> +	 * window whatever its order; any other value counts as 0.
> +	 */
> +	struct collapse_limits sub_pmd;
> +

Yeah, this is cleaner.

Skimmed over the remaining stuff and nothing jumped at me

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

-- 
Cheers,

David