Re: [PATCH 3/6] userfaultfd: use userfaultfd_*() helpers instead of open coded flag tests

[Mike Rapoport]

On Mon, Aug 24, 2026 at 04:10:51PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Sun, Aug 23, 2026 at 03:17:40PM +0300, Mike Rapoport (Microsoft) wrote:
> > Move userfaultfd_{missing,wp,minor,rwp}() and userfaultfd_protected()
> > ahead of uffd_disable_huge_pmd_share() and uffd_disable_fault_around()
> > and make the latter two use the helpers rather than open coded VMA flag
> > masks.
> >
> > Convert open coded VMA flag test in mfill_get_vma() to userfaultfd_wp()
> > as well.
> 
> It'd be better to do the moves and the reworks separately. We don't have a limit
> on patch count :)
> 
> >
> > With every user of the per-VMA uffd modes going through the helpers,
> > their underlying representation can be changed in the next step.
> >
> > No functional change.
> 
> There is a functional change, or at least seems to be, see below.
> 
> >
> > Assisted-by: copilot:claude-opus-5
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> > ---
> >  include/linux/userfaultfd_k.h | 70 +++++++++++++++++++++----------------------
> >  mm/userfaultfd.c              |  2 +-
> >  2 files changed, 35 insertions(+), 37 deletions(-)
> >
> > diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> > index 3396d270b159..d8262e3dc134 100644
> > --- a/include/linux/userfaultfd_k.h
> > +++ b/include/linux/userfaultfd_k.h
> > @@ -168,42 +168,6 @@ static inline bool is_mergeable_vm_userfaultfd_ctx(struct vm_area_struct *vma,
> >  	return vma->vm_userfaultfd_ctx.ctx == vm_ctx.ctx;
> >  }
> >
> > -/*
> > - * Never enable huge pmd sharing on some uffd registered vmas:
> > - *
> > - * - VM_UFFD_WP and VM_UFFD_RWP VMAs, because the write protect / access
> > - *   tracking information is per pgtable entry.
> > - *
> > - * - VM_UFFD_MINOR VMAs, because otherwise we would never get minor faults for
> > - *   VMAs which share huge pmds. (If you have two mappings to the same
> > - *   underlying pages, and fault in the non-UFFD-registered one with a write,
> > - *   with huge pmd sharing this would *also* setup the second UFFD-registered
> > - *   mapping, and we'd not get minor faults.)
> > - */
> > -static inline bool uffd_disable_huge_pmd_share(struct vm_area_struct *vma)
> > -{
> > -	return vma_test_any_mask(vma,
> > -		mk_vma_flags_from_masks(VMA_UFFD_WP, VMA_UFFD_RWP,
> > -					VMA_UFFD_MINOR));
> > -}
> > -
> > -/*
> > - * Don't do fault around for WP, RWP or MINOR registered uffd range.  For
> > - * MINOR registered range, fault around will be a total disaster and ptes can
> > - * be installed without notifications; for WP it should mostly be fine as long
> > - * as the fault around checks for pte_none() before the installation, however
> > - * to be super safe we just forbid it; for RWP, pre-faulted neighbours would
> > - * be indistinguishable from accessed pages in PAGEMAP_SCAN (PAGE_IS_ACCESSED)
> > - * and pollute the tracked working set, so each page must be populated by its
> > - * own fault.
> > - */
> > -static inline bool uffd_disable_fault_around(struct vm_area_struct *vma)
> > -{
> > -	return vma_test_any_mask(vma,
> > -		mk_vma_flags_from_masks(VMA_UFFD_WP, VMA_UFFD_RWP,
> > -					VMA_UFFD_MINOR));
> > -}
> > -
> >  static inline bool userfaultfd_missing(const struct vm_area_struct *vma)
> >  {
> >  	return vma_test_any_mask(vma, VMA_UFFD_MISSING);
> > @@ -235,6 +199,40 @@ static inline bool userfaultfd_protected(const struct vm_area_struct *vma)
> >  	return userfaultfd_wp(vma) || userfaultfd_rwp(vma);
> >  }
> >
> > +/*
> > + * Never enable huge pmd sharing on some uffd registered vmas:
> > + *
> > + * - uffd-WP and uffd-RWP VMAs, because the write protect / access tracking
> > + *   information is per pgtable entry.
> > + *
> > + * - uffd-MINOR VMAs, because otherwise we would never get minor faults for
> > + *   VMAs which share huge pmds. (If you have two mappings to the same
> > + *   underlying pages, and fault in the non-UFFD-registered one with a write,
> > + *   with huge pmd sharing this would *also* setup the second UFFD-registered
> > + *   mapping, and we'd not get minor faults.)
> > + */
> > +static inline bool uffd_disable_huge_pmd_share(struct vm_area_struct *vma)
> > +{
> > +	return userfaultfd_minor(vma) || userfaultfd_wp(vma) ||
> > +	       userfaultfd_rwp(vma);
> > +}
> > +
> > +/*
> > + * Don't do fault around for WP, RWP or MINOR registered uffd range.  For
> > + * MINOR registered range, fault around will be a total disaster and ptes can
> > + * be installed without notifications; for WP it should mostly be fine as long
> > + * as the fault around checks for pte_none() before the installation, however
> > + * to be super safe we just forbid it; for RWP, pre-faulted neighbours would
> > + * be indistinguishable from accessed pages in PAGEMAP_SCAN (PAGE_IS_ACCESSED)
> > + * and pollute the tracked working set, so each page must be populated by its
> > + * own fault.
> > + */
> > +static inline bool uffd_disable_fault_around(struct vm_area_struct *vma)
> > +{
> > +	return userfaultfd_minor(vma) || userfaultfd_wp(vma) ||
> > +	       userfaultfd_rwp(vma);
> 
> This is changing the logic.
> 
> Before we were testing only the flags, now we have:
> 
> static inline bool userfaultfd_rwp(const struct vm_area_struct *vma)
> {
> 	/*
> 	 * Callers gate PAGE_NONE usage on this; PAGE_NONE is a BUILD_BUG()
> 	 * without CONFIG_ARCH_HAS_PTE_PROTNONE, so fold to false.
> 	 */
> 	if (!IS_ENABLED(CONFIG_ARCH_HAS_PTE_PROTNONE))
> 		return false;
> 	return vma_test_single_mask(vma, VMA_UFFD_RWP);
> }
> 
> I.e. adding in a CONFIG_ARCH_HAS_PTE_PROTNONE check.

Without CONFIG_ARCH_HAS_PTE_PROTNONE VMA_UFFD_RWP is hardwired to VM_NONE
so it's functionally the same ;-)
 
> BTW side-note these:
> 
> static inline bool userfaultfd_missing(const struct vm_area_struct *vma)
> {
> 	return vma_test_any_mask(vma, VMA_UFFD_MISSING);
> }
> 
> static inline bool userfaultfd_wp(const struct vm_area_struct *vma)
> {
> 	return vma_test_any_mask(vma, VMA_UFFD_WP);
> }
> 
> static inline bool userfaultfd_minor(const struct vm_area_struct *vma)
> {
> 	return vma_test_any_mask(vma, VMA_UFFD_MINOR);
> }
> 
> Should all use vma_test_single_mask() really :)

These are changed anyway in a later patch.
 
> --
> Cheers, Lorenzo

-- 
Sincerely yours,
Mike.