Re: [PATCH 5/6] userfaultfd: decouple fault reason from VMA flags
From: Lorenzo Stoakes (ARM)
Date: Mon Aug 24 2026 - 12:29:30 EST
On Sun, Aug 23, 2026 at 03:17:42PM +0300, Mike Rapoport (Microsoft) wrote:
> Introduce enum uffd_reason to define reasons for user faults rather than
> overload VM_UFFD_* VMA flags for that.
>
> Using a dedicated enum makes the code clearer and decoupling the fault
> reason from VMA flags clears the way for moving the uffd mode bits out
> of VMA namespace.
>
> No functional change.
>
> Assisted-by: copilot:claude-opus-4.6
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> ---
> include/linux/userfaultfd_k.h | 16 ++++++++++++++--
> include/uapi/linux/userfaultfd.h | 6 +++---
> mm/huge_memory.c | 6 +++---
> mm/hugetlb.c | 10 +++++-----
> mm/memory.c | 10 +++++-----
> mm/shmem.c | 4 ++--
> mm/userfaultfd.c | 30 +++++++++++++++---------------
> 7 files changed, 47 insertions(+), 35 deletions(-)
>
> diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> index 45355bdb4ec7..f401623f315d 100644
> --- a/include/linux/userfaultfd_k.h
> +++ b/include/linux/userfaultfd_k.h
> @@ -9,6 +9,18 @@
> #ifndef _LINUX_USERFAULTFD_K_H
> #define _LINUX_USERFAULTFD_K_H
>
> +#include <linux/bits.h>
> +
> +/* Fault reason #PF handler passes to handle_userfault() */
> +enum uf_reason {
> + USERFAULT_MISSING = BIT(0),
> + USERFAULT_MINOR = BIT(1),
> + USERFAULT_RWP = BIT(2),
> + USERFAULT_WP = BIT(3),
> +};
Hmm your commit message says uffd_reason, uf_reason makes me think of the
character Ulf from House of the Dragon. But not uffd. So as per David let's
rename it :)
I'm also not sure if an enum is the right thing for flag values?
Anything that is parameterised by enum uffd_reason that combines flags will
break any switch statement in there and yada yada.
I wonder if better just as #define's + unsigned long or something?
Or you could do (and this leads to nicer stuff later):
enum uffd_reason {
USERFAULT_MISSING_BIT = 0,
USERFAULT_MINOR_BIT = 1,
USERFAULT_RWP_BIT = 2,
USERFAULT_WP_BIT = 3,
};
#define USERFAULT_MISSING BIT(USERFAULT_MISSING_BIT)
etc.
> +#define USERFAULT_ANY (USERFAULT_MISSING | USERFAULT_MINOR | \
> + USERFAULT_RWP | USERFAULT_WP)
> +
> #ifdef CONFIG_USERFAULTFD
>
> #include <linux/userfaultfd.h> /* linux/include/uapi/linux/userfaultfd.h */
> @@ -82,7 +94,7 @@ struct userfaultfd_ctx {
> struct mm_struct *mm;
> };
>
> -extern vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason);
> +vm_fault_t handle_userfault(struct vm_fault *vmf, enum uf_reason reason);
>
> /* VMA userfaultfd operations */
> struct vm_uffd_ops {
> @@ -333,7 +345,7 @@ static inline bool pte_swp_uffd_any(pte_t pte)
>
> /* mm helpers */
> static inline vm_fault_t handle_userfault(struct vm_fault *vmf,
> - unsigned long reason)
> + enum uf_reason reason)
See above re: enum parameterisation.
> {
> return VM_FAULT_SIGBUS;
> }
> diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h
> index cea11aad6b54..ed2c42d427b9 100644
> --- a/include/uapi/linux/userfaultfd.h
> +++ b/include/uapi/linux/userfaultfd.h
> @@ -168,9 +168,9 @@ struct uffd_msg {
>
> /* flags for UFFD_EVENT_PAGEFAULT */
> #define UFFD_PAGEFAULT_FLAG_WRITE (1<<0) /* If this was a write fault */
> -#define UFFD_PAGEFAULT_FLAG_WP (1<<1) /* If reason is VM_UFFD_WP */
> -#define UFFD_PAGEFAULT_FLAG_MINOR (1<<2) /* If reason is VM_UFFD_MINOR */
> -#define UFFD_PAGEFAULT_FLAG_RWP (1<<3) /* If reason is VM_UFFD_RWP */
> +#define UFFD_PAGEFAULT_FLAG_WP (1<<1) /* If reason is uffd-wp */
> +#define UFFD_PAGEFAULT_FLAG_MINOR (1<<2) /* If reason is uffd-minor */
> +#define UFFD_PAGEFAULT_FLAG_RWP (1<<3) /* If reason is uffd-rwp */
Is it worth retaining the same bit indexes as the reasons?
Reasons:
Bit number
MINOR 0
RWP 1
WP 2
Page fault flags:
Bit number
MINOR 2
RWP 3
WP 1
See below for some actual practical justification...
>
> struct uffdio_api {
> /* userland asks for an API number and the features to enable */
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index ced400f72d43..46d8497ce90e 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1410,7 +1410,7 @@ static vm_fault_t __do_huge_pmd_anonymous_page(struct vm_fault *vmf)
> spin_unlock(vmf->ptl);
> folio_put(folio);
> pte_free(vma->vm_mm, pgtable);
> - ret = handle_userfault(vmf, VM_UFFD_MISSING);
> + ret = handle_userfault(vmf, USERFAULT_MISSING);
> VM_BUG_ON(ret & VM_FAULT_FALLBACK);
> return ret;
> }
> @@ -1556,7 +1556,7 @@ vm_fault_t do_huge_pmd_anonymous_page(struct vm_fault *vmf)
> } else if (userfaultfd_missing(vma)) {
> spin_unlock(vmf->ptl);
> pte_free(vma->vm_mm, pgtable);
> - ret = handle_userfault(vmf, VM_UFFD_MISSING);
> + ret = handle_userfault(vmf, USERFAULT_MISSING);
> VM_BUG_ON(ret & VM_FAULT_FALLBACK);
> } else {
> set_huge_zero_folio(pgtable, vma->vm_mm, vma,
> @@ -2252,7 +2252,7 @@ vm_fault_t do_huge_pmd_uffd_rwp(struct vm_fault *vmf)
> pmd_t pmd;
>
> if (!userfaultfd_rwp_async(vma))
> - return handle_userfault(vmf, VM_UFFD_RWP);
> + return handle_userfault(vmf, USERFAULT_RWP);
>
> vmf->ptl = pmd_lock(vma->vm_mm, vmf->pmd);
> if (unlikely(!pmd_same(pmdp_get(vmf->pmd), vmf->orig_pmd))) {
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 73d65644be13..5e2ed80c1938 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -5728,7 +5728,7 @@ int hugetlb_add_to_page_cache(struct folio *folio, struct address_space *mapping
>
> static inline vm_fault_t hugetlb_handle_userfault(struct vm_fault *vmf,
> struct address_space *mapping,
> - unsigned long reason)
> + enum uf_reason reason)
> {
> u32 hash;
>
> @@ -5821,7 +5821,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
> }
>
> return hugetlb_handle_userfault(vmf, mapping,
> - VM_UFFD_MISSING);
> + USERFAULT_MISSING);
> }
>
> if (!(vma->vm_flags & VM_MAYSHARE)) {
> @@ -5897,7 +5897,7 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
> goto out;
> }
> return hugetlb_handle_userfault(vmf, mapping,
> - VM_UFFD_MINOR);
> + USERFAULT_MINOR);
> }
> }
>
> @@ -6120,7 +6120,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
>
> /* Sync: drop hugetlb locks before blocking in handle_userfault() */
> if (!userfaultfd_rwp_async(vma))
> - return hugetlb_handle_userfault(&vmf, mapping, VM_UFFD_RWP);
> + return hugetlb_handle_userfault(&vmf, mapping, USERFAULT_RWP);
>
> ptl = huge_pte_lock(h, mm, vmf.pte);
> pte = huge_ptep_get(mm, vmf.address, vmf.pte);
> @@ -6177,7 +6177,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> spin_unlock(vmf.ptl);
> hugetlb_vma_unlock_read(vma);
> mutex_unlock(&hugetlb_fault_mutex_table[hash]);
> - return handle_userfault(&vmf, VM_UFFD_WP);
> + return handle_userfault(&vmf, USERFAULT_WP);
> }
>
> vmf.orig_pte = huge_pte_clear_uffd(vmf.orig_pte);
> diff --git a/mm/memory.c b/mm/memory.c
> index c54943302553..1a9b41704b0c 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4389,7 +4389,7 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf)
> if (userfaultfd_pte_wp(vma, ptep_get(vmf->pte))) {
> if (!userfaultfd_wp_async(vma)) {
> pte_unmap_unlock(vmf->pte, vmf->ptl);
> - return handle_userfault(vmf, VM_UFFD_WP);
> + return handle_userfault(vmf, USERFAULT_WP);
> }
>
> /*
> @@ -5463,7 +5463,7 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
> /* Deliver the page fault to userland, check inside PT lock */
> if (userfaultfd_missing(vma)) {
> pte_unmap_unlock(vmf->pte, vmf->ptl);
> - return handle_userfault(vmf, VM_UFFD_MISSING);
> + return handle_userfault(vmf, USERFAULT_MISSING);
> }
> if (vmf_orig_pte_uffd_wp(vmf))
> entry = pte_mkuffd(entry);
> @@ -5514,7 +5514,7 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
> if (userfaultfd_missing(vma)) {
> pte_unmap_unlock(vmf->pte, vmf->ptl);
> folio_put(folio);
> - return handle_userfault(vmf, VM_UFFD_MISSING);
> + return handle_userfault(vmf, USERFAULT_MISSING);
> }
> map_anon_folio_pte_pf(folio, vmf->pte, vma, addr,
> vmf_orig_pte_uffd_wp(vmf));
> @@ -6263,7 +6263,7 @@ static vm_fault_t do_uffd_rwp(struct vm_fault *vmf)
> if (!userfaultfd_rwp_async(vmf->vma)) {
> /* Sync mode: unmap PTE and deliver to userfaultfd handler */
> pte_unmap(vmf->pte);
> - return handle_userfault(vmf, VM_UFFD_RWP);
> + return handle_userfault(vmf, USERFAULT_RWP);
> }
>
> spin_lock(vmf->ptl);
> @@ -6398,7 +6398,7 @@ static inline vm_fault_t wp_huge_pmd(struct vm_fault *vmf)
> userfaultfd_huge_pmd_wp(vma, vmf->orig_pmd)) {
> if (userfaultfd_wp_async(vmf->vma))
> goto split;
> - return handle_userfault(vmf, VM_UFFD_WP);
> + return handle_userfault(vmf, USERFAULT_WP);
> }
> return do_huge_pmd_wp_page(vmf);
> }
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 599665a3d6e7..2138a4e6b549 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2453,7 +2453,7 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index,
> if (folio && vma && userfaultfd_minor(vma)) {
> if (!xa_is_value(folio))
> folio_put(folio);
> - *fault_type = handle_userfault(vmf, VM_UFFD_MINOR);
> + *fault_type = handle_userfault(vmf, USERFAULT_MINOR);
> return 0;
> }
>
> @@ -2502,7 +2502,7 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index,
> */
>
> if (vma && userfaultfd_missing(vma)) {
> - *fault_type = handle_userfault(vmf, VM_UFFD_MISSING);
> + *fault_type = handle_userfault(vmf, USERFAULT_MISSING);
> return 0;
> }
>
> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index 119304547230..83587d34b189 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -2607,7 +2607,7 @@ static inline void msg_init(struct uffd_msg *msg)
> static inline struct uffd_msg userfault_msg(unsigned long address,
> unsigned long real_address,
> unsigned int flags,
> - unsigned long reason,
> + enum uf_reason reason,
> unsigned int features)
> {
> struct uffd_msg msg;
> @@ -2629,11 +2629,11 @@ static inline struct uffd_msg userfault_msg(unsigned long address,
> */
> if (flags & FAULT_FLAG_WRITE)
> msg.arg.pagefault.flags |= UFFD_PAGEFAULT_FLAG_WRITE;
> - if (reason & VM_UFFD_WP)
> + if (reason & USERFAULT_WP)
> msg.arg.pagefault.flags |= UFFD_PAGEFAULT_FLAG_WP;
> - if (reason & VM_UFFD_RWP)
> + if (reason & USERFAULT_RWP)
> msg.arg.pagefault.flags |= UFFD_PAGEFAULT_FLAG_RWP;
> - if (reason & VM_UFFD_MINOR)
> + if (reason & USERFAULT_MINOR)
> msg.arg.pagefault.flags |= UFFD_PAGEFAULT_FLAG_MINOR;
With matching flags and unsigned long you could do
msg.arg.pagefault.flags |= reason;
I think?
> if (features & UFFD_FEATURE_THREAD_ID)
> msg.arg.pagefault.feat.ptid = task_pid_vnr(current);
> @@ -2647,7 +2647,7 @@ static inline struct uffd_msg userfault_msg(unsigned long address,
> */
> static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx,
> struct vm_fault *vmf,
> - unsigned long reason)
> + enum uf_reason reason)
> {
> struct vm_area_struct *vma = vmf->vma;
> pte_t *ptep, pte;
> @@ -2684,13 +2684,13 @@ static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx,
> * If VMA has UFFD WP faults enabled and WP fault, wait for userspace to
> * resolve the fault.
> */
> - if (!huge_pte_write(pte) && (reason & VM_UFFD_WP))
> + if (!huge_pte_write(pte) && (reason & USERFAULT_WP))
> return true;
> /*
> * PTE is still RW-protected (protnone with uffd bit), wait for
> * resolution. Plain PROT_NONE without the marker is not an RWP fault.
> */
> - if (pte_protnone(pte) && huge_pte_uffd(pte) && (reason & VM_UFFD_RWP))
> + if (pte_protnone(pte) && huge_pte_uffd(pte) && (reason & USERFAULT_RWP))
> return true;
>
> return false;
> @@ -2698,7 +2698,7 @@ static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx,
> #else
> static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx,
> struct vm_fault *vmf,
> - unsigned long reason)
> + enum uf_reason reason)
> {
> /* Should never get here. */
> VM_WARN_ON_ONCE(1);
> @@ -2715,7 +2715,7 @@ static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx,
> */
> static inline bool userfaultfd_must_wait(struct userfaultfd_ctx *ctx,
> struct vm_fault *vmf,
> - unsigned long reason)
> + enum uf_reason reason)
> {
> struct mm_struct *mm = ctx->mm;
> unsigned long address = vmf->address;
> @@ -2753,10 +2753,10 @@ static inline bool userfaultfd_must_wait(struct userfaultfd_ctx *ctx,
> return false;
>
> if (pmd_trans_huge(_pmd)) {
> - if (!pmd_write(_pmd) && (reason & VM_UFFD_WP))
> + if (!pmd_write(_pmd) && (reason & USERFAULT_WP))
> return true;
> if (pmd_protnone(_pmd) && pmd_uffd(_pmd) &&
> - (reason & VM_UFFD_RWP))
> + (reason & USERFAULT_RWP))
> return true;
> return false;
> }
> @@ -2793,14 +2793,14 @@ static inline bool userfaultfd_must_wait(struct userfaultfd_ctx *ctx,
> * If VMA has UFFD WP faults enabled and WP fault, wait for userspace to
> * resolve the fault.
> */
> - if (!pte_write(ptent) && (reason & VM_UFFD_WP))
> + if (!pte_write(ptent) && (reason & USERFAULT_WP))
I wonder if you could actually
You do this quite a lot and they read a bit horribly with the && and & on the
same sight-line. With the changes to the enum proposed above you could do:
if (!pte_write(ptent) && test_bit(reason, USERFAULT_WP_BIT))
> goto out;
> /*
> * PTE is still RW-protected (protnone with uffd bit), wait for
> * userspace to resolve. Plain PROT_NONE without the marker is not
> * an RWP fault.
> */
> - if (pte_protnone(ptent) && pte_uffd(ptent) && (reason & VM_UFFD_RWP))
> + if (pte_protnone(ptent) && pte_uffd(ptent) && (reason & USERFAULT_RWP))
> goto out;
>
> ret = false;
> @@ -2835,7 +2835,7 @@ static inline unsigned int userfaultfd_get_blocking_state(unsigned int flags)
> * fatal_signal_pending()s, and the mmap_lock must be released before
> * returning it.
> */
> -vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason)
> +vm_fault_t handle_userfault(struct vm_fault *vmf, enum uf_reason reason)
Hmm what was the 'reason' here before? The flags? Maybe more reason (no pun
intended) to keep the values the same?
> {
> struct vm_area_struct *vma = vmf->vma;
> struct mm_struct *mm = vma->vm_mm;
> @@ -2861,7 +2861,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason)
> VM_WARN_ON_ONCE(ctx->mm != mm);
>
> /* Any unrecognized flag is a bug. */
> - VM_WARN_ON_ONCE(reason & ~__VM_UFFD_FLAGS);
> + VM_WARN_ON_ONCE(reason & ~USERFAULT_ANY);
> /* 0 or > 1 flags set is a bug; we expect exactly 1. */
> VM_WARN_ON_ONCE(!reason || (reason & (reason - 1)));
>
>
> --
> 2.53.0
>
--
Cheers, Lorenzo