Re: + fix-zero-copy-i-o-on-__get_user_pages-allocated-pages.patch added to mm-hotfixes-unstable branch

[Pantelis Antoniou <p.antoniou@partner.samsung.com>]

On Thu, 8 May 2025 17:10:10 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 08. 05. 25 17: 08, Peter Xu wrote: > On Thu, May 08, 2025 at 05:
> 36: 12PM +0300, Pantelis Antoniou wrote: >> On Thu, 8 May 2025 10:
> 16: 31 -0400 >> Peter Xu <peterx@ redhat. com> wrote: >> >> Hi Peter,

> On 08.05.25 17:08, Peter Xu wrote:
> > On Thu, May 08, 2025 at 05:36:12PM +0300, Pantelis Antoniou wrote:
> >> On Thu, 8 May 2025 10:16:31 -0400
> >> Peter Xu <peterx@redhat.com> wrote:
> >>
> >> Hi Peter,
> > 
> > Hi, Pantelis,
> > 
> > [...]
> > 
> >>>> @@ -1271,8 +1287,6 @@ static int check_vma_flags(struct vm_are
> >>>>   	int foreign = (gup_flags & FOLL_REMOTE);
> >>>>   	bool vma_anon = vma_is_anonymous(vma);
> >>>>   
> >>>> -	if (vm_flags & (VM_IO | VM_PFNMAP))
> >>>> -		return -EFAULT;
> >>>
> >>> Is there's any justification that this won't break some existing
> >>> GUP users that may rely on properly failing at pfnmaps?
> >>>
> >>> IIUC netfs isn't the first one that wants to GUP on top of
> >>> pfnmaps, KVM does it for years and so far it was processed in a
> >>> standalone path:
> >>>
> >>> hva_to_pfn:
> >>> 	else if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
> >>> 		r = hva_to_pfn_remapped(vma, kfp, &pfn);
> >>>
> >>> That started with supporting real pfnmaps (with no page struct),
> >>> but pfnmap with page structs can also happen afaict, and kvm
> >>> processes that too by checking page==NULL ultimately, e.g. in
> >>> kvm_release_faultin_page().
> >>>
> >>
> >> I see. The problem is that we're not the owners of the code in
> >> netfslib, and it is considerably more intrusive to fix things
> >> there.
> >>
> >> This is a hotfix for a userspace regression. I sort of agree that
> >> having different handling for these areas in netfslib would be
> >> ideal.
> > 
> > Do you mean this used to work in older kernels?  Some more info on
> > the regression would be more than welcomed if so..  If it fixes a
> > kernel regression, we may want a Fixes for whatever patch at last.
> 
> To be precise: Whoever decided to use remap_pfn_range() essentially 
> decided that GUP cannot possibly work.
> 
> So is the regression introduced by a conversion to remap_pfn_range()
> in some code, or because suddenly someone relies on GUP for these
> things?
> 

I don't think there was a deliberate decision here, but there was no
conversion to remap_pfn_range(), the code (in DRM) was always there.

The regression occurred when netfslib started using GUP for I/O and
when filesystems switched to it we hit this case.

Regards

-- Pantelis