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

[Kiryl Shutsemau <kirill@shutemov.name>]

On Fri, Sep 11, 2026 at 03:02:22PM -0400, Zi Yan wrote:
> On Thu Sep 10, 2026 at 8:02 AM EDT, Kiryl Shutsemau wrote:
> > 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.

I will add a comment to each function at its definition: what it needs,
what it does, and what state the lock is in on entry and exit.

The overview stays. Per-function comments tell you about one call at a
time; the overview is what ties the four together and says in which
order they are made and who holds the lock in between.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov