Re: [RFC PATCH v3 3/8] mm/gup: split follow_page_pte_commit() out of follow_page_pte()

[Rik van Riel <riel@surriel.com>]

On Wed, 2026-08-12 at 13:50 +0200, David Hildenbrand (Arm) wrote:
> On 8/11/26 04:51, Rik van Riel wrote:
> > 
> > +/*
> > + * The caller has already run every per-PTE safety check (present,
> > + * write-fault, gup_must_unshare()) on the PTE, so this only does
> > the
> > + * per-folio work: the refcount grab, the FOLL_PIN accessibility
> > fault-in,
> > + * dirty/accessed marking, and the array fill with the cache
> > flush.
> > + */
> 
> If a function needs 4 lines of internal doc to understand what it
> does, then
> maybe it's not the right function name or abstraction. :)
> 
> I don't particularly enjoy the "commit" terminology, it looses the
> detail of the
> most important thing this function is supposed to to: grab a folio
> reference.
> 

Should we rename it to gup_grab_folio() ?

> We're passing in the pte only to do a single pte_dirty(). If the
> helper is
> actually what we want, likely we just just pass in the pte_dirty()
> directly and
> remove all pte leftovers from the helper.
> 
> I'd assume that we'd want a function where we can similarly punch in
> a PMD/PUD
> and have it do the same thing as well, instead of special-casing
> ptes.

Looking at the differences between the pte and pmd
code, there is another big thing that stands out.

The pte code will call mark_folio_dirty() when
needed, but the pmd and pud code do not.

For hugetlbfs we can get away with that, but for
huge folios coming from a filesystem, don't we
want to call mark_folio_dirty() somewhere?

I'm happy to do whatever cleanups we can do to
this code while we're here.

-- 
All Rights Reversed.