Re: [RFC PATCH v3 3/8] mm/gup: split follow_page_pte_commit() out of follow_page_pte()
Rik van Riel <[email protected]>
| Newsgroups | org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
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.