Re: [PATCH v3 1/4] mm: Introduce vm_uffd_ops API

[David Hildenbrand <david@redhat.com>]

>>>> I briefly wondered whether we could use actual UFFD_FEATURE_* here, but they
>>>> are rather unsuited for this case here (e.g., different feature flags for
>>>> hugetlb support/shmem support etc).
>>>>
>>>> But reading "uffd_ioctls" below, can't we derive the suitable vma flags from
>>>> the supported ioctls?
>>>>
>>>> _UFFDIO_COPY | _UFDIO_ZEROPAGE -> VM_UFFD_MISSING
>>>> _UFFDIO_WRITEPROTECT -> VM_UFFD_WP
>>>> _UFFDIO_CONTINUE -> VM_UFFD_MINOR
>>>
>>> Yes we can deduce that, but it'll be unclear then when one stares at a
>>> bunch of ioctls and cannot easily digest the modes the memory type
>>> supports.  Here, the modes should be the most straightforward way to
>>> describe the capability of a memory type.
>>
>> I rather dislike the current split approach between vm-flags and ioctls.
>>
>> I briefly thought about abstracting it for internal purposes further and
>> just have some internal backend ("memory type") flags.
>>
>> UFFD_BACKEND_FEAT_MISSING -> _UFFDIO_COPY and VM_UFFD_MISSING
>> UFFD_BACKEND_FEAT_ZEROPAGE -> _UFDIO_ZEROPAGE
>> UFFD_BACKEND_FEAT_WP -> _UFFDIO_WRITEPROTECT and VM_UFFD_WP
>> UFFD_BACKEND_FEAT_MINOR -> _UFFDIO_CONTINUE and VM_UFFD_MINOR
>> UFFD_BACKEND_FEAT_POISON -> _UFFDIO_POISON
> 
> This layer of mapping can be helpful to some, but maybe confusing to
> others.. who is familiar with existing userfaultfd definitions.
> 

Just wondering, is this confusing to you, and if so, which part?

To me it makes perfect sense and cleans up this API and not have to sets 
of flags that are somehow interlinked.

>>>
>>> If hugetlbfs supported ZEROPAGE, then we can deduce the ioctls the other
>>> way round, and we can drop the uffd_ioctls.  However we need the ioctls now
>>> for hugetlbfs to make everything generic.
>>
>> POISON is not a VM_ flag, so that wouldn't work completely, right?
> 
> Logically speaking, POISON should be meaningful if MISSING|MINOR is
> supported.  However, in reality, POISON should always be supported across
> all types..

Do you know what the plans are with guest_memfd?

> 
>>
>> As a side note, hugetlbfs support for ZEROPAGE should be fairly easy:
>> similar to shmem support, simply allocate a zeroed hugetlb folio.
> 
> IMHO it'll be good if we do not introduce ZEROPAGE only because we want to
> remove some flags.. We could be introducing dead codes that nobody uses.
> 
> I think it'll be good if we put that as a separate discussion, and define
> the vm_uffd_ops based on the current situation.

Right. I'd vote for an abstraction in the lines of what I proposed 
above. Doesn't have to be the terminology I used above, but some simple 
single set of flag that we can map to the underlying details.

But again, hoping to hear other opinions on this topic.

-- 
Cheers

David / dhildenb