Re: [PATCH v2 13/20] x86/mm: enable page table sharing

[Anthony Yznaga <anthony.yznaga@oracle.com>]

Hi Yongting,

On 8/18/25 2:44 AM, Yongting Lin wrote:
> Thank you! Anthony.
> 
> Yep, I checked the comments in arch/mm/x86/fault.c file which says as your
> advices in previous email.
> 
> 
> I changed my code in kernel 5.5 as below:
> 
>         if (unlikely(is_shared_vma) && ((fault & VM_FAULT_RETRY) &&
>             (flags & FAULT_FLAG_ALLOW_RETRY) || fault_signal_pending(fault, regs)))
>                 mmap_read_unlock(mm);
> 
> BTW: I wrote some selftests in my github repostory, which perform
> the basic function of mshare, and I will write some complicated cases
> to support the new functions or defect found in mshare. For example,
> once you support mshare as a VMA in KVM (just as the defeat viewed by
> Jann Horn), I will add extra test cases to verify its correctiness for
> this scenario.

This is great! I'll take a look at them in more detail. I just sent out 
an updated series.

> 
>  From Jann Horn's review:
> https://lore.kernel.org/all/CAG48ez3cUZf+xOtP6UkkS2-CmOeo+3K5pvny0AFL_XBkHh5q_g@mail.gmail.com/

My new series does not yet have support for mmu notifiers. It's 
something I'm working on, but there are key issues to overcome. One is 
that I need to update the implementation of mm_take_all_locks() to also 
carefully take all locks in any mapped mshare regions. The other is that 
passing through mmu notifier calls for arch_invalidate_secondary_tlbs 
callbacks is especially tricky because the callback is not allowed to 
sleep due to holding a ptl spin lock.

> 
> Currently, I put my selftest in my github repostory, and you could retrieve it
> as below:
> 
>      git remote add yongting-mshare-selftests https://github.com/ivanalgo/linux-kernel-develop/
>      git fetch yongting-mshare-selftests dev-mshare-v2-selftest-v1
>      git cherry-pick a64f2ff6497d13c09badc0fc68c44d9995bc2fef
> 
> At this stage, I am not sure what is the best way to proceed:
> - Should I send them as part of your next version (v3)?
> - Or should I post them separately as [RFC PATCH] for early review?
> 
> Please let me know your preference and any sugestion is welcome.
> I am happy to rebase and resend in the format that works best for
> the community.

I may have more feedback once I take look. I suggest starting by 
updating them to work with the series I just sent out.

Thanks,
Anthony
  >
> Thanks
> Yongting
> 
>> Anthony
>>
>>>
>>> As a result, needs to release vma->vm_mm.mmap_lock as well.
>>>
>>> So it is supposed to be like below:
>>>
>>> -    fault = handle_mm_fault(vma, address, flags, regs);
>>> +    fault = handle_mm_fault(vma, addr, flags, regs);
>>> +
>>> +    if (unlikely(is_shared_vma) && ((fault & VM_FAULT_COMPLETED) ||
>>> +        (fault & VM_FAULT_RETRY) || fault_signal_pending(fault, regs))) {
>>> +        mmap_read_unlock(vma->vm_mm);
>>> +        mmap_read_unlock(mm);
>>> +    }
>>>
>>>>          if (fault_signal_pending(fault, regs)) {
>>>>            /*
>>>> @@ -1413,6 +1446,8 @@ void do_user_addr_fault(struct pt_regs *regs,
>>>>            goto retry;
>>>>        }
>>>>    +    if (unlikely(is_shared_vma))
>>>> +        mmap_read_unlock(vma->vm_mm);
>>>>        mmap_read_unlock(mm);
>>>>    done:
>>>>        if (likely(!(fault & VM_FAULT_ERROR)))
>>>> diff --git a/mm/Kconfig b/mm/Kconfig
>>>> index e6c90db83d01..8a5a159457f2 100644
>>>> --- a/mm/Kconfig
>>>> +++ b/mm/Kconfig
>>>> @@ -1344,7 +1344,7 @@ config PT_RECLAIM
>>>>      config MSHARE
>>>>        bool "Mshare"
>>>> -    depends on MMU
>>>> +    depends on MMU && ARCH_SUPPORTS_MSHARE
>>>>        help
>>>>          Enable msharefs: A ram-based filesystem that allows multiple
>>>>          processes to share page table entries for shared pages. A file
>>>
>>> Yongting Lin.
>>
>>