Re: [PATCH 2/3] mm: mprotect: avoid unnecessary struct page accessing if pte_protnone()

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 2025/10/14 15:16, David Hildenbrand wrote:
>>>
>>>> +            if (prot_numa && pte_protnone(oldpte))
>>>> +                continue;
>>>> +
>>>>                page = vm_normal_page(vma, addr, oldpte);
>>>>                if (page)
>>>>                    folio = page_folio(page);
>>>
>>> I could have sworn we discussed that while fixing the prot_numa_skip()
>>> fallout.
>>
>> I'm not follow the thread, but we found that vm_normal_page does
>> introduce regression for mprotect benchmark(libMicro) with
>> this vm_normal_page().
>>
> 
> Right, I raised it here:
> 
> https://lkml.kernel.org/r/aa496798-5ac6-4cb0-bdc2-91515172e935@redhat.com

Thanks for the links, let's fix it now ;)

> 
> I questioned how relevant it would be in practice.
> 
> I'm surprised it shows up in a mprotect() benchmark: mprotect() itself 
> would never be able to set MM_CP_PROT_NUMA, so the code wold not 
> actually be executed.
> 

Sorry, my description is very clear, the regression is not about prot
numa, I mean the vm_normal_page does introduce some regression when
mprotect benchmark in libMicro, before cac1db8c3aad （"mm: optimize
mprotect() by PTE batching"), we only call vm_normal_page in
can_change_pte_writable(), but now it is unconditional called and
10% regression in some libMicro mprotect benchmark.

We could add limit to call of vm_normal_page， eg，cp_flags
check, and using pte_batch_hint, but it is unclear whether it is worth
doing so.