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

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, May 28, 2025 at 08:22:08PM +0200, David Hildenbrand wrote:
> On 28.05.25 20:15, Jason Gunthorpe wrote:
> > On Wed, May 28, 2025 at 08:00:34PM +0200, David Hildenbrand wrote:
> > 
> > > Having refcounted anon page in a VM_PFNMAP doesn't suddenly turn the whole
> > > thing into a MIXEDMAP where other things with a "struct page" are suddenly
> > > refcounted as well.
> > 
> > The special COW rules for PFNMAP require a single linear mapping so
> > that vma->vm_pgoff can encode the "special" range.
> 
> Yes, I have a semi-ugly patch to adjust remove that special casing.
> 
> (less ugly than current handling: *still* lookup the struct page, and if it
> references an anonymous folio, we ... have an anonymous folio. Because
> nobody should ever,ever,ever PFNMAP an anonymous folio. Ever.)

Hmm, maybe so.

> In remap_pfn_range_internal() we sanity-check that right now. And fail if it
> is not in place.
> 
> Any COW VM_PFNMAP that doesn't go through remap_pfn_range() would be ...
> broken. I don't think we have them.

It is a really easy driver mistake to make though, you have to omit
the rejection of !VM_SHARED, then go on and do something else.

How many drivers use VM_PFNMAP without remap_pfn_range() and don't
check for VM_SHARED?

From a driver perspective it would be much easier to understand if the
driver could just tell the core code what functions it wants to use

if (vma_prepare(vma, USE_REMAP_PFN_RANGE |
                USE_IO_REMAP_PFN_RANGE |
                USE_VMF_INSERT_PAGE |
                USE_VMF_INSERT_IO_PFN |
                [.. etc ..]))

Then the above would set the VM flags properly and fail in cases that
are wrong. Like you can't do USE_VMF_INSERT_IO_PFN without VM_SHARED.

No more messes of drivers doing random and confusing mixtures of
VM_XX.

So much easier to audit if the drivers are doing the right stuff
because we just check what mm functions they call against the bit mask
they provide. A debug kernel could enforce the check.

Then we can say with clarity that VM_XX flags are used with various
insertion functions exclusively and know alot better what it is they
mean.

Jason