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

"Lorenzo Stoakes (ARM)" <[email protected]>
Newsgroups org.kernel.vger.linux-doc,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-trace-kernel,org.kvack.linux-mm
Message-ID <aoxdWWMEeQbcG5Pq@gremlin>
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) <[email protected]>
> ---
>  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.

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


> +}
> +
>  static inline bool userfaultfd_pte_wp(struct vm_area_struct *vma,
>  				      pte_t pte)
>  {
> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index 74f04c323c50..32003aa04943 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -261,7 +261,7 @@ static int mfill_get_vma(struct mfill_state *state)
>  	 * 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 ((flags & MFILL_ATOMIC_WP) && !userfaultfd_wp(dst_vma))
>  		goto out_unlock;
>
>  	if (is_vm_hugetlb_page(dst_vma))
>
> --
> 2.53.0
>

--
Cheers, Lorenzo
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.