linux-kernel - Re: [PATCH] mempolicy: Optimize queue_folios_pte

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <09c77ab5-65fc-4bca-a7e5-2b11bba9330d@redhat.com>
Date: Tue, 15 Apr 2025 12:17:51 +0200
From: David Hildenbrand <david@...hat.com>
To: Dev Jain <dev.jain@....com>, akpm@...ux-foundation.org
Cc: ryan.roberts@....com, willy@...radead.org, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, hughd@...gle.com, vishal.moola@...il.com,
 yang@...amperecomputing.com, ziy@...dia.com
Subject: Re: [PATCH] mempolicy: Optimize queue_folios_pte_range by PTE
 batching

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