Re: [PATCH v3 1/9] mm: fix eager write-bit restore for write-notified shared mappings

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Thu, Nov 12, 2026 at 03:20:30PM +0100, Carlos López wrote:
> change_protection() applies vma->vm_page_prot, which lacks the write bit
> for CoW and write-notified mappings. Callers set
> MM_CP_TRY_CHANGE_WRITABLE when vma_wants_manual_pte_write_upgrade()
> returns true, so that entries that can be written without a fault, such
> as dirty PTEs in a shared mapping, are made writable right away.
>
> For shared mappings, that function passes vma->vm_page_prot to
> vma_wants_writenotify(). Once writenotify is enabled, this protection
> lacks the write bit, while the vma_pgprot_modify() result it is compared
> against adds it back. The two never match, so vma_wants_writenotify()
> returns false before checking for soft-dirty, uffd-wp and writeback.
>
> As a result, write-notified shmem mappings never get
> MM_CP_TRY_CHANGE_WRITABLE. For example, commit 931298e103c2
> ("mm/userfaultfd: rely on vma->vm_page_prot in uffd_wp_range()") made
> uffd-wp unprotect set the flag so that dirty shmem PTEs become writable
> again, but this never takes effect. Unprotecting N dirty shmem pages
> still leads to N faults that only set the write bit.
>
> This dates back to the commit in the Fixes tag, which moved the
> equivalent check in mprotect_fixup() before vma_set_page_prot().
>
> Fix this by passing the protection recomputed from the VMA flags, as
> vma_set_page_prot() does.
>
> Note: this only affects performance, as the next write fault restores
> the write bit anyway.
>
> Fixes: 64e455079e1b ("mm: softdirty: enable write notifications on VMAs after VM_SOFTDIRTY cleared")
> Signed-off-by: Carlos López <clopez@suse.de>
> ---
>  mm/vma.h | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/mm/vma.h b/mm/vma.h
> index 3fc3b47a954b..e83700e248e4 100644
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -589,6 +589,14 @@ int split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
>  	      unsigned long addr, int new_below);
>  #endif
>
> +#ifdef CONFIG_MMU
> +static inline pgprot_t vma_pgprot_modify(pgprot_t oldprot, vma_flags_t vma_flags)
> +{
> +	const pgprot_t prot = vma_flags_to_page_prot(vma_flags);
> +
> +	return pgprot_modify(oldprot, prot);
> +}
> +
>  static inline bool vma_wants_manual_pte_write_upgrade(struct vm_area_struct *vma)
>  {
>  	/*
> @@ -596,19 +604,16 @@ static inline bool vma_wants_manual_pte_write_upgrade(struct vm_area_struct *vma
>  	 * if we can't do that automatically for all PTEs in a mapping. For
>  	 * private mappings, that's always the case when we have write
>  	 * permissions as we properly have to handle COW.
> +	 *
> +	 * vma->vm_page_prot lacks the write bit if writenotify is already
> +	 * enabled, so pass it with the bits restored, as vma_set_page_prot()
> +	 * does.

We don't write code in comments.

>  	 */
>  	if (vma->vm_flags & VM_SHARED)
> -		return vma_wants_writenotify(vma, vma->vm_page_prot);
> +		return vma_wants_writenotify(vma,
> +		       vma_pgprot_modify(vma->vm_page_prot, vma->flags));

You mention vma_set_page_prot() but then implement it differently from there and
make no effort to abstract it?

And so both callers now do the same thing, and both open code it... you should
abstract that.

>  	return !!(vma->vm_flags & VM_WRITE);
>  }
> -
> -#ifdef CONFIG_MMU
> -static inline pgprot_t vma_pgprot_modify(pgprot_t oldprot, vma_flags_t vma_flags)
> -{
> -	const pgprot_t prot = vma_flags_to_page_prot(vma_flags);
> -
> -	return pgprot_modify(oldprot, prot);
> -}
>  #endif
>
>  static inline struct vm_area_struct *vma_prev_limit(struct vma_iterator *vmi,
> --
> 2.51.0
>

--
Cheers, Lorenzo