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

[David Hildenbrand <david@redhat.com>]

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.

-- 
Cheers

David / dhildenb