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

[David Hildenbrand <david@redhat.com>]

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.


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

-- 
Cheers

David / dhildenb