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

[Lance Yang <lance.yang@linux.dev>]

On 2026/10/20 21:03, David Hildenbrand (Arm) wrote:
> 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

+1. Did it ever actually work there :) Why not just skip them and
call it a day :D

> pte bits :(
>