Re: [PATCH] mm: fix COW mapping handing in generic_access_phys

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, May 28, 2025 at 07:32:59PM +0200, David Hildenbrand wrote:
> On 28.05.25 18:29, Jason Gunthorpe wrote:
> > On Wed, May 28, 2025 at 12:06:07PM -0400, Peter Xu wrote:
> > > #define VM_PFNMAP	0x00000400	/* Page-ranges managed without "struct page", just pure PFN */
> > > 
> > > I'm not confident to blame any driver yet to have those special cases for
> > > VM_PFNMAP, because it only says "managed without struct page", it didn't
> > > say "it must not contain struct page"..  Hence it hints the core mm "please
> > > do not manage these mappings with struct page at all".  Still sounds fair
> > > contract, even if not ideal.
> > 
> > I think it is pretty clear, if a VMA has VM_PFNMAP then nothing must
> > ever try to obtain a struct page from any PTEs in it, for any reason,
> > even if things in it might have a struct page. In practice it means
> > nothing can call vm_normal_page() on a VM_PFNMAP.
> 
> No, not until we remove any COW mappings of VM_PFNMAP.

I stand by the statement, the COW mapping thing is broken. Just
because it is wrong doesn't mean it is allowed to call
vm_normal_page() on VM_PFNMAP.

It makes no sense at all to have VM_PFNMAP where someone has COW'd the
pages and effectively turned it into a MIXEDMAP.

It should just be a MIXEDMAP at that point!!

IMHO we should have the core code complain with a dmesg for VM_PFNMAP
without VM_SHARED and get driver authors to fix it. Either add the
missed VM_SHARED or changed to MIXEDMAP. Make it WARN_ON in a few
years.

If they can't follow the rules required to use MIXEDMAP then they
certainly do not have working COW!!

> > In either case GUP doesn't really work on the VMA. PFNMAP is totally
> > blocked, and for MIXEDMAP userspace has no way to discover which
> > subset of the VMA is GUPable. I think that GUP is supported on
> > MIXEDMAP at all is a bit of a weirdo thing.
> 
> IIRC (after recent discussions with Lorenzo) there are use cases for that.

Really? Somehow userspace can know which sub slice of the VMA is
GUP'able? Gross :P

> And there is no way to block GUP-fast either way without
> pte_special(). And pte_special() ... is not for refcounted pages.

Yes, on arches with the special bit GUP-fast is properly blocked. I
view those as the reference behavior. If non-special arches can't do
this check then it still not something that is uAPI because GUP-fast
could fallback unpredictably and go to GUP slow which will do the
check. Userspace can't rely on it to work!

Jason