[PATCH v3 0/4] mm/userfaultfd: modulize memory types

[PATCH v3 0/4] mm/userfaultfd: modulize memory types

[Peter Xu]

[based on latest akpm/mm-new of Sep 26th, commit e612c80ae0aeb]

v3 changelog:
- Fixed checkpatch issues on spaces or typedef
- Dropped uffd_copy() API
- Refined commit messages here and there to reflect the removal of uffd_copy()

v1: https://lore.kernel.org/r/20250620190342.1780170-1-peterx@redhat.com
v2: https://lore.kernel.org/r/20250627154655.2085903-1-peterx@redhat.com

This series is an alternative proposal of what Nikita proposed here on the
initial three patches:

  https://lore.kernel.org/r/20250404154352.23078-1-kalyazin@amazon.com

This is not yet relevant to any guest-memfd support, but paving way for it.
Here, the major goal is to make kernel modules be able to opt-in with any
form of userfaultfd supports, like guest-memfd.  This alternative option
should hopefully be cleaner, and avoid leaking userfault details into
vm_ops.fault().

It also means this series does not depend on anything.  It's a pure
refactoring of userfaultfd internals to provide a generic API, so that
other types of files, especially RAM based, can support userfaultfd without
touching mm/ at all.

To achieve it, this series introduced a file operation called vm_uffd_ops.
The ops needs to be provided when a file type supports any of userfaultfd.

With that, I moved both hugetlbfs and shmem over, whenever possible.  So
far due to concerns on exposing an uffd_copy() API, the MISSING faults are
still separately processed and can only be done within mm/.  Hugetlbfs kept
its special paths untouched.

An example of shmem uffd_ops:

static const vm_uffd_ops shmem_uffd_ops = {
	.uffd_features	= 	__VM_UFFD_FLAGS,
	.uffd_ioctls	= 	BIT(_UFFDIO_COPY) |
				BIT(_UFFDIO_ZEROPAGE) |
				BIT(_UFFDIO_WRITEPROTECT) |
				BIT(_UFFDIO_CONTINUE) |
				BIT(_UFFDIO_POISON),
	.uffd_get_folio	=	shmem_uffd_get_folio,
};

No functional change expected at all after the whole series applied.  There
might be some slightly stricter check on uffd ops here and there in the
last patch, but that really shouldn't stand out anywhere to anyone.

For testing: besides the cross-compilation tests, I did also try with
uffd-stress in a VM to measure any perf difference before/after the change;
The static call becomes a pointer now.  I really cannot measure anything
different, which is more or less expected.

Comments welcomed, thanks.

Peter Xu (4):
  mm: Introduce vm_uffd_ops API
  mm/shmem: Support vm_uffd_ops API
  mm/hugetlb: Support vm_uffd_ops API
  mm: Apply vm_uffd_ops API to core mm

 include/linux/mm.h            |   9 +++
 include/linux/userfaultfd_k.h |  83 ++++++++++++++++-----------
 mm/hugetlb.c                  |  19 +++++++
 mm/shmem.c                    |  25 +++++++++
 mm/userfaultfd.c              | 102 ++++++++++++++++++++++++++--------
 5 files changed, 181 insertions(+), 57 deletions(-)

-- 
2.50.1

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

[Peter Xu]

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.

This API will not work for anonymous memory. Handling of userfault
operations for anonymous memory remains unchanged in core mm.

Due to a security concern while reviewing older versions of this series
[1], uffd_copy() will be temprorarily removed.  IOW, so far MISSING-capable
memory types can only be hard-coded and implemented in mm/.  It would also
affect UFFDIO_COPY and UFFDIO_ZEROPAGE.  Other functions should still be
able to be provided from vm_uffd_ops.

Introduces the API only so that existing userfaultfd users can be moved
over without breaking them.

[1] https://lore.kernel.org/all/20250627154655.2085903-1-peterx@redhat.com/

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/linux/mm.h            |  9 +++++++++
 include/linux/userfaultfd_k.h | 37 +++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6b6c6980f46c2..8afb93387e2c6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -620,6 +620,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
@@ -705,6 +707,13 @@ struct vm_operations_struct {
 	struct page *(*find_normal_page)(struct vm_area_struct *vma,
 					 unsigned long addr);
 #endif /* CONFIG_FIND_NORMAL_PAGE */
+#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 c0e716aec26aa..b1949d8611238 100644
--- a/include/linux/userfaultfd_k.h
+++ b/include/linux/userfaultfd_k.h
@@ -92,6 +92,43 @@ 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.
+	 *
+	 * NOTE: VM_UFFD_MISSING is still only supported under mm/ so far.
+	 */
+	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_MINOR is supported,
+	 * _UFFDIO_CONTINUE must be supported as an ioctl.
+	 */
+	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);
+};
+
 #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.50.1

[PATCH v3 2/4] mm/shmem: Support vm_uffd_ops API

[Peter Xu]

Add support for the new vm_uffd_ops API for shmem.  Note that this only
introduces the support, the API is not yet used by core mm.

It only needs a separate uffd_get_folio() definition but that's oneliner.

Due to the limitation of the current vm_uffd_ops on MISSING mode support,
the shmem UFFDIO_COPY/ZEROPAGE process are still hard-coded in mm/.

Cc: Hugh Dickins <hughd@google.com>
Acked-by: Mike Rapoport <rppt@kernel.org>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 mm/shmem.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/mm/shmem.c b/mm/shmem.c
index 4855eee227310..e7b44efbfddf2 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3148,6 +3148,13 @@ static inline struct inode *shmem_get_inode(struct mnt_idmap *idmap,
 #endif /* CONFIG_TMPFS_QUOTA */
 
 #ifdef CONFIG_USERFAULTFD
+
+static int shmem_uffd_get_folio(struct inode *inode, pgoff_t pgoff,
+				struct folio **folio)
+{
+	return shmem_get_folio(inode, pgoff, 0, folio, SGP_NOALLOC);
+}
+
 int shmem_mfill_atomic_pte(pmd_t *dst_pmd,
 			   struct vm_area_struct *dst_vma,
 			   unsigned long dst_addr,
@@ -5191,6 +5198,18 @@ static int shmem_error_remove_folio(struct address_space *mapping,
 	return 0;
 }
 
+#ifdef CONFIG_USERFAULTFD
+static const struct vm_uffd_ops shmem_uffd_ops = {
+	.uffd_features	=	__VM_UFFD_FLAGS,
+	.uffd_ioctls	=	BIT(_UFFDIO_COPY) |
+				BIT(_UFFDIO_ZEROPAGE) |
+				BIT(_UFFDIO_WRITEPROTECT) |
+				BIT(_UFFDIO_CONTINUE) |
+				BIT(_UFFDIO_POISON),
+	.uffd_get_folio	=	shmem_uffd_get_folio,
+};
+#endif
+
 static const struct address_space_operations shmem_aops = {
 	.dirty_folio	= noop_dirty_folio,
 #ifdef CONFIG_TMPFS
@@ -5293,6 +5312,9 @@ static const struct vm_operations_struct shmem_vm_ops = {
 	.set_policy     = shmem_set_policy,
 	.get_policy     = shmem_get_policy,
 #endif
+#ifdef CONFIG_USERFAULTFD
+	.userfaultfd_ops = &shmem_uffd_ops,
+#endif
 };
 
 static const struct vm_operations_struct shmem_anon_vm_ops = {
@@ -5302,6 +5324,9 @@ static const struct vm_operations_struct shmem_anon_vm_ops = {
 	.set_policy     = shmem_set_policy,
 	.get_policy     = shmem_get_policy,
 #endif
+#ifdef CONFIG_USERFAULTFD
+	.userfaultfd_ops = &shmem_uffd_ops,
+#endif
 };
 
 int shmem_init_fs_context(struct fs_context *fc)
-- 
2.50.1

[PATCH v3 3/4] mm/hugetlb: Support vm_uffd_ops API

[Peter Xu]

Add support for the new vm_uffd_ops API for hugetlb.  Note that this only
introduces the support, the API is not yet used by core mm.

Due to legacy reasons, it's still not trivial to move hugetlb completely to
the API (like shmem).  But it will still use uffd_features and uffd_ioctls
properly on the API because that's pretty general.

Cc: Muchun Song <muchun.song@linux.dev>
Cc: Oscar Salvador <osalvador@suse.de>
Acked-by: Mike Rapoport <rppt@kernel.org>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 mm/hugetlb.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index cb5c4e79e0b8f..b6eb92828ee15 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5492,6 +5492,22 @@ static vm_fault_t hugetlb_vm_op_fault(struct vm_fault *vmf)
 	return 0;
 }
 
+#ifdef CONFIG_USERFAULTFD
+static const struct vm_uffd_ops hugetlb_uffd_ops = {
+	.uffd_features	=	__VM_UFFD_FLAGS,
+	/* _UFFDIO_ZEROPAGE not supported */
+	.uffd_ioctls	=	BIT(_UFFDIO_COPY) |
+				BIT(_UFFDIO_WRITEPROTECT) |
+				BIT(_UFFDIO_CONTINUE) |
+				BIT(_UFFDIO_POISON),
+	/*
+	 * Hugetlbfs still has its own hard-coded handler in userfaultfd,
+	 * due to limitations similar to vm_operations_struct.fault().
+	 * TODO: generalize it to use the API functions.
+	 */
+};
+#endif
+
 /*
  * When a new function is introduced to vm_operations_struct and added
  * to hugetlb_vm_ops, please consider adding the function to shm_vm_ops.
@@ -5505,6 +5521,9 @@ const struct vm_operations_struct hugetlb_vm_ops = {
 	.close = hugetlb_vm_op_close,
 	.may_split = hugetlb_vm_op_split,
 	.pagesize = hugetlb_vm_op_pagesize,
+#ifdef CONFIG_USERFAULTFD
+	.userfaultfd_ops = &hugetlb_uffd_ops,
+#endif
 };
 
 static pte_t make_huge_pte(struct vm_area_struct *vma, struct folio *folio,
-- 
2.50.1

[PATCH v3 4/4] mm: Apply vm_uffd_ops API to core mm

[Peter Xu]

Move userfaultfd core to use new vm_uffd_ops API. After this change file
systems that implement vm_operations_struct can start using new API for
userfaultfd operations.

When at it, moving vma_can_userfault() into mm/userfaultfd.c instead,
because it's getting too big.  It's only used in slow paths so it shouldn't
be an issue.  Move the pte marker check before wp_async, which might be
more intuitive because wp_async depends on pte markers.  That shouldn't
cause any functional change though because only one check would take effect
depending on whether pte marker was selected in config.

This will also remove quite some hard-coded checks for either shmem or
hugetlbfs.  Now all the old checks should still work but with vm_uffd_ops.

Note that anonymous memory will still need to be processed separately
because it doesn't have vm_ops at all.

Reviewed-by: James Houghton <jthoughton@google.com>
Acked-by: Mike Rapoport <rppt@kernel.org>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/linux/userfaultfd_k.h |  46 +++++----------
 mm/userfaultfd.c              | 102 ++++++++++++++++++++++++++--------
 2 files changed, 91 insertions(+), 57 deletions(-)

diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
index b1949d8611238..e3704e27376ad 100644
--- a/include/linux/userfaultfd_k.h
+++ b/include/linux/userfaultfd_k.h
@@ -134,9 +134,14 @@ struct vm_uffd_ops {
 #define MFILL_ATOMIC_FLAG(nr) ((__force uffd_flags_t) MFILL_ATOMIC_BIT(nr))
 #define MFILL_ATOMIC_MODE_MASK ((__force uffd_flags_t) (MFILL_ATOMIC_BIT(0) - 1))
 
+static inline enum mfill_atomic_mode uffd_flags_get_mode(uffd_flags_t flags)
+{
+	return (__force enum mfill_atomic_mode)(flags & MFILL_ATOMIC_MODE_MASK);
+}
+
 static inline bool uffd_flags_mode_is(uffd_flags_t flags, enum mfill_atomic_mode expected)
 {
-	return (flags & MFILL_ATOMIC_MODE_MASK) == ((__force uffd_flags_t) expected);
+	return uffd_flags_get_mode(flags) == expected;
 }
 
 static inline uffd_flags_t uffd_flags_set_mode(uffd_flags_t flags, enum mfill_atomic_mode mode)
@@ -245,41 +250,16 @@ static inline bool userfaultfd_armed(struct vm_area_struct *vma)
 	return vma->vm_flags & __VM_UFFD_FLAGS;
 }
 
-static inline bool vma_can_userfault(struct vm_area_struct *vma,
-				     vm_flags_t vm_flags,
-				     bool wp_async)
+static inline const struct vm_uffd_ops *vma_get_uffd_ops(struct vm_area_struct *vma)
 {
-	vm_flags &= __VM_UFFD_FLAGS;
-
-	if (vma->vm_flags & VM_DROPPABLE)
-		return false;
-
-	if ((vm_flags & VM_UFFD_MINOR) &&
-	    (!is_vm_hugetlb_page(vma) && !vma_is_shmem(vma)))
-		return false;
-
-	/*
-	 * If wp async enabled, and WP is the only mode enabled, allow any
-	 * memory type.
-	 */
-	if (wp_async && (vm_flags == VM_UFFD_WP))
-		return true;
-
-#ifndef CONFIG_PTE_MARKER_UFFD_WP
-	/*
-	 * If user requested uffd-wp but not enabled pte markers for
-	 * uffd-wp, then shmem & hugetlbfs are not supported but only
-	 * anonymous.
-	 */
-	if ((vm_flags & VM_UFFD_WP) && !vma_is_anonymous(vma))
-		return false;
-#endif
-
-	/* By default, allow any of anon|shmem|hugetlb */
-	return vma_is_anonymous(vma) || is_vm_hugetlb_page(vma) ||
-	    vma_is_shmem(vma);
+	if (vma->vm_ops && vma->vm_ops->userfaultfd_ops)
+		return vma->vm_ops->userfaultfd_ops;
+	return NULL;
 }
 
+bool vma_can_userfault(struct vm_area_struct *vma,
+		       unsigned long vm_flags, bool wp_async);
+
 static inline bool vma_has_uffd_without_event_remap(struct vm_area_struct *vma)
 {
 	struct userfaultfd_ctx *uffd_ctx = vma->vm_userfaultfd_ctx.ctx;
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index af61b95c89e4e..0a863ac123d84 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -20,6 +20,43 @@
 #include "internal.h"
 #include "swap.h"
 
+bool vma_can_userfault(struct vm_area_struct *vma, vm_flags_t vm_flags,
+		       bool wp_async)
+{
+	unsigned long supported;
+
+	if (vma->vm_flags & VM_DROPPABLE)
+		return false;
+
+	vm_flags &= __VM_UFFD_FLAGS;
+
+#ifndef CONFIG_PTE_MARKER_UFFD_WP
+	/*
+	 * If user requested uffd-wp but not enabled pte markers for
+	 * uffd-wp, then any file system (like shmem or hugetlbfs) are not
+	 * supported but only anonymous.
+	 */
+	if ((vm_flags & VM_UFFD_WP) && !vma_is_anonymous(vma))
+		return false;
+#endif
+	/*
+	 * If wp async enabled, and WP is the only mode enabled, allow any
+	 * memory type.
+	 */
+	if (wp_async && (vm_flags == VM_UFFD_WP))
+		return true;
+
+	if (vma_is_anonymous(vma))
+		/* Anonymous has no page cache, MINOR not supported */
+		supported = VM_UFFD_MISSING | VM_UFFD_WP;
+	else if (vma_get_uffd_ops(vma))
+		supported = vma_get_uffd_ops(vma)->uffd_features;
+	else
+		return false;
+
+	return !(vm_flags & (~supported));
+}
+
 static __always_inline
 bool validate_dst_vma(struct vm_area_struct *dst_vma, unsigned long dst_end)
 {
@@ -382,13 +419,17 @@ static int mfill_atomic_pte_continue(pmd_t *dst_pmd,
 				     unsigned long dst_addr,
 				     uffd_flags_t flags)
 {
+	const struct vm_uffd_ops *uffd_ops = vma_get_uffd_ops(dst_vma);
 	struct inode *inode = file_inode(dst_vma->vm_file);
 	pgoff_t pgoff = linear_page_index(dst_vma, dst_addr);
 	struct folio *folio;
 	struct page *page;
 	int ret;
 
-	ret = shmem_get_folio(inode, pgoff, 0, &folio, SGP_NOALLOC);
+	if (WARN_ON_ONCE(!uffd_ops || !uffd_ops->uffd_get_folio))
+		return -EINVAL;
+
+	ret = uffd_ops->uffd_get_folio(inode, pgoff, &folio);
 	/* Our caller expects us to return -EFAULT if we failed to find folio */
 	if (ret == -ENOENT)
 		ret = -EFAULT;
@@ -504,18 +545,6 @@ static __always_inline ssize_t mfill_atomic_hugetlb(
 	u32 hash;
 	struct address_space *mapping;
 
-	/*
-	 * There is no default zero huge page for all huge page sizes as
-	 * supported by hugetlb.  A PMD_SIZE huge pages may exist as used
-	 * by THP.  Since we can not reliably insert a zero page, this
-	 * feature is not supported.
-	 */
-	if (uffd_flags_mode_is(flags, MFILL_ATOMIC_ZEROPAGE)) {
-		up_read(&ctx->map_changing_lock);
-		uffd_mfill_unlock(dst_vma);
-		return -EINVAL;
-	}
-
 	src_addr = src_start;
 	dst_addr = dst_start;
 	copied = 0;
@@ -694,6 +723,41 @@ static __always_inline ssize_t mfill_atomic_pte(pmd_t *dst_pmd,
 	return err;
 }
 
+static inline bool
+vma_uffd_ops_supported(struct vm_area_struct *vma, uffd_flags_t flags)
+{
+	enum mfill_atomic_mode mode = uffd_flags_get_mode(flags);
+	const struct vm_uffd_ops *uffd_ops;
+	unsigned long uffd_ioctls;
+
+	if ((flags & MFILL_ATOMIC_WP) && !(vma->vm_flags & VM_UFFD_WP))
+		return false;
+
+	/* Anonymous supports everything except CONTINUE */
+	if (vma_is_anonymous(vma))
+		return mode != MFILL_ATOMIC_CONTINUE;
+
+	uffd_ops = vma_get_uffd_ops(vma);
+	if (!uffd_ops)
+		return false;
+
+	uffd_ioctls = uffd_ops->uffd_ioctls;
+	switch (mode) {
+	case MFILL_ATOMIC_COPY:
+		return uffd_ioctls & BIT(_UFFDIO_COPY);
+	case MFILL_ATOMIC_ZEROPAGE:
+		return uffd_ioctls & BIT(_UFFDIO_ZEROPAGE);
+	case MFILL_ATOMIC_CONTINUE:
+		if (!(vma->vm_flags & VM_SHARED))
+			return false;
+		return uffd_ioctls & BIT(_UFFDIO_CONTINUE);
+	case MFILL_ATOMIC_POISON:
+		return uffd_ioctls & BIT(_UFFDIO_POISON);
+	default:
+		return false;
+	}
+}
+
 static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx,
 					    unsigned long dst_start,
 					    unsigned long src_start,
@@ -752,11 +816,7 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx,
 	    dst_vma->vm_flags & VM_SHARED))
 		goto out_unlock;
 
-	/*
-	 * validate 'mode' now that we know the dst_vma: don't allow
-	 * a wrprotect copy if the userfaultfd didn't register as WP.
-	 */
-	if ((flags & MFILL_ATOMIC_WP) && !(dst_vma->vm_flags & VM_UFFD_WP))
+	if (!vma_uffd_ops_supported(dst_vma, flags))
 		goto out_unlock;
 
 	/*
@@ -766,12 +826,6 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx,
 		return  mfill_atomic_hugetlb(ctx, dst_vma, dst_start,
 					     src_start, len, flags);
 
-	if (!vma_is_anonymous(dst_vma) && !vma_is_shmem(dst_vma))
-		goto out_unlock;
-	if (!vma_is_shmem(dst_vma) &&
-	    uffd_flags_mode_is(flags, MFILL_ATOMIC_CONTINUE))
-		goto out_unlock;
-
 	while (src_addr < src_start + len) {
 		pmd_t dst_pmdval;
 
-- 
2.50.1

Re: [PATCH v3 4/4] mm: Apply vm_uffd_ops API to core mm

[David Hildenbrand]

On 26.09.25 23:16, Peter Xu wrote:
> Move userfaultfd core to use new vm_uffd_ops API. After this change file
> systems that implement vm_operations_struct can start using new API for
> userfaultfd operations.
> 
> When at it, moving vma_can_userfault() into mm/userfaultfd.c instead,
> because it's getting too big.  It's only used in slow paths so it shouldn't
> be an issue.  Move the pte marker check before wp_async, which might be
> more intuitive because wp_async depends on pte markers.  That shouldn't
> cause any functional change though because only one check would take effect
> depending on whether pte marker was selected in config.
> 
> This will also remove quite some hard-coded checks for either shmem or
> hugetlbfs.  Now all the old checks should still work but with vm_uffd_ops.
> 
> Note that anonymous memory will still need to be processed separately
> because it doesn't have vm_ops at all.
> 
> Reviewed-by: James Houghton <jthoughton@google.com>
> Acked-by: Mike Rapoport <rppt@kernel.org>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---

[...]

> +++ b/mm/userfaultfd.c
> @@ -20,6 +20,43 @@
>   #include "internal.h"
>   #include "swap.h"
>   
> +bool vma_can_userfault(struct vm_area_struct *vma, vm_flags_t vm_flags,
> +		       bool wp_async)
> +{
> +	unsigned long supported;
> +
> +	if (vma->vm_flags & VM_DROPPABLE)
> +		return false;
> +
> +	vm_flags &= __VM_UFFD_FLAGS;
> +
> +#ifndef CONFIG_PTE_MARKER_UFFD_WP

While at it, you can turn that into an
!IS_ENABLED(CONFIG_PTE_MARKER_UFFD_WP) to avoid the ifdef.

> +	/*
> +	 * If user requested uffd-wp but not enabled pte markers for
> +	 * uffd-wp, then any file system (like shmem or hugetlbfs) are not
> +	 * supported but only anonymous.
> +	 */
> +	if ((vm_flags & VM_UFFD_WP) && !vma_is_anonymous(vma))
> +		return false;
> +#endif
> +	/*
> +	 * If wp async enabled, and WP is the only mode enabled, allow any
> +	 * memory type.
> +	 */
> +	if (wp_async && (vm_flags == VM_UFFD_WP))
> +		return true;


> +
> +	if (vma_is_anonymous(vma))
> +		/* Anonymous has no page cache, MINOR not supported */
> +		supported = VM_UFFD_MISSING | VM_UFFD_WP;
> +	else if (vma_get_uffd_ops(vma))
> +		supported = vma_get_uffd_ops(vma)->uffd_features;
> +	else
> +		return false;

To avoid the hidde return here, I think you can just do

	supported = 0;


Or even cleaner, just do

unsigned long supported = 0
...
if (vma_is_anonymous(vma))
	supported = ...
else if (vma_get_uffd_ops(vma))
	supported = ...
return ...

> +
> +	return !(vm_flags & (~supported));

I think this can just be:

	return !(vm_flags & ~supported);


-- 
Cheers

David / dhildenb

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

[David Hildenbrand]

On 26.09.25 23:16, Peter Xu wrote:
> 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.
> 
> This API will not work for anonymous memory. Handling of userfault
> operations for anonymous memory remains unchanged in core mm.
> 
> Due to a security concern while reviewing older versions of this series
> [1], uffd_copy() will be temprorarily removed.  IOW, so far MISSING-capable
> memory types can only be hard-coded and implemented in mm/.  It would also
> affect UFFDIO_COPY and UFFDIO_ZEROPAGE.  Other functions should still be
> able to be provided from vm_uffd_ops.
> 
> Introduces the API only so that existing userfaultfd users can be moved
> over without breaking them.
> 
> [1] https://lore.kernel.org/all/20250627154655.2085903-1-peterx@redhat.com/
> 

Looks much better with the uffdio_copy stuff removed for now.

> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   include/linux/mm.h            |  9 +++++++++
>   include/linux/userfaultfd_k.h | 37 +++++++++++++++++++++++++++++++++++
>   2 files changed, 46 insertions(+)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 6b6c6980f46c2..8afb93387e2c6 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -620,6 +620,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
> @@ -705,6 +707,13 @@ struct vm_operations_struct {
>   	struct page *(*find_normal_page)(struct vm_area_struct *vma,
>   					 unsigned long addr);
>   #endif /* CONFIG_FIND_NORMAL_PAGE */
> +#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 c0e716aec26aa..b1949d8611238 100644
> --- a/include/linux/userfaultfd_k.h
> +++ b/include/linux/userfaultfd_k.h
> @@ -92,6 +92,43 @@ 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.
> +	 *
> +	 * NOTE: VM_UFFD_MISSING is still only supported under mm/ so far.
> +	 */
> +	unsigned long uffd_features;

This variable name is a bit confusing , because it's all about vma 
flags, not uffd features. Just reading the variable, I would rather 
connect it to things like UFFD_FEATURE_WP_UNPOPULATED.

As currently used for VM flags, maybe you should call this

	unsigned long uffd_vm_flags;

or sth like that.

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

> +	/**
> +	 * @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_MINOR is supported,
> +	 * _UFFDIO_CONTINUE must be supported as an ioctl.
> +	 */
> +	unsigned long uffd_ioctls;
> +	/**
> +	 * uffd_get_folio: Handler to resolve UFFDIO_CONTINUE request.

Just wondering if we could incorporate the "continue" / "minor" aspect 
into the callback name.

uffd_minor_get_folio / uffd_continue_get_folio

Or do you see use of that callback in the context of other uffd features?

-- 
Cheers

David / dhildenb

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

[Mike Rapoport]

On Tue, Sep 30, 2025 at 11:36:53AM +0200, David Hildenbrand wrote:
> On 26.09.25 23:16, Peter Xu wrote:
> > +	/**
> > +	 * uffd_get_folio: Handler to resolve UFFDIO_CONTINUE request.
> 
> Just wondering if we could incorporate the "continue" / "minor" aspect into
> the callback name.
> 
> uffd_minor_get_folio / uffd_continue_get_folio
> 
> Or do you see use of that callback in the context of other uffd features?

If someone picks the gauntlet of refactoring the loop in mcopy_atomic()
we'd need a similar callback for uffd copy. And as I see it it would be
different enough to warrant emphasizing minor/continue in the name here.

I also think we can drop uffd_ prefix for the callback, as it's called as
uffd_ops->get_folio() or whatever it's be called.
 
> -- 
> Cheers
> 
> David / dhildenb
> 

-- 
Sincerely yours,
Mike.

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

[David Hildenbrand]

On 30.09.25 12:07, Mike Rapoport wrote:
> On Tue, Sep 30, 2025 at 11:36:53AM +0200, David Hildenbrand wrote:
>> On 26.09.25 23:16, Peter Xu wrote:
>>> +	/**
>>> +	 * uffd_get_folio: Handler to resolve UFFDIO_CONTINUE request.
>>
>> Just wondering if we could incorporate the "continue" / "minor" aspect into
>> the callback name.
>>
>> uffd_minor_get_folio / uffd_continue_get_folio
>>
>> Or do you see use of that callback in the context of other uffd features?
> 
> If someone picks the gauntlet of refactoring the loop in mcopy_atomic()
> we'd need a similar callback for uffd copy. And as I see it it would be
> different enough to warrant emphasizing minor/continue in the name here.
> 
> I also think we can drop uffd_ prefix for the callback, as it's called as
> uffd_ops->get_folio() or whatever it's be called.

Agreed. I got annoyed yesterday when typing vma->vm_mm often enough 
(vma->mm! ).

-- 
Cheers

David / dhildenb

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

[Peter Xu]

On Tue, Sep 30, 2025 at 12:18:37PM +0200, David Hildenbrand wrote:
> On 30.09.25 12:07, Mike Rapoport wrote:
> > On Tue, Sep 30, 2025 at 11:36:53AM +0200, David Hildenbrand wrote:
> > > On 26.09.25 23:16, Peter Xu wrote:
> > > > +	/**
> > > > +	 * uffd_get_folio: Handler to resolve UFFDIO_CONTINUE request.
> > > 
> > > Just wondering if we could incorporate the "continue" / "minor" aspect into
> > > the callback name.
> > > 
> > > uffd_minor_get_folio / uffd_continue_get_folio
> > > 
> > > Or do you see use of that callback in the context of other uffd features?
> > 
> > If someone picks the gauntlet of refactoring the loop in mcopy_atomic()
> > we'd need a similar callback for uffd copy. And as I see it it would be
> > different enough to warrant emphasizing minor/continue in the name here.

Sure, I can go with uffd_minor_get_folio when I repost.

> > 
> > I also think we can drop uffd_ prefix for the callback, as it's called as
> > uffd_ops->get_folio() or whatever it's be called.
> 
> Agreed. I got annoyed yesterday when typing vma->vm_mm often enough
> (vma->mm! ).

That's also why I kept uffd_ because that's the tradition mm/ uses in many
important data structures like vma and mm.  It helps most tagging systems
that most Linux developers use to avoid global name collisions.

So I tend to keep the prefix for now, until we want to switch away from
Hungarian-like notations completely. But let me know if anyone has strong
feelings.

Thanks,

-- 
Peter Xu

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

[Peter Xu]

On Tue, Sep 30, 2025 at 11:36:53AM +0200, David Hildenbrand wrote:
> > +/* 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.
> > +	 *
> > +	 * NOTE: VM_UFFD_MISSING is still only supported under mm/ so far.
> > +	 */
> > +	unsigned long uffd_features;
> 
> This variable name is a bit confusing , because it's all about vma flags,
> not uffd features. Just reading the variable, I would rather connect it to
> things like UFFD_FEATURE_WP_UNPOPULATED.
> 
> As currently used for VM flags, maybe you should call this
> 
> 	unsigned long uffd_vm_flags;
> 
> or sth like that.

Indeed it's slightly confusing.  However uffd_vm_flags is confusing in
another way, where it seems to imply some flags similar to vm_flags that is
prone to change.

How about uffd_vm_flags_supported / uffd_modes_supported?

> 
> 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.

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.

Do you mind I still keep it as-is?  So far that's still the clearest I can
think of.  It's only set when some support is added to a memory type, so
it's a one-time shot.

Thanks,

-- 
Peter Xu

Re: [PATCH v3 4/4] mm: Apply vm_uffd_ops API to core mm

[Peter Xu]

On Tue, Sep 30, 2025 at 11:23:19AM +0200, David Hildenbrand wrote:
> On 26.09.25 23:16, Peter Xu wrote:
> > Move userfaultfd core to use new vm_uffd_ops API. After this change file
> > systems that implement vm_operations_struct can start using new API for
> > userfaultfd operations.
> > 
> > When at it, moving vma_can_userfault() into mm/userfaultfd.c instead,
> > because it's getting too big.  It's only used in slow paths so it shouldn't
> > be an issue.  Move the pte marker check before wp_async, which might be
> > more intuitive because wp_async depends on pte markers.  That shouldn't
> > cause any functional change though because only one check would take effect
> > depending on whether pte marker was selected in config.
> > 
> > This will also remove quite some hard-coded checks for either shmem or
> > hugetlbfs.  Now all the old checks should still work but with vm_uffd_ops.
> > 
> > Note that anonymous memory will still need to be processed separately
> > because it doesn't have vm_ops at all.
> > 
> > Reviewed-by: James Houghton <jthoughton@google.com>
> > Acked-by: Mike Rapoport <rppt@kernel.org>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> 
> [...]
> 
> > +++ b/mm/userfaultfd.c
> > @@ -20,6 +20,43 @@
> >   #include "internal.h"
> >   #include "swap.h"
> > +bool vma_can_userfault(struct vm_area_struct *vma, vm_flags_t vm_flags,
> > +		       bool wp_async)
> > +{
> > +	unsigned long supported;
> > +
> > +	if (vma->vm_flags & VM_DROPPABLE)
> > +		return false;
> > +
> > +	vm_flags &= __VM_UFFD_FLAGS;
> > +
> > +#ifndef CONFIG_PTE_MARKER_UFFD_WP
> 
> While at it, you can turn that into an
> !IS_ENABLED(CONFIG_PTE_MARKER_UFFD_WP) to avoid the ifdef.
> 
> > +	/*
> > +	 * If user requested uffd-wp but not enabled pte markers for
> > +	 * uffd-wp, then any file system (like shmem or hugetlbfs) are not
> > +	 * supported but only anonymous.
> > +	 */
> > +	if ((vm_flags & VM_UFFD_WP) && !vma_is_anonymous(vma))
> > +		return false;
> > +#endif
> > +	/*
> > +	 * If wp async enabled, and WP is the only mode enabled, allow any
> > +	 * memory type.
> > +	 */
> > +	if (wp_async && (vm_flags == VM_UFFD_WP))
> > +		return true;
> 
> 
> > +
> > +	if (vma_is_anonymous(vma))
> > +		/* Anonymous has no page cache, MINOR not supported */
> > +		supported = VM_UFFD_MISSING | VM_UFFD_WP;
> > +	else if (vma_get_uffd_ops(vma))
> > +		supported = vma_get_uffd_ops(vma)->uffd_features;
> > +	else
> > +		return false;
> 
> To avoid the hidde return here, I think you can just do
> 
> 	supported = 0;
> 
> 
> Or even cleaner, just do
> 
> unsigned long supported = 0
> ...
> if (vma_is_anonymous(vma))
> 	supported = ...
> else if (vma_get_uffd_ops(vma))
> 	supported = ...
> return ...
> 
> > +
> > +	return !(vm_flags & (~supported));
> 
> I think this can just be:
> 
> 	return !(vm_flags & ~supported);

Sure thing, I'll apply everything you mentioned above.

Thanks,

-- 
Peter Xu

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

[David Hildenbrand]

On 30.09.25 20:48, Peter Xu wrote:
> On Tue, Sep 30, 2025 at 11:36:53AM +0200, David Hildenbrand wrote:
>>> +/* 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.
>>> +	 *
>>> +	 * NOTE: VM_UFFD_MISSING is still only supported under mm/ so far.
>>> +	 */
>>> +	unsigned long uffd_features;
>>
>> This variable name is a bit confusing , because it's all about vma flags,
>> not uffd features. Just reading the variable, I would rather connect it to
>> things like UFFD_FEATURE_WP_UNPOPULATED.
>>
>> As currently used for VM flags, maybe you should call this
>>
>> 	unsigned long uffd_vm_flags;
>>
>> or sth like that.
> 
> Indeed it's slightly confusing.  However uffd_vm_flags is confusing in
> another way, where it seems to imply some flags similar to vm_flags that is
> prone to change.
> 
> How about uffd_vm_flags_supported / uffd_modes_supported?

The former would make things clearer when we are at least not talking 
about uffd features.

> 
>>
>> 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
> 
> 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?

As a side note, hugetlbfs support for ZEROPAGE should be fairly easy: 
similar to shmem support, simply allocate a zeroed hugetlb folio.

> 
> Do you mind I still keep it as-is?

I would prefer if we find a way to not have this dependency between both 
feature/ioctl thingies. It just looks rather odd.

But let's hear if there are other opinions.

-- 
Cheers

David / dhildenb

Re: [PATCH v3 0/4] mm/userfaultfd: modulize memory types

[Liam R. Howlett]

* Peter Xu <peterx@redhat.com> [250926 17:17]:
> [based on latest akpm/mm-new of Sep 26th, commit e612c80ae0aeb]
> 
> v3 changelog:
> - Fixed checkpatch issues on spaces or typedef
> - Dropped uffd_copy() API
> - Refined commit messages here and there to reflect the removal of uffd_copy()
> 
> v1: https://lore.kernel.org/r/20250620190342.1780170-1-peterx@redhat.com
> v2: https://lore.kernel.org/r/20250627154655.2085903-1-peterx@redhat.com
> 
> This series is an alternative proposal of what Nikita proposed here on the
> initial three patches:
> 
>   https://lore.kernel.org/r/20250404154352.23078-1-kalyazin@amazon.com
> 
> This is not yet relevant to any guest-memfd support, but paving way for it.

It would be much easier to review this with the guest-memfd support in
this patch set.  Any chance of including the target user?

> Here, the major goal is to make kernel modules be able to opt-in with any
> form of userfaultfd supports, like guest-memfd.  This alternative option
> should hopefully be cleaner, and avoid leaking userfault details into
> vm_ops.fault().
> 
> It also means this series does not depend on anything.  It's a pure
> refactoring of userfaultfd internals to provide a generic API, so that
> other types of files, especially RAM based, can support userfaultfd without
> touching mm/ at all.
> 
> To achieve it, this series introduced a file operation called vm_uffd_ops.
> The ops needs to be provided when a file type supports any of userfaultfd.
> 
> With that, I moved both hugetlbfs and shmem over, whenever possible.  So
> far due to concerns on exposing an uffd_copy() API, the MISSING faults are
> still separately processed and can only be done within mm/.  Hugetlbfs kept
> its special paths untouched.
> 
> An example of shmem uffd_ops:
> 
> static const vm_uffd_ops shmem_uffd_ops = {
> 	.uffd_features	= 	__VM_UFFD_FLAGS,
> 	.uffd_ioctls	= 	BIT(_UFFDIO_COPY) |
> 				BIT(_UFFDIO_ZEROPAGE) |
> 				BIT(_UFFDIO_WRITEPROTECT) |
> 				BIT(_UFFDIO_CONTINUE) |
> 				BIT(_UFFDIO_POISON),
> 	.uffd_get_folio	=	shmem_uffd_get_folio,
> };
> 
> No functional change expected at all after the whole series applied.  There
> might be some slightly stricter check on uffd ops here and there in the
> last patch, but that really shouldn't stand out anywhere to anyone.
> 
> For testing: besides the cross-compilation tests, I did also try with
> uffd-stress in a VM to measure any perf difference before/after the change;
> The static call becomes a pointer now.  I really cannot measure anything
> different, which is more or less expected.
> 
> Comments welcomed, thanks.
> 
> Peter Xu (4):
>   mm: Introduce vm_uffd_ops API
>   mm/shmem: Support vm_uffd_ops API
>   mm/hugetlb: Support vm_uffd_ops API
>   mm: Apply vm_uffd_ops API to core mm
> 
>  include/linux/mm.h            |   9 +++
>  include/linux/userfaultfd_k.h |  83 ++++++++++++++++-----------
>  mm/hugetlb.c                  |  19 +++++++
>  mm/shmem.c                    |  25 +++++++++
>  mm/userfaultfd.c              | 102 ++++++++++++++++++++++++++--------
>  5 files changed, 181 insertions(+), 57 deletions(-)
> 
> -- 
> 2.50.1
> 

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

[Peter Xu]

On Tue, Sep 30, 2025 at 09:19:05PM +0200, David Hildenbrand wrote:
> On 30.09.25 20:48, Peter Xu wrote:
> > On Tue, Sep 30, 2025 at 11:36:53AM +0200, David Hildenbrand wrote:
> > > > +/* 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.
> > > > +	 *
> > > > +	 * NOTE: VM_UFFD_MISSING is still only supported under mm/ so far.
> > > > +	 */
> > > > +	unsigned long uffd_features;
> > > 
> > > This variable name is a bit confusing , because it's all about vma flags,
> > > not uffd features. Just reading the variable, I would rather connect it to
> > > things like UFFD_FEATURE_WP_UNPOPULATED.
> > > 
> > > As currently used for VM flags, maybe you should call this
> > > 
> > > 	unsigned long uffd_vm_flags;
> > > 
> > > or sth like that.
> > 
> > Indeed it's slightly confusing.  However uffd_vm_flags is confusing in
> > another way, where it seems to imply some flags similar to vm_flags that is
> > prone to change.
> > 
> > How about uffd_vm_flags_supported / uffd_modes_supported?
> 
> The former would make things clearer when we are at least not talking about
> uffd features.

I'll go with it.

> 
> > 
> > > 
> > > 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.

> > 
> > 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..

> 
> 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.

> 
> > 
> > Do you mind I still keep it as-is?
> 
> I would prefer if we find a way to not have this dependency between both
> feature/ioctl thingies. It just looks rather odd.
> 
> But let's hear if there are other opinions.

Sure.

-- 
Peter Xu

Re: [PATCH v3 0/4] mm/userfaultfd: modulize memory types

[Peter Xu]

On Tue, Sep 30, 2025 at 03:49:34PM -0400, Liam R. Howlett wrote:
> * Peter Xu <peterx@redhat.com> [250926 17:17]:
> > [based on latest akpm/mm-new of Sep 26th, commit e612c80ae0aeb]
> > 
> > v3 changelog:
> > - Fixed checkpatch issues on spaces or typedef
> > - Dropped uffd_copy() API
> > - Refined commit messages here and there to reflect the removal of uffd_copy()
> > 
> > v1: https://lore.kernel.org/r/20250620190342.1780170-1-peterx@redhat.com
> > v2: https://lore.kernel.org/r/20250627154655.2085903-1-peterx@redhat.com
> > 
> > This series is an alternative proposal of what Nikita proposed here on the
> > initial three patches:
> > 
> >   https://lore.kernel.org/r/20250404154352.23078-1-kalyazin@amazon.com
> > 
> > This is not yet relevant to any guest-memfd support, but paving way for it.
> 
> It would be much easier to review this with the guest-memfd support in
> this patch set.  Any chance of including the target user?

It should be something like what Nikita posted previously here against v1:

https://lore.kernel.org/all/114133f5-0282-463d-9d65-3143aa658806@amazon.com/

I'll add the link into the cover letter when I repost. That formal patch
logically will need to be reviewed from KVM side after this patch lands.

Thanks,

-- 
Peter Xu

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

[David Hildenbrand]

>>>> 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

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

[Peter Xu]

On Wed, Oct 01, 2025 at 03:58:14PM +0200, David Hildenbrand wrote:
> > > > > 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.

It adds the extra layer of mapping that will only be used in vm_uffd_ops
and the helper that will consume it.

But I confess this might be subjective.

> 
> > > > 
> > > > 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?

I am not aware of anyone discussing this yet, but IMHO we need to support
it at least for the !CoCo use cases.

I do not know how CoCo manages poisoned pages, e.g. if they are kept being
encrypted or not even if corrupted.

Thanks,

-- 
Peter Xu

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

[David Hildenbrand]

On 01.10.25 16:35, Peter Xu wrote:
> On Wed, Oct 01, 2025 at 03:58:14PM +0200, David Hildenbrand wrote:
>>>>>> 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.
> 
> It adds the extra layer of mapping that will only be used in vm_uffd_ops
> and the helper that will consume it.

Agreed, while making the API cleaner. I don't easily see what's 
confusing about that, though.

I think it can be done with a handful of LOC and avoid having to use VM_ 
flags in this API.

-- 
Cheers

David / dhildenb

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

[Peter Xu]

On Wed, Oct 01, 2025 at 04:39:50PM +0200, David Hildenbrand wrote:
> On 01.10.25 16:35, Peter Xu wrote:
> > On Wed, Oct 01, 2025 at 03:58:14PM +0200, David Hildenbrand wrote:
> > > > > > > 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.
> > 
> > It adds the extra layer of mapping that will only be used in vm_uffd_ops
> > and the helper that will consume it.
> 
> Agreed, while making the API cleaner. I don't easily see what's confusing
> about that, though.

It will introduce another set of userfaultfd features, making it hard to
say what is the difference between the new set and UFFD_FEATURE_*.

> 
> I think it can be done with a handful of LOC and avoid having to use VM_
> flags in this API.

I waited for a few days, unfortunately we didn't get a second opinion.

David, do you feel OK I repost with the rest comments (almost renames), and
if we want yet another new set of features for userfaultfd, we work it on
top?  It'll be a trivial one patch to do the mappings if we want.  The
current patch is also the minimum changeset we need to unblock guest-memfd
minor fault.

Thanks,

-- 
Peter Xu

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

[David Hildenbrand]

Hi Peter,

>>
>> Agreed, while making the API cleaner. I don't easily see what's confusing
>> about that, though.
> 
> It will introduce another set of userfaultfd features, making it hard to
> say what is the difference between the new set and UFFD_FEATURE_*.

That's why I suggested we use something like "BACKEND" as part of the name.

If we can come up with something better, like talking about capabilities 
than features to better distinguish it, even better (UFFD_BACKEND_CAP_* 
etc).

> 
>>
>> I think it can be done with a handful of LOC and avoid having to use VM_
>> flags in this API.
> 
> I waited for a few days, unfortunately we didn't get a second opinion.
> 

We are currently in the merge window and many people are either out or 
were attending kernel recipes and are on their way back.

I'd suggest waiting a couple of days for more feedback -- right now 
there is no need to rush either way, we have the complete development 
cycle starting in one week.

-- 
Cheers

David / dhildenb

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

[Liam R. Howlett]

* Peter Xu <peterx@redhat.com> [251003 10:02]:
> On Wed, Oct 01, 2025 at 04:39:50PM +0200, David Hildenbrand wrote:
> > On 01.10.25 16:35, Peter Xu wrote:
> > > On Wed, Oct 01, 2025 at 03:58:14PM +0200, David Hildenbrand wrote:
> > > > > > > > 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).

I think this supports the need for a code clean up before applying an
API that generalizes it?

I would expect the uffd code that needs the same uffd_feature would
logically have the same uffd flags for the uffd_ops, but that's not the
case here?

Is this because uffd_feature != UFFD_FEATURE_* ... or are the internal
UFFD_FEATURE_* not the same thing?

> > > > > > > > 
> > > > > > > > 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.
> > > 
> > > It adds the extra layer of mapping that will only be used in vm_uffd_ops
> > > and the helper that will consume it.
> > 
> > Agreed, while making the API cleaner. I don't easily see what's confusing
> > about that, though.
> 
> It will introduce another set of userfaultfd features, making it hard to
> say what is the difference between the new set and UFFD_FEATURE_*.

If it's not using UFFD_FEATURE_ defines, then please don't use
uffd_feature for it in the uffd_ops.  That seems like a recipe for
confusion.

> 
> > 
> > I think it can be done with a handful of LOC and avoid having to use VM_
> > flags in this API.
> 
> I waited for a few days, unfortunately we didn't get a second opinion.

Sorry, been pretty busy here.

If we can avoid the flags/features, then I'd rather that (the derived
from uffd_ops == NULL for support).  We can always add something else
later.

If we have to have a feature/flag setting, then please avoid using
uffd_feature unless we use it with UFFD_FEATURE_ - which I think, we've
ruled out?

Thanks,
Liam

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

[Peter Xu]

On Mon, Oct 06, 2025 at 03:06:39PM -0400, Liam R. Howlett wrote:
> * Peter Xu <peterx@redhat.com> [251003 10:02]:
> > On Wed, Oct 01, 2025 at 04:39:50PM +0200, David Hildenbrand wrote:
> > > On 01.10.25 16:35, Peter Xu wrote:
> > > > On Wed, Oct 01, 2025 at 03:58:14PM +0200, David Hildenbrand wrote:
> > > > > > > > > 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).
> 
> I think this supports the need for a code clean up before applying an
> API that generalizes it?
> 
> I would expect the uffd code that needs the same uffd_feature would
> logically have the same uffd flags for the uffd_ops, but that's not the
> case here?
> 
> Is this because uffd_feature != UFFD_FEATURE_* ... or are the internal
> UFFD_FEATURE_* not the same thing?
> 
> > > > > > > > > 
> > > > > > > > > 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.
> > > > 
> > > > It adds the extra layer of mapping that will only be used in vm_uffd_ops
> > > > and the helper that will consume it.
> > > 
> > > Agreed, while making the API cleaner. I don't easily see what's confusing
> > > about that, though.
> > 
> > It will introduce another set of userfaultfd features, making it hard to
> > say what is the difference between the new set and UFFD_FEATURE_*.
> 
> If it's not using UFFD_FEATURE_ defines, then please don't use
> uffd_feature for it in the uffd_ops.  That seems like a recipe for
> confusion.
> 
> > 
> > > 
> > > I think it can be done with a handful of LOC and avoid having to use VM_
> > > flags in this API.
> > 
> > I waited for a few days, unfortunately we didn't get a second opinion.
> 
> Sorry, been pretty busy here.
> 
> If we can avoid the flags/features, then I'd rather that (the derived
> from uffd_ops == NULL for support).  We can always add something else
> later.
> 
> If we have to have a feature/flag setting, then please avoid using
> uffd_feature unless we use it with UFFD_FEATURE_ - which I think, we've
> ruled out?

Yes, there was no plan to use UFFD_FEATURE_* in vm_uffd_ops.  It's because
UFFD_FEATURE_* was introduced to almost let userapp have a way to probe the
capability of the kernel, rather than the layer to describe what features
userfaultfd has.

For example, we have UFFD_FEATURE_MISSING_SHMEM declaring that "the current
kernel supports MISSING mode userfaultfd on shmem".  This feature flag is
essentially of no use for any other memory types, hence not applicable to
vm_uffd_ops.  OTOH, we don't have a feature flag to represent "userfaultfd
MISSING mode".

Thanks,

-- 
Peter Xu

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

[Liam R. Howlett]

* Peter Xu <peterx@redhat.com> [251006 17:02]:
> On Mon, Oct 06, 2025 at 03:06:39PM -0400, Liam R. Howlett wrote:
> > * Peter Xu <peterx@redhat.com> [251003 10:02]:
> > > On Wed, Oct 01, 2025 at 04:39:50PM +0200, David Hildenbrand wrote:
> > > > On 01.10.25 16:35, Peter Xu wrote:
> > > > > On Wed, Oct 01, 2025 at 03:58:14PM +0200, David Hildenbrand wrote:
> > > > > > > > > > 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).
> > 
> > I think this supports the need for a code clean up before applying an
> > API that generalizes it?
> > 
> > I would expect the uffd code that needs the same uffd_feature would
> > logically have the same uffd flags for the uffd_ops, but that's not the
> > case here?
> > 
> > Is this because uffd_feature != UFFD_FEATURE_* ... or are the internal
> > UFFD_FEATURE_* not the same thing?
> > 
> > > > > > > > > > 
> > > > > > > > > > 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.
> > > > > 
> > > > > It adds the extra layer of mapping that will only be used in vm_uffd_ops
> > > > > and the helper that will consume it.
> > > > 
> > > > Agreed, while making the API cleaner. I don't easily see what's confusing
> > > > about that, though.
> > > 
> > > It will introduce another set of userfaultfd features, making it hard to
> > > say what is the difference between the new set and UFFD_FEATURE_*.
> > 
> > If it's not using UFFD_FEATURE_ defines, then please don't use
> > uffd_feature for it in the uffd_ops.  That seems like a recipe for
> > confusion.
> > 
> > > 
> > > > 
> > > > I think it can be done with a handful of LOC and avoid having to use VM_
> > > > flags in this API.
> > > 
> > > I waited for a few days, unfortunately we didn't get a second opinion.
> > 
> > Sorry, been pretty busy here.
> > 
> > If we can avoid the flags/features, then I'd rather that (the derived
> > from uffd_ops == NULL for support).  We can always add something else
> > later.
> > 
> > If we have to have a feature/flag setting, then please avoid using
> > uffd_feature unless we use it with UFFD_FEATURE_ - which I think, we've
> > ruled out?
> 
> Yes, there was no plan to use UFFD_FEATURE_* in vm_uffd_ops.  It's because
> UFFD_FEATURE_* was introduced to almost let userapp have a way to probe the
> capability of the kernel, rather than the layer to describe what features
> userfaultfd has.
> 
> For example, we have UFFD_FEATURE_MISSING_SHMEM declaring that "the current
> kernel supports MISSING mode userfaultfd on shmem".  This feature flag is
> essentially of no use for any other memory types, hence not applicable to
> vm_uffd_ops.  OTOH, we don't have a feature flag to represent "userfaultfd
> MISSING mode".
> 

hehe, the overloaded terms here are numerous, but I think I get what you
are saying.  It's funny that FEATURE_MISSING isn't a check for a missing
feature, but really to check if the register mode missing is available.

I'd also rather not have ioctls and features/flags.  It seems reasonable
to drop the ioctl, like David said.

I assume there is some future plan for flags, or is this for versioning?

I'd like to one day even remove the suggested backend types and instead
use handlers in the uffd_ops directly, although it is difficult to know
if this is reasonable today.

Thanks,
Liam

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

[Peter Xu]

On Mon, Oct 06, 2025 at 11:31:19PM -0400, Liam R. Howlett wrote:
> * Peter Xu <peterx@redhat.com> [251006 17:02]:
> > On Mon, Oct 06, 2025 at 03:06:39PM -0400, Liam R. Howlett wrote:
> > > * Peter Xu <peterx@redhat.com> [251003 10:02]:
> > > > On Wed, Oct 01, 2025 at 04:39:50PM +0200, David Hildenbrand wrote:
> > > > > On 01.10.25 16:35, Peter Xu wrote:
> > > > > > On Wed, Oct 01, 2025 at 03:58:14PM +0200, David Hildenbrand wrote:
> > > > > > > > > > > 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).
> > > 
> > > I think this supports the need for a code clean up before applying an
> > > API that generalizes it?
> > > 
> > > I would expect the uffd code that needs the same uffd_feature would
> > > logically have the same uffd flags for the uffd_ops, but that's not the
> > > case here?
> > > 
> > > Is this because uffd_feature != UFFD_FEATURE_* ... or are the internal
> > > UFFD_FEATURE_* not the same thing?
> > > 
> > > > > > > > > > > 
> > > > > > > > > > > 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.
> > > > > > 
> > > > > > It adds the extra layer of mapping that will only be used in vm_uffd_ops
> > > > > > and the helper that will consume it.
> > > > > 
> > > > > Agreed, while making the API cleaner. I don't easily see what's confusing
> > > > > about that, though.
> > > > 
> > > > It will introduce another set of userfaultfd features, making it hard to
> > > > say what is the difference between the new set and UFFD_FEATURE_*.
> > > 
> > > If it's not using UFFD_FEATURE_ defines, then please don't use
> > > uffd_feature for it in the uffd_ops.  That seems like a recipe for
> > > confusion.
> > > 
> > > > 
> > > > > 
> > > > > I think it can be done with a handful of LOC and avoid having to use VM_
> > > > > flags in this API.
> > > > 
> > > > I waited for a few days, unfortunately we didn't get a second opinion.
> > > 
> > > Sorry, been pretty busy here.
> > > 
> > > If we can avoid the flags/features, then I'd rather that (the derived
> > > from uffd_ops == NULL for support).  We can always add something else
> > > later.
> > > 
> > > If we have to have a feature/flag setting, then please avoid using
> > > uffd_feature unless we use it with UFFD_FEATURE_ - which I think, we've
> > > ruled out?
> > 
> > Yes, there was no plan to use UFFD_FEATURE_* in vm_uffd_ops.  It's because
> > UFFD_FEATURE_* was introduced to almost let userapp have a way to probe the
> > capability of the kernel, rather than the layer to describe what features
> > userfaultfd has.
> > 
> > For example, we have UFFD_FEATURE_MISSING_SHMEM declaring that "the current
> > kernel supports MISSING mode userfaultfd on shmem".  This feature flag is
> > essentially of no use for any other memory types, hence not applicable to
> > vm_uffd_ops.  OTOH, we don't have a feature flag to represent "userfaultfd
> > MISSING mode".
> > 
> 
> hehe, the overloaded terms here are numerous, but I think I get what you
> are saying.  It's funny that FEATURE_MISSING isn't a check for a missing
> feature, but really to check if the register mode missing is available.
> 
> I'd also rather not have ioctls and features/flags.  It seems reasonable
> to drop the ioctl, like David said.
> 
> I assume there is some future plan for flags, or is this for versioning?
> 
> I'd like to one day even remove the suggested backend types and instead
> use handlers in the uffd_ops directly, although it is difficult to know
> if this is reasonable today.

The current proposal will be two fields (modes_supported,
ioctls_supported).  If we add yet another feature-backend flags, that will
only be used to map to the two fields but using one flag.

Could you elaborate what's the handler you described?  Is that one handler
returning both of the fields?

If so, I'd prefer that rather than introducing feature-backend flags,
because I want to avoid introducing another different feature set to uffd.

Thanks,

-- 
Peter Xu

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

[Liam R. Howlett]

* Peter Xu <peterx@redhat.com> [251007 09:52]:
> On Mon, Oct 06, 2025 at 11:31:19PM -0400, Liam R. Howlett wrote:
> > * Peter Xu <peterx@redhat.com> [251006 17:02]:
> > > On Mon, Oct 06, 2025 at 03:06:39PM -0400, Liam R. Howlett wrote:
> > > > * Peter Xu <peterx@redhat.com> [251003 10:02]:
> > > > > On Wed, Oct 01, 2025 at 04:39:50PM +0200, David Hildenbrand wrote:
> > > > > > On 01.10.25 16:35, Peter Xu wrote:
> > > > > > > On Wed, Oct 01, 2025 at 03:58:14PM +0200, David Hildenbrand wrote:
> > > > > > > > > > > > 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).
> > > > 
> > > > I think this supports the need for a code clean up before applying an
> > > > API that generalizes it?
> > > > 
> > > > I would expect the uffd code that needs the same uffd_feature would
> > > > logically have the same uffd flags for the uffd_ops, but that's not the
> > > > case here?
> > > > 
> > > > Is this because uffd_feature != UFFD_FEATURE_* ... or are the internal
> > > > UFFD_FEATURE_* not the same thing?
> > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 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.
> > > > > > > 
> > > > > > > It adds the extra layer of mapping that will only be used in vm_uffd_ops
> > > > > > > and the helper that will consume it.
> > > > > > 
> > > > > > Agreed, while making the API cleaner. I don't easily see what's confusing
> > > > > > about that, though.
> > > > > 
> > > > > It will introduce another set of userfaultfd features, making it hard to
> > > > > say what is the difference between the new set and UFFD_FEATURE_*.
> > > > 
> > > > If it's not using UFFD_FEATURE_ defines, then please don't use
> > > > uffd_feature for it in the uffd_ops.  That seems like a recipe for
> > > > confusion.
> > > > 
> > > > > 
> > > > > > 
> > > > > > I think it can be done with a handful of LOC and avoid having to use VM_
> > > > > > flags in this API.
> > > > > 
> > > > > I waited for a few days, unfortunately we didn't get a second opinion.
> > > > 
> > > > Sorry, been pretty busy here.
> > > > 
> > > > If we can avoid the flags/features, then I'd rather that (the derived
> > > > from uffd_ops == NULL for support).  We can always add something else
> > > > later.
> > > > 
> > > > If we have to have a feature/flag setting, then please avoid using
> > > > uffd_feature unless we use it with UFFD_FEATURE_ - which I think, we've
> > > > ruled out?
> > > 
> > > Yes, there was no plan to use UFFD_FEATURE_* in vm_uffd_ops.  It's because
> > > UFFD_FEATURE_* was introduced to almost let userapp have a way to probe the
> > > capability of the kernel, rather than the layer to describe what features
> > > userfaultfd has.
> > > 
> > > For example, we have UFFD_FEATURE_MISSING_SHMEM declaring that "the current
> > > kernel supports MISSING mode userfaultfd on shmem".  This feature flag is
> > > essentially of no use for any other memory types, hence not applicable to
> > > vm_uffd_ops.  OTOH, we don't have a feature flag to represent "userfaultfd
> > > MISSING mode".
> > > 
> > 
> > hehe, the overloaded terms here are numerous, but I think I get what you
> > are saying.  It's funny that FEATURE_MISSING isn't a check for a missing
> > feature, but really to check if the register mode missing is available.
> > 
> > I'd also rather not have ioctls and features/flags.  It seems reasonable
> > to drop the ioctl, like David said.
> > 
> > I assume there is some future plan for flags, or is this for versioning?
> > 
> > I'd like to one day even remove the suggested backend types and instead
> > use handlers in the uffd_ops directly, although it is difficult to know
> > if this is reasonable today.
> 
> The current proposal will be two fields (modes_supported,
> ioctls_supported).  If we add yet another feature-backend flags, that will
> only be used to map to the two fields but using one flag.
> 
> Could you elaborate what's the handler you described?  Is that one handler
> returning both of the fields?
> 
> 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?

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.

We also seem to call all the mfill options with a static uffd flag,
which then gets validated against that ioctl_supports.  So we already
know at the call site where we want to go, but we're doing it in a
round-about way.

I think this is what Matthew was saying about a middle-layer [1].

If you had dispatched to a fault handler, then all the hugetlbfs checks
in here [2] will hopefully go away, for example.

Thanks,
Liam

[1]. https://lore.kernel.org/kvm/Z0DOdTRAaK3whZKW@casper.infradead.org/
[2]. https://elixir.bootlin.com/linux/v6.17.1/source/fs/userfaultfd.c#L476

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

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

[Peter Xu]

On Tue, Oct 07, 2025 at 06:14:01PM +0200, David Hildenbrand wrote:
> > > 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,

This is not needed because the logic to handle userfault isn't very type
sensitive. Hugetlb is the only one that differs very lightly, but again I
think we should better take hugetlbfs as special always as of now, per all
the previous discussions on hugetlb unifications.

> >          .write_protect = mwriteprotect_range,

This is not needed.  WP processing is the same.

> >          .minor = hugetlb_handle_userfault_minor,

We can do that, but ultimately we do almost exactly the same for all memory
types except a fetch on the page cache.  IMHO this will make it awkward..
because 99% of the minor hooks will do the same thing..  OTOH, it makes
more sense to me that the hook is defined to cover what is different on the
memory type.

I'll stop commenting on the rest.

> > 
> >          .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).

I didn't send RFC, likely you meant this reply in v1?

https://lore.kernel.org/all/0126fa5f-b5aa-4a17-80d6-d428105e45c7@redhat.com/

        I agree that another special-purpose file (like implemented by
        guest_memfd) would need that. But if we could get rid of
        "hugetlb"/"shmem" special-casing in userfaultfd, it would be a
        rasonable independent cleanup.

Get rid of hugetlbfs is still not my goal as of in this series.

OTOH, I generalized shmem and removed shmem.h header from userfaultfd, but
that was prior versions when with uffd_copy() and it was rejected.

What should I do now to move this series forward?  Could anyone provide a
solid answer?

Thanks,

-- 
Peter Xu

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

[Liam R. Howlett]

* Peter Xu <peterx@redhat.com> [251007 12:47]:

...

> > > 
> > > 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).
> 
> I didn't send RFC, likely you meant this reply in v1?
> 
> https://lore.kernel.org/all/0126fa5f-b5aa-4a17-80d6-d428105e45c7@redhat.com/
> 
>         I agree that another special-purpose file (like implemented by
>         guest_memfd) would need that. But if we could get rid of
>         "hugetlb"/"shmem" special-casing in userfaultfd, it would be a
>         rasonable independent cleanup.
> 
> Get rid of hugetlbfs is still not my goal as of in this series.

My example picked hugetlbfs because it is the most special of the types
of memory we have (so very special).  If the interface works for
hugetlbfs, then the rest will use a subset of the features and be happy.

IOW, doing the hard thing first makes what follows easy.  Doing the easy
thing first may mean rewriting the easy thing once you arrive at the
more difficult part.

> 
> OTOH, I generalized shmem and removed shmem.h header from userfaultfd, but
> that was prior versions when with uffd_copy() and it was rejected.
> 
> What should I do now to move this series forward?  Could anyone provide a
> solid answer?

My understanding is that we need an interface for memory types so they
are modularised, with the short term goal of solving the faulting
support for guest_memfd and the long term goal of code cleanup, or at
least don't make things worse.

I think we all agree on that?

I propose that we need to add the minimum amount of uffd_ops to support
guest_memfd's specialness without creating an interface that makes
things worse.

It is very difficult to see a reason to pass in two variables (modes and
ioctls) to dispatch to the correct function in a struct that could
simply point to the function in the first place.  If we can avoid that,
then it would be good.

Looking at the example you pointed to here [1], It appears the minimal
viable product would need to implement this:

uffd_ops = {
        .get_folio = <>,
        .minor_fault = <>,
        .atomic_fill_continue = <>,
}

Then shmem and hugetlb can define these and end up calling them in
today's spaghetti, but we are free to append more uffd_ops to reduce the
spaghetti later.

If using new #defines to clears up translations of features/modes and
ioctl codes, then please do that.  These should be removable once the
uffd_ops grows to support all necessary calls.

If there are places where you need to consult the modes/ioctls and a
translation does not work, then you could add something to uffd_ops that
is NULL for guest_memfd and use it to determine if the code path is
valid.  But this code should already exist for the other memory types.

What does everyone think?

[1]. https://lore.kernel.org/all/114133f5-0282-463d-9d65-3143aa658806@amazon.com/

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

[Peter Xu]

On Tue, Oct 07, 2025 at 02:46:46PM -0400, Liam R. Howlett wrote:
> * Peter Xu <peterx@redhat.com> [251007 12:47]:
> 
> ...
> 
> > > > 
> > > > 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).
> > 
> > I didn't send RFC, likely you meant this reply in v1?
> > 
> > https://lore.kernel.org/all/0126fa5f-b5aa-4a17-80d6-d428105e45c7@redhat.com/
> > 
> >         I agree that another special-purpose file (like implemented by
> >         guest_memfd) would need that. But if we could get rid of
> >         "hugetlb"/"shmem" special-casing in userfaultfd, it would be a
> >         rasonable independent cleanup.
> > 
> > Get rid of hugetlbfs is still not my goal as of in this series.
> 
> My example picked hugetlbfs because it is the most special of the types
> of memory we have (so very special).  If the interface works for
> hugetlbfs, then the rest will use a subset of the features and be happy.
> 
> IOW, doing the hard thing first makes what follows easy.  Doing the easy
> thing first may mean rewriting the easy thing once you arrive at the
> more difficult part.

In general I agree, but hugetlbfs is special when it is major-feature
frozen.  IMHO we shouldn't design an API to suite hugetlbfs, but only
trying to move it closer to all the rest of file systems as much as
possible.

So the generic API should be designed without hugetlbfs involvement.  Then
if there is guest-memfd / hugetlbfsv2 / ... they should fit into this API.

> 
> > 
> > OTOH, I generalized shmem and removed shmem.h header from userfaultfd, but
> > that was prior versions when with uffd_copy() and it was rejected.
> > 
> > What should I do now to move this series forward?  Could anyone provide a
> > solid answer?
> 
> My understanding is that we need an interface for memory types so they
> are modularised, with the short term goal of solving the faulting
> support for guest_memfd and the long term goal of code cleanup, or at
> least don't make things worse.
> 
> I think we all agree on that?
> 
> I propose that we need to add the minimum amount of uffd_ops to support
> guest_memfd's specialness without creating an interface that makes
> things worse.
> 
> It is very difficult to see a reason to pass in two variables (modes and
> ioctls) to dispatch to the correct function in a struct that could

The reason is "modes" cannot directly be intepreted into ioctls.  But
indeed ioctls can be intepreted into supported modes.

> simply point to the function in the first place.  If we can avoid that,
> then it would be good.
> 
> Looking at the example you pointed to here [1], It appears the minimal
> viable product would need to implement this:
> 
> uffd_ops = {
>         .get_folio = <>,
>         .minor_fault = <>,
>         .atomic_fill_continue = <>,

These three are fundamentally the same thing.  As explained, if we have
get_folio() we don't need the rest.  However we still need something to
describe e.g. shmem supports MISSING mode.

> }
> 
> Then shmem and hugetlb can define these and end up calling them in
> today's spaghetti, but we are free to append more uffd_ops to reduce the
> spaghetti later.
> 
> If using new #defines to clears up translations of features/modes and
> ioctl codes, then please do that.  These should be removable once the
> uffd_ops grows to support all necessary calls.
> 
> If there are places where you need to consult the modes/ioctls and a
> translation does not work, then you could add something to uffd_ops that
> is NULL for guest_memfd and use it to determine if the code path is
> valid.  But this code should already exist for the other memory types.
> 
> What does everyone think?
> 
> [1]. https://lore.kernel.org/all/114133f5-0282-463d-9d65-3143aa658806@amazon.com/

Would it look better to you if I drop uffd_modes_supported, deducing it
from uffd_ioctls_supported?

I believe that's what David mentioned very initially here:

https://lore.kernel.org/r/f1da3505-f17f-4829-80c1-696b1d99057d@redhat.com

I'd rather go with the two fields, but if we're trying to introduce another
feature sets almost only for vm_uffd_ops, I'd prefer keeping it simple, and
deduce the modes from ioctls.

Is that ok for you?  So it'll have (1) get_folio(), (2) supported_ioctls.
That's all.

Thanks,

-- 
Peter Xu

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

[David Hildenbrand]

> I believe that's what David mentioned very initially here:
> 
> https://lore.kernel.org/r/f1da3505-f17f-4829-80c1-696b1d99057d@redhat.com
> 
> I'd rather go with the two fields, but if we're trying to introduce another
> feature sets almost only for vm_uffd_ops, I'd prefer keeping it simple, and
> deduce the modes from ioctls.
> 
> Is that ok for you?  So it'll have (1) get_folio(), (2) supported_ioctls.
> That's all.

I cannot speak for Liam, but for me that looks like the best way forward 
for now.

I'd hope that we could get this all cleaned up at some point -- 
including hugetlb, because it's a pain to have the hugetlb special 
casing all over the place even if it's at some point a legacy thing (we 
all know it will stick around for years to come).

-- 
Cheers

David / dhildenb

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

[Liam R. Howlett]

* Peter Xu <peterx@redhat.com> [251007 15:42]:
> On Tue, Oct 07, 2025 at 02:46:46PM -0400, Liam R. Howlett wrote:
> > * Peter Xu <peterx@redhat.com> [251007 12:47]:
> > 
> > ...
> > 
> > > > > 
> > > > > 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).
> > > 
> > > I didn't send RFC, likely you meant this reply in v1?
> > > 
> > > https://lore.kernel.org/all/0126fa5f-b5aa-4a17-80d6-d428105e45c7@redhat.com/
> > > 
> > >         I agree that another special-purpose file (like implemented by
> > >         guest_memfd) would need that. But if we could get rid of
> > >         "hugetlb"/"shmem" special-casing in userfaultfd, it would be a
> > >         rasonable independent cleanup.
> > > 
> > > Get rid of hugetlbfs is still not my goal as of in this series.
> > 
> > My example picked hugetlbfs because it is the most special of the types
> > of memory we have (so very special).  If the interface works for
> > hugetlbfs, then the rest will use a subset of the features and be happy.
> > 
> > IOW, doing the hard thing first makes what follows easy.  Doing the easy
> > thing first may mean rewriting the easy thing once you arrive at the
> > more difficult part.
> 
> In general I agree, but hugetlbfs is special when it is major-feature
> frozen.  IMHO we shouldn't design an API to suite hugetlbfs, but only
> trying to move it closer to all the rest of file systems as much as
> possible.
> 
> So the generic API should be designed without hugetlbfs involvement.  Then
> if there is guest-memfd / hugetlbfsv2 / ... they should fit into this API.

Since there is no end date for hugetlbfs, we should include it in the
design of modularized memory types.

And if everything is less special, the generic api that can facilitate
hugetlbfs can facilitate everything else.

> 
> > 
> > > 
> > > OTOH, I generalized shmem and removed shmem.h header from userfaultfd, but
> > > that was prior versions when with uffd_copy() and it was rejected.
> > > 
> > > What should I do now to move this series forward?  Could anyone provide a
> > > solid answer?
> > 
> > My understanding is that we need an interface for memory types so they
> > are modularised, with the short term goal of solving the faulting
> > support for guest_memfd and the long term goal of code cleanup, or at
> > least don't make things worse.
> > 
> > I think we all agree on that?
> > 
> > I propose that we need to add the minimum amount of uffd_ops to support
> > guest_memfd's specialness without creating an interface that makes
> > things worse.
> > 
> > It is very difficult to see a reason to pass in two variables (modes and
> > ioctls) to dispatch to the correct function in a struct that could
> 
> The reason is "modes" cannot directly be intepreted into ioctls.  But
> indeed ioctls can be intepreted into supported modes.
> 
> > simply point to the function in the first place.  If we can avoid that,
> > then it would be good.
> > 
> > Looking at the example you pointed to here [1], It appears the minimal
> > viable product would need to implement this:
> > 
> > uffd_ops = {
> >         .get_folio = <>,
> >         .minor_fault = <>,
> >         .atomic_fill_continue = <>,
> 
> These three are fundamentally the same thing.  As explained, if we have
> get_folio() we don't need the rest.  However we still need something to
> describe e.g. shmem supports MISSING mode.
> 

So encode ioctls into this uffd_ops, including a uffd_ops->missing_mode
and set it to NULL for all but shmem.

Without digging in and actually doing the work, I cannot really define
the exact interface needed.  If these are all not necessary, then reduce
what is there or expand it until you don't need new things defined.

> > }
> > 
> > Then shmem and hugetlb can define these and end up calling them in
> > today's spaghetti, but we are free to append more uffd_ops to reduce the
> > spaghetti later.
> > 
> > If using new #defines to clears up translations of features/modes and
> > ioctl codes, then please do that.  These should be removable once the
> > uffd_ops grows to support all necessary calls.
> > 
> > If there are places where you need to consult the modes/ioctls and a
> > translation does not work, then you could add something to uffd_ops that
> > is NULL for guest_memfd and use it to determine if the code path is
> > valid.  But this code should already exist for the other memory types.
> > 
> > What does everyone think?
> > 
> > [1]. https://lore.kernel.org/all/114133f5-0282-463d-9d65-3143aa658806@amazon.com/
> 
> Would it look better to you if I drop uffd_modes_supported, deducing it
> from uffd_ioctls_supported?
> 
> I believe that's what David mentioned very initially here:
> 
> https://lore.kernel.org/r/f1da3505-f17f-4829-80c1-696b1d99057d@redhat.com
> 
> I'd rather go with the two fields, but if we're trying to introduce another
> feature sets almost only for vm_uffd_ops, I'd prefer keeping it simple, and
> deduce the modes from ioctls.
> 
> Is that ok for you?  So it'll have (1) get_folio(), (2) supported_ioctls.
> That's all.

This is still middleware - a translation of flags passed in to figure
out what function to call.  I don't think this is the best path forward
as it means we have to complicate the layer for every user we add while
we are already providing the most flexible return of a folio.

This will end up making things worse, IMO.

Think, for example, we add hugetlbfs_v2 - every place we have
"if (is_hugetlbfs())" will now need an "else if(is_hugetlbfsv2())" to
accommodate something that probably has the same uffd_ops as hugetlbfs
v1.

Why would we do this instead of actually making your uffd_ops a complete
API, or at least a subset of the API that supports guest-memfd?

Thanks,
Liam

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

[Peter Xu]

On Tue, Oct 07, 2025 at 04:25:48PM -0400, Liam R. Howlett wrote:
> > Would it look better to you if I drop uffd_modes_supported, deducing it
> > from uffd_ioctls_supported?
> > 
> > I believe that's what David mentioned very initially here:
> > 
> > https://lore.kernel.org/r/f1da3505-f17f-4829-80c1-696b1d99057d@redhat.com
> > 
> > I'd rather go with the two fields, but if we're trying to introduce another
> > feature sets almost only for vm_uffd_ops, I'd prefer keeping it simple, and
> > deduce the modes from ioctls.
> > 
> > Is that ok for you?  So it'll have (1) get_folio(), (2) supported_ioctls.
> > That's all.
> 
> This is still middleware - a translation of flags passed in to figure
> out what function to call.  I don't think this is the best path forward
> as it means we have to complicate the layer for every user we add while
> we are already providing the most flexible return of a folio.
> 
> This will end up making things worse, IMO.
> 
> Think, for example, we add hugetlbfs_v2 - every place we have
> "if (is_hugetlbfs())" will now need an "else if(is_hugetlbfsv2())" to
> accommodate something that probably has the same uffd_ops as hugetlbfs
> v1.
> 
> Why would we do this instead of actually making your uffd_ops a complete
> API, or at least a subset of the API that supports guest-memfd?

It will be the complete API with (1) and (2) on minor fault.

When one proposes hugetlbfsv2, it should make sure it will work with the
API that only has (1)+(2).  Some uffd paths may need touch up (e.g. on
detecting VMA sizes), but it'll never be "if (is_hugetlbfsv2())".

That's IMHO one of the major purposes of having hugetlbfsv2 after all,
which is to start using the common MM apis, including the one we're going
to introduce here.

Thanks,

-- 
Peter Xu