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

[Peter Xu <peterx@redhat.com>]

On Wed, May 28, 2025 at 01:29:15PM -0300, 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.
> 
> It would be nice to update the comment to make it clearer.

Yes that would help. Maybe the hard part is making sure how it is
documented will be how it is used..

> 
> If the VMA owner wanted to permit access to the struct page then it
> should have used VM_MIXEDMAP.
> 
> The fundamental difference between PFNMAP and MIXEDMAP is that
> vm_normal_page() is allowed on MIXEDMAP. That comes with some extra
> rules and restrictions to support arches without the special pte bit.

If in the ideal world where VM_PFNMAP has a stricter semantics, it sounds
fair to disable vm_normal_page() on top of VM_PFNMAP, yes.

> 
> VM_IO | VM_PFNMAP further means that all the pfns in the VMA require
> the use of io accessors (writel/readl) to access them.

Hmm.. I'm not 100% sure on this one.  E.g., vDSO is VM_IO now but it's
definitely accessible that got mapped into userspace.  Same case when
e.g. mmap() VM_IO regions then accessing using the virtual addresses.

What you said sounds more like what __iomem declares rather than VM_IO.
But I confess I at least don't know why VM_IO existed, considering there're
also VM_*MAP and VM_DONTDUMP.

> 
> No idea what VM_IO | VM_MIXEDMAP is supposed to mean. Only the special
> ptes need io accessors?
> 
> 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.

Does it imply that in the ideal case one should use follow_pfnmap_start()
for MIXEDMAP?

I don't have a strong feeling yet on how GUP should treat MIXEDMAP, either
(1) fail MIXEDMAP like you said, falling back to follow_pfnmap_start(), or
(2) allow MIXEDMAP only on page-backed mappings, then fallback to
follow_pfnmap_start() on non-page-backed mappings only.

> 
> The main value of MIXEDMAP having access to the struct page is to
> allow the page table itself to manage the life-cycle of memory that
> was allocated specifically and only for the VMA. It will do fork and
> unmap properly and free the memory at the right time.

Agree.

> 
> If a driver is managing the memory lifecylce on its own and using zap
> to clear the PTEs (because it must to manage the non-struct page PTE
> life-cycle), then there isn't much point in using MIXEDMAP, IMHO. I
> wonder if some of the GPU drivers are confused like this.
> 
> Jason
> 

-- 
Peter Xu