Re: [PATCH v2] mm/ksm: keep soft-dirty and uffd-wp when merging a page

["David Hildenbrand (Arm)" <david@kernel.org>]

On 10/20/26 07:55, Lance Yang wrote:
> 
> On Mon, Oct 19, 2026 at 03:24:11PM +0200, David Hildenbrand (Arm) wrote:
>> On 10/19/26 03:09, Samarth Narang wrote:
>>> replace_page() maps the KSM page with a PTE built from scratch, so the
>>> soft-dirty and uffd bits of the PTE it replaces are lost. Due to this, a
>>> write before the merge no longer shows as soft-dirty, the next write to
>>> a uffd-wp page raises no write-protect fault, and because the page is
>>> mapped with the VMA's normal permissions, on a uffd-rwp VMA the next
>>> access raises no RWP fault.
>>>
>>> Copy the soft-dirty and uffd bits from the original PTE to the new PTE,
>>> and restore PAGE_NONE for RWP VMAs, as try_to_map_unused_to_zeropage()
>>> does.
>>>
>>> Fixes: 0f8975ec4db2 ("mm: soft-dirty bits for user memory changes tracking")
>>> Fixes: 63b2d4174c4a ("userfaultfd: wp: add the writeprotect API to userfaultfd ioctl")
>>> Fixes: 2d427bb01639 ("mm/userfaultfd: add RWP fault delivery and expose UFFDIO_REGISTER_MODE_RWP")
>>
>> I would actually split that up into separate patches.
>>
>> Over-indicating soft-dirty (as raised by Sashiko) is in general not a problem.
> 
> Right.
> 
>> Losing WP bits is.
>>
>> UFFD, the gift that keeps on giving.
> 
> Haha, yeah, I stared at the code a bit. Say we arm RWP on an existing KSM
> mapping, then do MADV_UNMERGEABLE. We're just replacing the shared page
> with a private copy. That shouldn't count as an access to the mapping ...
> no?
> 
> But the protnone check sends us to do_uffd_rwp() before the
> FAULT_FLAG_UNSHARE handling. No FAULT_FLAG_ALLOW_RETRY here, so sync RWP
> gives us VM_FAULT_SIGBUS. Then break_ksm() does this:
> 
> static int break_ksm(struct vm_area_struct *vma, unsigned long addr,
> 		unsigned long end, bool lock_vma)
> {
> 	vm_fault_t ret = 0;
> ...
> 	do {
> ...
> 		ret = handle_mm_fault(vma, addr,
> 				      FAULT_FLAG_UNSHARE | FAULT_FLAG_REMOTE,
> 				      NULL);
> 	} while (!(ret & (VM_FAULT_SIGBUS | VM_FAULT_SIGSEGV | VM_FAULT_OOM)));
> ...
> 	return (ret & VM_FAULT_OOM) ? -ENOMEM : 0;
> }
> 
> So we report success with the page still merged. Async RWP does unshare
> it, but only after clearing the uffd bit.
> 
> I'd let FAULT_FLAG_UNSHARE skip do_uffd_rwp(). We'd still need to carry
> PAGE_NONE over in wp_page_copy(), though. That PTE is rebuilt from
> vma->vm_page_prot, so the uffd bit alone won't stop reads.
> 
>> How to deal RWP properly is a good question (see Sashiko review). I feel like
>> some more stuff might be missing there to handle this correctly.
> 
> Tested, this is what I had in mind:
> 
> ---8<---
> diff --git a/mm/memory.c b/mm/memory.c
> index 976cd03954e3..d99f5ef1dd8c 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4119,8 +4119,11 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
>  		if (unlikely(unshare)) {
>  			if (pte_soft_dirty(vmf->orig_pte))
>  				entry = pte_mksoft_dirty(entry);
> -			if (pte_uffd(vmf->orig_pte))
> +			if (pte_uffd(vmf->orig_pte)) {
>  				entry = pte_mkuffd(entry);
> +				if (userfaultfd_rwp(vma))
> +					entry = pte_modify(entry, PAGE_NONE);
> +			}
>  		} else {
>  			entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>  		}
> @@ -6656,9 +6659,12 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>  		 * VM_UFFD_RWP VMA, a protnone PTE without the uffd bit is
>  		 * NUMA hinting and must still fall through to do_numa_page().
>  		 */
> -		if (userfaultfd_pte_rwp(vmf->vma, vmf->orig_pte))
> -			return do_uffd_rwp(vmf);
> -		return do_numa_page(vmf);
> +		if (userfaultfd_pte_rwp(vmf->vma, vmf->orig_pte)) {
> +			if (!(vmf->flags & FAULT_FLAG_UNSHARE))
> +				return do_uffd_rwp(vmf);
> +		} else {
> +			return do_numa_page(vmf);
> +		}
>  	}

Looks like a reasonable direction (without looking into all the details).

I've been also wondering whether KSM should simply skip these uffd regions, they
it never worked properly ... but I'm sure we'd run into issue with leftover uffd
pte bits :(

-- 
Cheers,

David