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

On 8/11/26 04:51, Rik van Riel wrote:
> follow_page_pte() does two things once it has resolved a present PTE
> to a page: run the per-PTE safety checks (write-fault, unshare), then
> commit to that page: grab a ref, fault it in if pinning, mark it
> dirty/accessed, and hand it back to the caller.
> 
> Split the second part into its own follow_page_pte_commit(), unchanged
> except for taking its inputs as parameters instead of local variables,
> so the checks and the commit can be applied at different granularities.
> 
> No functional changes intended.
> 
> Suggested-by: David Hildenbrand <[email protected]>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Rik van Riel <[email protected]>
> ---
>  mm/gup.c | 78 +++++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 49 insertions(+), 29 deletions(-)
> 
> diff --git a/mm/gup.c b/mm/gup.c
> index e4e6d0993424..b755ceaac0f5 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -822,6 +822,52 @@ static inline bool can_follow_write_pte(pte_t pte, struct page *page,
>  	return !userfaultfd_pte_wp(vma, pte);
>  }
>  
> +/*
> + * 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.

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.

Let me think about this some more, I might have to play with the code myself to
understand how this should all be cleaned up.

-- 
Cheers,

David