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

Peter Xu
2025-07-02T19:30:22Z

On Sun, Jun 29, 2025 at 11:50:11AM +0300, Mike Rapoport wrote:
> Hi Peter,

Hi, Mike,

>
> On Fri, Jun 27, 2025 at 11:46:52AM -0400, Peter Xu wrote:
> > Introduce a generic userfaultfd API for vm_operations_struct, so that one
> > vma, especially when as a module, can support userfaults without modifying
> > the core files. More importantly, when the module can be compiled out of
> > the kernel.
> >
> > So, instead of having core mm referencing modules that may not ever exist,
> > we need to have modules opt-in on core mm hooks instead.
> >
> > After this API applied, if a module wants to support userfaultfd, the
> > module should only need to touch its own file and properly define
> > vm_uffd_ops, instead of changing anything in core mm.
>
> I liked the changelog update you proposed in v1 thread. I took liberty to

It's definitely hard to satisfy all reviewers on one version of commit
message..

> slightly update it and here's what I've got:
>
> Currently, most of the userfaultfd features are implemented directly in the
> core mm. It will invoke VMA specific functions whenever necessary. So far
> it is fine because it almost only interacts with shmem and hugetlbfs.
>
> Introduce a generic userfaultfd API extension for vm_operations_struct,
> so that any code that implements vm_operations_struct (including kernel
> modules that can be compiled separately from the kernel core) can support
> userfaults without modifying the core files.
>
> With this API applied, if a module wants to support userfaultfd, the
> module should only need to properly define vm_uffd_ops and hook it to
> vm_operations_struct, instead of changing anything in core mm.

Thanks, I very much appreciate explicit suggestions on the wordings.
Personally I like it and the rest suggestions, I'll use it when repost, but
I'll also wait for others if anyone has other things to say.

>
> > Note that such API will not work for anonymous. Core mm will process
> > anonymous memory separately for userfault operations like before.
>
> Maybe:
>
> This API will not work for anonymous memory. Handling of userfault
> operations for anonymous memory remains unchanged in core mm.
>
> > This patch only introduces the API alone so that we can start to move
> > existing users over but without breaking them.
>
> Please use imperative mood, e.g.
>
> Only introduce the new API so that ...
>
> > Currently the uffd_copy() API is almost designed to be the simplistic with
> > minimum mm changes to move over to the API.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > include/linux/mm.h | 9 ++++++
> > include/linux/userfaultfd_k.h | 52 +++++++++++++++++++++++++++++++++++
> > 2 files changed, 61 insertions(+)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index ef40f68c1183..6a5447bd43fd 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -576,6 +576,8 @@ struct vm_fault {
> > */
> > };
> >
> > +struct vm_uffd_ops;
> > +
> > /*
> > * These are the virtual MM functions - opening of an area, closing and
> > * unmapping it (needed to keep files on disk up-to-date etc), pointer
> > @@ -653,6 +655,13 @@ struct vm_operations_struct {
> > */
> > struct page *(*find_special_page)(struct vm_area_struct *vma,
> > unsigned long addr);
> > +#ifdef CONFIG_USERFAULTFD
> > + /*
> > + * Userfaultfd related ops. Modules need to define this to support
> > + * userfaultfd.
> > + */
> > + const struct vm_uffd_ops *userfaultfd_ops;
> > +#endif
> > };
> >
> > #ifdef CONFIG_NUMA_BALANCING
> > diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> > index df85330bcfa6..c9a093c4502b 100644
> > --- a/include/linux/userfaultfd_k.h
> > +++ b/include/linux/userfaultfd_k.h
> > @@ -92,6 +92,58 @@ enum mfill_atomic_mode {
> > NR_MFILL_ATOMIC_MODES,
> > };
> >
> > +/* VMA userfaultfd operations */
> > +struct vm_uffd_ops {
> > + /**
> > + * @uffd_features: features supported in bitmask.
> > + *
> > + * When the ops is defined, the driver must set non-zero features
> > + * to be a subset (or all) of: VM_UFFD_MISSING|WP|MINOR.
> > + */
> > + unsigned long uffd_features;
> > + /**
> > + * @uffd_ioctls: ioctls supported in bitmask.
> > + *
> > + * Userfaultfd ioctls supported by the module. Below will always
> > + * be supported by default whenever a module provides vm_uffd_ops:
> > + *
> > + * _UFFDIO_API, _UFFDIO_REGISTER, _UFFDIO_UNREGISTER, _UFFDIO_WAKE
> > + *
> > + * The module needs to provide all the rest optionally supported
> > + * ioctls. For example, when VM_UFFD_MISSING was supported,
> > + * _UFFDIO_COPY must be supported as ioctl, while _UFFDIO_ZEROPAGE
> > + * is optional.
> > + */
> > + unsigned long uffd_ioctls;
> > + /**
> > + * uffd_get_folio: Handler to resolve UFFDIO_CONTINUE request.
> > + *
> > + * @inode: the inode for folio lookup
> > + * @pgoff: the pgoff of the folio
> > + * @folio: returned folio pointer
> > + *
> > + * Return: zero if succeeded, negative for errors.
> > + */
> > + int (*uffd_get_folio)(struct inode *inode, pgoff_t pgoff,
> > + struct folio **folio);
> > + /**
> > + * uffd_copy: Handler to resolve UFFDIO_COPY|ZEROPAGE request.
> > + *
> > + * @dst_pmd: target pmd to resolve page fault
> > + * @dst_vma: target vma
> > + * @dst_addr: target virtual address
> > + * @src_addr: source address to copy from
> > + * @flags: userfaultfd request flags
> > + * @foliop: previously allocated folio
> > + *
> > + * Return: zero if succeeded, negative for errors.
> > + */
> > + int (*uffd_copy)(pmd_t *dst_pmd, struct vm_area_struct *dst_vma,
> > + unsigned long dst_addr, unsigned long src_addr,
> > + uffd_flags_t flags, struct folio **foliop);
> > +};
> > +typedef struct vm_uffd_ops vm_uffd_ops;
>
> Either use vm_uffd_ops_t for the typedef or drop the typedef entirely. My
> preference is for the second option.

Andrew helped me to fix some hidden spaces which I appreciated, then I
found checkpatch warns on this one too besides the spaces fixed in mm-new.
I do not know why checkpatch doesn't like typedefs even if typedefs are
massively used in Linux..

I think I'll simply stick with not using typedefs.

Thanks,

>
> > +
> > #define MFILL_ATOMIC_MODE_BITS (const_ilog2(NR_MFILL_ATOMIC_MODES - 1) + 1)
> > #define MFILL_ATOMIC_BIT(nr) BIT(MFILL_ATOMIC_MODE_BITS + (nr))
> > #define MFILL_ATOMIC_FLAG(nr) ((__force uffd_flags_t) MFILL_ATOMIC_BIT(nr))
> > --
> > 2.49.0
> >
>
> --
> Sincerely yours,
> Mike.
>

--
Peter Xu