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

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

On 2025/10/14 16:56, David Hildenbrand wrote:
> On 14.10.25 10:02, Kefeng Wang wrote:
>>
>>
>> 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.
> 
> Right, I think we discussed that as well at some point, and possible 
> ways to optimize if we ever have to. We don't have to optimize for each 
> and every microbenchmark that heavily, though.
> 

OK, let's do it if real benchmark hint the issue.