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 11:08:14 -0400
Peter Xu <peterx@redhat.com> 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
> > com> +1287,6 @@ static int ZjQcmQRYFpfptBannerStart
> 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.
> 

Yes, it used to work in older kernels, before filesystems like 9p
switched to using the accessors. The problem is that there is not
a single patch that I can point as the culprit.

It took a long time to figure out but the timeline was:

1. Before any netfslib and 9pfs changes, I/O from remap_pfn_page ranges
works.
2. netfslib accessors are merged in mainline. Userspace still works.
3. 9pfs picks up netfslib accessors, things break.

I doubt any kernel CI would have a test-case for it, because it is
quite esoteric.

We do have a relatively simple buildroot patch that we can share, that
exhibits the problem, and that contains both a kernel, a kernel module
and a user space program that performs the I/O.

> Or do you mean it's a regression caused by userspace change?
>

No userspace changes.

> Thanks,
> 

Regards

-- Pantelis