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

[Nikita Kalyazin <kalyazin@amazon.com>]

On 20/06/2025 20:03, Peter Xu wrote:
> [based on akpm/mm-new]
> 
> 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.

Hi Peter,

Thanks for posting this.  I confirmed that minor fault handling was 
working for guest_memfd based on this series and looked simple (a draft 
based on mmap support in guest_memfd v7 [1]):

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 5abb6d52a375..6ddc73419724 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -5,6 +5,9 @@
  #include <linux/pagemap.h>
  #include <linux/anon_inodes.h>
  #include <linux/set_memory.h>
+#ifdef CONFIG_USERFAULTFD
+#include <linux/userfaultfd_k.h>
+#endif

  #include "kvm_mm.h"

@@ -396,6 +399,14 @@ static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
  		kvm_gmem_mark_prepared(folio);
  	}

+#ifdef CONFIG_USERFAULTFD
+	if (userfaultfd_minor(vmf->vma)) {
+		folio_unlock(folio);
+		filemap_invalidate_unlock_shared(inode->i_mapping);
+		return handle_userfault(vmf, VM_UFFD_MINOR);
+	}
+#endif
+
  	vmf->page = folio_file_page(folio, vmf->pgoff);

  out_folio:
@@ -410,8 +421,39 @@ static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
  	return ret;
  }

+#ifdef CONFIG_USERFAULTFD
+static int kvm_gmem_uffd_get_folio(struct inode *inode, pgoff_t pgoff,
+				struct folio **foliop)
+{
+	struct folio *folio;
+	folio = kvm_gmem_get_folio(inode, pgoff);
+
+	if (IS_ERR(folio)) {
+		*foliop = NULL;
+		return PTR_ERR(folio);
+	}
+
+	if (!folio_test_uptodate(folio)) {
+		clear_highpage(folio_page(folio, 0));
+		kvm_gmem_mark_prepared(folio);
+	}
+
+	*foliop = folio;
+	return 0;
+}
+
+static const vm_uffd_ops kvm_gmem_uffd_ops = {
+	.uffd_features	= 	VM_UFFD_MINOR,
+	.uffd_ioctls	= 	BIT(_UFFDIO_CONTINUE),
+	.uffd_get_folio	=	kvm_gmem_uffd_get_folio,
+};
+#endif
+
  static const struct vm_operations_struct kvm_gmem_vm_ops = {
  	.fault = kvm_gmem_fault,
+#ifdef CONFIG_USERFAULTFD
+	.userfaultfd_ops = &kvm_gmem_uffd_ops,
+#endif
  };

  static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma)

[1]: https://lore.kernel.org/kvm/20250318161823.4005529-1-tabba@google.com/

> 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.
> 
> Hugetlbfs is still very special that it will only use partial of the
> vm_uffd_ops API, due to similar reason why hugetlb_vm_op_fault() has a
> BUG() and so far hard-coded into core mm.  But this should still be better,
> because at least hugetlbfs is still always involved in feature probing
> (e.g. where it used to not support ZEROPAGE and we have a hard-coded line
> to fail that, and some more).  Meanwhile after this series, shmem will be
> completely converted to the new vm_uffd_ops API; the final vm_uffd_ops for
> shmem looks like this:
> 
> 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,
>          .uffd_copy      =       shmem_mfill_atomic_pte,
> };
> 
> As I mentioned in one of my reply to Nikita, I don't like the current
> interface of uffd_copy(), but this will be the minimum change version of
> such API to support complete extrenal-module-ready userfaultfd.  Here, very
> minimal change will be needed from shmem side to support that.
> 
> Meanwhile, the vm_uffd_ops is also not the only place one will need to
> provide to support userfaultfd.  Normally vm_ops.fault() will also need to
> be updated, but that's a generic function and it'll play together with the
> new vm_uffd_ops to make everything fly.
> 
> 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            |  71 +++++++++++++++++++++
>   include/linux/shmem_fs.h      |  14 -----
>   include/linux/userfaultfd_k.h |  58 ++++-------------
>   mm/hugetlb.c                  |  19 ++++++
>   mm/shmem.c                    |  28 ++++++++-
>   mm/userfaultfd.c              | 115 +++++++++++++++++++++++++---------
>   6 files changed, 217 insertions(+), 88 deletions(-)
> 
> --
> 2.49.0
>