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

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

On 2025/10/13 23:53, David Hildenbrand wrote:
> On 13.10.25 14:15, Kefeng Wang wrote:
>> If the pte_protnone() is true, we could avoid unnecessary struct page
>> accessing and reduce cache footprint when scanning page tables for prot
>> numa, the performance test of pmbench memory accessing benchmark
>> should be benifit, see more commit a818f5363a0e ("autonuma: reduce cache
>> footprint when scanning page tables").
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   mm/mprotect.c | 21 +++++++++++----------
>>   1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/mm/mprotect.c b/mm/mprotect.c
>> index 51a28781de9d..0f31c09c1726 100644
>> --- a/mm/mprotect.c
>> +++ b/mm/mprotect.c
>> @@ -118,18 +118,13 @@ static int mprotect_folio_pte_batch(struct folio 
>> *folio, pte_t *ptep,
>>       return folio_pte_batch_flags(folio, NULL, ptep, &pte, 
>> max_nr_ptes, flags);
>>   }
>> -static bool prot_numa_skip(struct vm_area_struct *vma, unsigned long 
>> addr,
>> -               pte_t oldpte, pte_t *pte, int target_node,
>> -               struct folio *folio)
>> +static bool prot_numa_skip(struct vm_area_struct *vma, int target_node,
>> +        struct folio *folio)
>>   {
>>       bool ret = true;
>>       bool toptier;
>>       int nid;
>> -    /* Avoid TLB flush if possible */
>> -    if (pte_protnone(oldpte))
>> -        goto skip;
>> -
>>       if (!folio)
>>           goto skip;
>> @@ -307,17 +302,23 @@ static long change_pte_range(struct mmu_gather 
>> *tlb,
>>               struct page *page;
>>               pte_t ptent;
>> +            /*
>> +             * Avoid TLB flush if possible and unnecessary struct
>> +             * page accessing when prot numa.
>> +             */
> 
> I think we should just simplify to
> 
> "/* Already in the desired state. */"
> 
> or sth. like that. No need to mention the struct-page implementation 
> details in this comment. Also, I don't think there is a need to mention 
> the TLB flush when it's really in the desired state already.

Sure.

> 
> 
>> +            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().