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

[David Hildenbrand <david@redhat.com>]

On 28.05.25 19:59, Jason Gunthorpe wrote:
> On Wed, May 28, 2025 at 02:47:54PM -0300, Jason Gunthorpe wrote:
>> 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.
> 
> I guess I got this backwards in my memory, the vm_normal_page() thing
> is specifically to fix the COW nastyness on VM_PFNMAP.
> 
> So it is OK to call vm_normal_page() on both kinds of VMAs and it will
> return NULL if you are not allowed to touch the struct page.
> 
> This is still what GUP should be doing, vm_normal_page() and if no
> struct page comes back then failure. It should never try to get a
> struct page some other way.

Exactly. And we can even do that for VM_PFNMAP and just have COW 
VM_PFNMAP supported in GUP, likely resolving the issue here.

Should be fairly easy ...

> 
> Yes, GUP fast will be inconsistent with GUP slow on non-special
> arches, but that is unfixable without taking a performance hit.

IIRC, GUP-fast requires PTE-special.

So it should then all work as expected ...

-- 
Cheers,

David / dhildenb