Re: [PATCH] mempolicy: Optimize queue_folios_pte_range by PTE batching

[David Hildenbrand <david@redhat.com>]

On 11.04.25 10:13, Dev Jain wrote:
> After the check for queue_folio_required(), the code only cares about the
> folio in the for loop, i.e the PTEs are redundant. Therefore, optimize this
> loop by skipping over a PTE batch mapping the same folio.
> 
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
> Unfortunately I have only build tested this since my test environment is
> broken.
> 
>   mm/mempolicy.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index b28a1e6ae096..b019524da8a2 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -573,6 +573,9 @@ static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
>   	pte_t *pte, *mapped_pte;
>   	pte_t ptent;
>   	spinlock_t *ptl;
> +	int max_nr;
> +	const fpb_t fpb_flags = FPB_IGNORE_DIRTY | FPB_IGNORE_SOFT_DIRTY;
> +	int nr = 1;

Try sticking to reverse xmas tree, please. (not completely the case 
here, but fpb_flags can easily be moved all he way to the top)

Also, why are you initializing nr to 1 here if you reinitialize it below?

>   
 >   	ptl = pmd_trans_huge_lock(pmd, vma);>   	if (ptl) {
> @@ -586,7 +589,8 @@ static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
>   		walk->action = ACTION_AGAIN;
>   		return 0;
>   	}
 > -	for (; addr != end; pte++, addr += PAGE_SIZE) {> +	for (; addr != 
end; pte += nr, addr += nr * PAGE_SIZE) {
> +		nr = 1;
>   		ptent = ptep_get(pte);
>   		if (pte_none(ptent))
>   			continue;
> @@ -607,6 +611,11 @@ static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
>   		if (!queue_folio_required(folio, qp))
>   			continue;
>   		if (folio_test_large(folio)) {
> +			max_nr = (end - addr) >> PAGE_SHIFT;
> +			if (max_nr != 1)
> +				nr = folio_pte_batch(folio, addr, pte, ptent,
> +						     max_nr, fpb_flags,
> +						     NULL, NULL, NULL);

We should probably do that immediately after we verified that 
vm_normal_folio() have us something reasonable.

>   			/*
>   			 * A large folio can only be isolated from LRU once,
>   			 * but may be mapped by many PTEs (and Copy-On-Write may
> @@ -633,6 +642,7 @@ static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
>   			qp->nr_failed++;
>   			if (strictly_unmovable(flags))
>   				break;
> +			qp->nr_failed += nr - 1;

Can't we do qp->nr_failed += nr; above?

Weird enough, queue_folios_pmd() also only does qp->nr_failed++, but 
queue_pages_range() documents it that way.

>   		}
>   	}
>   	pte_unmap_unlock(mapped_pte, ptl);


-- 
Cheers,

David / dhildenb