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

[Kiryl Shutsemau]

On Fri, Sep 04, 2026 at 10:44:58PM -0400, Zi Yan wrote:
> On Fri Sep 4, 2026 at 11:10 AM EDT, 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.
> >
> > 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 judged 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: Claude-Code:claude-opus-5
> > Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> > ---
> >  mm/collapse.h   |  40 ++++++++++++++++-
> >  mm/khugepaged.c | 114 ++++++++++++++++++++++++++----------------------
> >  2 files changed, 102 insertions(+), 52 deletions(-)
> >
> > diff --git a/mm/collapse.h b/mm/collapse.h
> > index 1c40229b9554..05282eed9a35 100644
> > --- a/mm/collapse.h
> > +++ b/mm/collapse.h
> > @@ -48,8 +48,46 @@ enum scan_result {
> >  	SCAN_PAGE_DIRTY_OR_WRITEBACK,
> >  };
> >  
> > +/* What a collapse is allowed to do, decided by the caller that asks for it */
> > +struct collapse_policy {
> > +	/* Limits, stated per PMD; HPAGE_PMD_NR means "no limit" */
> > +	unsigned int max_ptes_none;
> > +	unsigned int max_ptes_swap;
> > +	unsigned int max_ptes_shared;
> > +
> > +	/*
> > +	 * Hold a sub-PMD window to a stricter rule than a PMD: no swapped-out
> > +	 * and no shared PTEs at all, and max_ptes_none as
> > +	 * collapse_max_ptes_none() scales it.
> > +	 */
> > +	bool strict_sub_pmd;
> > +
> > +	/*
> > +	 * Collapse only where it looks worth doing: require some sign the
> > +	 * range is in use, and leave clean lazyfree folios for reclaim rather
> > +	 * than collapsing them into a folio that is not lazyfree.
> > +	 */
> > +	bool skip_lazyfree;
> > +	bool require_referenced;
> > +
> > +	/*
> > +	 * Finish the job rather than leaving it half done for a fault to pick
> > +	 * up: map the PMD over a file collapse before returning, and write
> > +	 * dirty pages back and retry once instead of refusing them.  Both cost
> > +	 * latency the caller has to be willing to pay.
> > +	 */
> > +	bool install_pmd;
> > +	bool writeback_dirty;
> > +
> > +	/* How hard to try for a destination folio */
> > +	gfp_t gfp;
> > +
> > +	/* Which VMAs are eligible, as thp_vma_allowable_orders() spells it */
> > +	enum tva_type tva_type;
> > +};
> > +
> >  struct collapse_control {
> > -	bool is_khugepaged;
> > +	struct collapse_policy policy;
> 
> Can it be made const since it seems to be read-only?

Not as a member: khugepaged fills it once per pass from the sysfs knobs,
so there is no constant object to point at. And madvise has to
initialize it somehow after cc is allocated.

It could become a const pointer to a struct the caller owns, but that
adds a lifetime to track for a struct that is written in one place and
read everywhere else.  I would rather keep it embedded.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov