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) <[email protected]>
> ---
>  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

Reply via email to