Re: [PATCH v2 05/14] mm: add MM_CP_UFFD_RWP change_protection() flag

[Kiryl Shutsemau <kas@kernel.org>]

On Tue, May 12, 2026 at 07:45:07PM +0300, Mike Rapoport wrote:
> On Fri, May 08, 2026 at 04:55:17PM +0100, Kiryl Shutsemau (Meta) wrote:
> > Preparatory patch. Add the change_protection() primitive that
> > userfaultfd RWP will use.
> > 
> > An RWP-protected PTE is PAGE_NONE with the uffd PTE bit set. The
> > PROT_NONE half makes the CPU fault on any access; the uffd bit
> > distinguishes an RWP fault from a plain mprotect(PROT_NONE) or NUMA
> > hinting fault. MM_CP_UFFD_WP and MM_CP_UFFD_RWP share the same PTE
> > bit, so the two cannot be used together on the same range.
> > 
> > Two new change_protection() flags:
> > 
> >   MM_CP_UFFD_RWP            install PAGE_NONE and set the uffd bit
> >   MM_CP_UFFD_RWP_RESOLVE    restore vma->vm_page_prot, clear the uffd bit
> > 
> > Both are wired through change_pte_range(), change_huge_pmd(), and
> > hugetlb_change_protection() so anon, shmem, THP, and hugetlb all
> > share the same semantics.
> > 
> > Signed-off-by: Kiryl Shutsemau <kas@kernel.org>
> > Assisted-by: Claude:claude-opus-4-6
> > ---
> >  include/linux/mm.h            |  5 +++++
> >  include/linux/userfaultfd_k.h |  1 -
> >  mm/huge_memory.c              | 20 ++++++++++++------
> >  mm/hugetlb.c                  | 25 ++++++++++++++++------
> >  mm/mprotect.c                 | 40 +++++++++++++++++++++++++++++------
> >  5 files changed, 71 insertions(+), 20 deletions(-)
> > 
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 3f53d1e978c0..2b65416bb760 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -3291,6 +3291,11 @@ int get_cmdline(struct task_struct *task, char *buffer, int buflen);
> >  #define  MM_CP_UFFD_WP_RESOLVE             (1UL << 3) /* Resolve wp */
> >  #define  MM_CP_UFFD_WP_ALL                 (MM_CP_UFFD_WP | \
> >  					    MM_CP_UFFD_WP_RESOLVE)
> > +/* Whether this change is for uffd RWP */
> > +#define  MM_CP_UFFD_RWP                    (1UL << 4) /* do rwp */
> > +#define  MM_CP_UFFD_RWP_RESOLVE            (1UL << 5) /* Resolve rwp */
> 
> Nit: any reason except copy/paset to use different case in "do rwp" and
> "Resolve rwp"? ;-)

copy/paste it is. Will use "resolve rwp"

> > +#define  MM_CP_UFFD_RWP_ALL                (MM_CP_UFFD_RWP | \
> > +					    MM_CP_UFFD_RWP_RESOLVE)
> >  
> >  bool can_change_pte_writable(struct vm_area_struct *vma, unsigned long addr,
> >  			     pte_t pte);
> > diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> > index fcf308dba311..3725e61a7041 100644
> > --- a/include/linux/userfaultfd_k.h
> > +++ b/include/linux/userfaultfd_k.h
> > @@ -397,7 +397,6 @@ static inline bool userfaultfd_huge_pmd_wp(struct vm_area_struct *vma,
> >  	return false;
> >  }
> >  
> > -
> >  static inline bool userfaultfd_armed(struct vm_area_struct *vma)
> >  {
> >  	return false;
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index d88fcccd386d..2537dca63c6c 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -2665,6 +2665,8 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> >  	spinlock_t *ptl;
> >  	pmd_t oldpmd, entry;
> >  	bool prot_numa = cp_flags & MM_CP_PROT_NUMA;
> > +	bool uffd_rwp = cp_flags & MM_CP_UFFD_RWP;
> > +	bool uffd_rwp_resolve = cp_flags & MM_CP_UFFD_RWP_RESOLVE;
> >  	bool uffd_wp = cp_flags & MM_CP_UFFD_WP;
> >  	bool uffd_wp_resolve = cp_flags & MM_CP_UFFD_WP_RESOLVE;
> 
> It looks like uffd_wp* are always ORed with uffd_rwp, we could fold this to
> e.g.
> 
> 	bool uffd_prot = cp_flags & (MM_CP_UFFD_WP | MM_CP_UFFD_RWP);

Makes sense.

> >  	int ret = 1;
> > @@ -2679,11 +2681,18 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> >  		return 0;
> >  
> >  	if (thp_migration_supported() && pmd_is_valid_softleaf(*pmd)) {
> > -		change_non_present_huge_pmd(mm, addr, pmd, uffd_wp,
> > -					    uffd_wp_resolve);
> > +		change_non_present_huge_pmd(mm, addr, pmd,
> > +					    uffd_wp || uffd_rwp,
> > +					    uffd_wp_resolve || uffd_rwp_resolve);
> >  		goto unlock;
> >  	}
> >  
> > +	/* Already in the desired state */
> > +	if (prot_numa && pmd_protnone(*pmd))
> > +		goto unlock;
> > +	if (uffd_rwp && pmd_protnone(*pmd) && pmd_uffd(*pmd))
> > +		goto unlock;
> > +
> >  	if (prot_numa) {
> >  
> >  		/*
> > @@ -2694,9 +2703,6 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> >  		if (is_huge_zero_pmd(*pmd))
> >  			goto unlock;
> >  
> > -		if (pmd_protnone(*pmd))
> > -			goto unlock;
> > -
> >  		if (!folio_can_map_prot_numa(pmd_folio(*pmd), vma,
> >  					     vma_is_single_threaded_private(vma)))
> >  			goto unlock;
> > @@ -2725,9 +2731,9 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> >  	oldpmd = pmdp_invalidate_ad(vma, addr, pmd);
> >  
> >  	entry = pmd_modify(oldpmd, newprot);
> > -	if (uffd_wp)
> > +	if (uffd_wp || uffd_rwp)
> >  		entry = pmd_mkuffd(entry);
> > -	else if (uffd_wp_resolve)
> > +	else if (uffd_wp_resolve || uffd_rwp_resolve)
> >  		/*
> >  		 * Leave the write bit to be handled by PF interrupt
> >  		 * handler, then things like COW could be properly
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 61cda9992043..63f6b19418b9 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -6434,6 +6436,11 @@ long hugetlb_change_protection(struct vm_area_struct *vma,
> >  
> >  		ptep = hugetlb_walk(vma, address, psize);
> >  		if (!ptep) {
> > +			/*
> > +			 * uffd_wp installs a pte marker on the unpopulated
> > +			 * entry; RWP does not install markers so the
> 
> Nit:                              uffd_rwp

Ack.

> 
> > +			 * allocation is unnecessary for it.
> > +			 */
> >  			if (!uffd_wp) {
> >  				address |= last_addr_mask;
> >  				continue;
> > diff --git a/mm/mprotect.c b/mm/mprotect.c
> > index 8340c8b228c6..23e71f68cf7a 100644
> > --- a/mm/mprotect.c
> > +++ b/mm/mprotect.c
> > @@ -216,6 +216,8 @@ static long change_softleaf_pte(struct vm_area_struct *vma,
> >  {
> >  	const bool uffd_wp = cp_flags & MM_CP_UFFD_WP;
> >  	const bool uffd_wp_resolve = cp_flags & MM_CP_UFFD_WP_RESOLVE;
> > +	const bool uffd_rwp = cp_flags & MM_CP_UFFD_RWP;
> > +	const bool uffd_rwp_resolve = cp_flags & MM_CP_UFFD_RWP_RESOLVE;
> 
> And here a single pair of bools should be enough I think.

Yep.

> 
> >  	softleaf_t entry = softleaf_from_pte(oldpte);
> >  	pte_t newpte;
> >  
> > @@ -256,7 +258,7 @@ static long change_softleaf_pte(struct vm_area_struct *vma,
> >  		 * to unprotect it, drop it; the next page
> >  		 * fault will trigger without uffd trapping.
> >  		 */
> > -		if (uffd_wp_resolve) {
> > +		if (uffd_wp_resolve || uffd_rwp_resolve) {
> >  			pte_clear(vma->vm_mm, addr, pte);
> >  			return 1;
> >  		}
> > @@ -265,9 +267,9 @@ static long change_softleaf_pte(struct vm_area_struct *vma,
> >  		newpte = oldpte;
> >  	}
> >  
> > -	if (uffd_wp)
> > +	if (uffd_wp || uffd_rwp)
> >  		newpte = pte_swp_mkuffd(newpte);
> > -	else if (uffd_wp_resolve)
> > +	else if (uffd_wp_resolve || uffd_rwp_resolve)
> >  		newpte = pte_swp_clear_uffd(newpte);
> >  
> >  	if (!pte_same(oldpte, newpte)) {
> 
> -- 
> Sincerely yours,
> Mike.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov