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

[David Hildenbrand (Arm)]

On 8/12/26 15:02, Rik van Riel wrote:
> 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() ?

It does more than grabbing, hm, let me think.

gup_grab_and_store_folio_pages()

not sure.

> 
>> 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.

Yes, and my gut feeling is that the same helper should be also used in the other
places. Maybe there is no real bug there, but the inconsistency is concerning.

-- 
Cheers,

David