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

[David Hildenbrand]

> > If so, I'd prefer that rather than introducing feature-backend flags,
> > because I want to avoid introducing another different feature set to uffd.
>
> I was talking about uffd_features.  I thought it was being renamed to
> flags, not modes_supported.  It was pretty late when I responded.
>
> FWIU, David was saying we don't need both of modes and ioctl listed in
> the uffd_ops?

Right, I would have abstracted the features to clean it up and avoid 
using VM_ flags in this interface.

> I was thinking that we could just put the features directly as function
> pointers in the uffd_ops and check if they are NULL or not for
> 'support'.
>
> ie:
>
> struct vm_uffd_ops hugetlb_uffd_ops = {
>          .missing = hugetlb_handle_userfault,
>          .write_protect = mwriteprotect_range,
>          .minor = hugetlb_handle_userfault_minor,
>
>          .mfill_atomic = hugetlb_mfill_atomic_pte,
>          .mfill_atomic_continue = ...
>          .mfill_zeropage = ...
>          .mfill_poison = ...
>          .mfill_copy = NULL, /* For example */
> };
>
> Then mfill_atomic_copy() becomes:
> {
>          /*
>           * Maybe some setup, used for all mfill operations from
>           * mfill_atomic()
>           */
>
>           ...
>
>          dst_vma = uffd_mfill_lock()
>          uffd_ops = vma_get_uffd_ops(vma);
>          if (!uffd_ops)
>                  return false;
>
>          if (!uffd_ops->mfill_copy) /* unlikely? */
>                  return false;
>
>          return uffd_ops->mfill_copy(dst_vma,..);
> }
>
> This way is_vm_hugetlb_page() never really needs to be used because the
> function pointer already makes that distinction.
>
> Right now, we have checks for hugetlb through other functions that "pass
> off to appropriate routine", and we end up translating the
> ioctl_supports into the function call eventually, anyways.

Right, it would be great to get rid of that. I recall I asked for such a 
cleanup in RFC (or was it v1).

--
Cheers

David / dhildenb