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

[David Hildenbrand (Arm)]

On 10/20/26 15:29, Lance Yang wrote:
> 
> 
> On 2026/10/20 21:03, David Hildenbrand (Arm) wrote:
>> On 10/20/26 07:55, Lance Yang wrote:
>>>
>>>
>>> Right.
>>>
>>>
>>> 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.
>>>
>>>
>>> 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

I think QEMU just always sets MADV_MERGABLE and later activates uffd. So that
needs some care to not disable them. So let's rather try to get it fixed properly.

Can you send out proper patches? Thanks!

-- 
Cheers,

David