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