Re: [PATCH 1/2] mm/gup: Fix hugepd handling in hugetlb rework

[David Hildenbrand <david@redhat.com>]

On 28.04.24 21:01, Peter Xu wrote:
> Commit a12083d721d7 added hugepd handling for gup-slow, reusing gup-fast
> functions.  follow_hugepd() correctly took the vma pointer in, however
> didn't pass it over into the lower functions, which was overlooked.
> 
> The issue is gup_fast_hugepte() uses the vma pointer to make the correct
> decision on whether an unshare is needed for a FOLL_PIN|FOLL_LONGTERM.  Now
> without vma ponter it will constantly return "true" (needs an unshare) for
> a page cache, even though in the SHARED case it will be wrong to unshare.
> 
> The other problem is, even if an unshare is needed, it now returns 0 rather
> than -EMLINK, which will not trigger a follow up FAULT_FLAG_UNSHARE fault.
> That will need to be fixed too when the unshare is wanted.
> 
> gup_longterm test didn't expose this issue in the past because it didn't
> yet test R/O unshare in this case, another separate patch will enable that
> in future tests.
> 
> Fix it by passing vma correctly to the bottom, rename gup_fast_hugepte()
> back to gup_hugepte() as it is shared between the fast/slow paths, and also
> allow -EMLINK to be returned properly by gup_hugepte() even though gup-fast
> will take it the same as zero.
> 
> Reported-by: David Hildenbrand <david@redhat.com>
> Fixes: a12083d721d7 ("mm/gup: handle hugepd for follow_page()")
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---

LGTM

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb