linux-kernel - Re: [RFC PATCH 3/3] mm/mremap: Use can_pte_batch_count() instead of folio_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>] [day] [month] [year] [list]

Message-ID: <c484887a684e42c3a775f13af93c3a9f@huawei.com>
Date: Tue, 28 Oct 2025 13:01:07 +0000
From: zhangqilong <zhangqilong3@...wei.com>
To: David Hildenbrand <david@...hat.com>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>, "lorenzo.stoakes@...cle.com"
	<lorenzo.stoakes@...cle.com>, "Liam.Howlett@...cle.com"
	<Liam.Howlett@...cle.com>, "vbabka@...e.cz" <vbabka@...e.cz>,
	"rppt@...nel.org" <rppt@...nel.org>, "surenb@...gle.com" <surenb@...gle.com>,
	"mhocko@...e.com" <mhocko@...e.com>, "jannh@...gle.com" <jannh@...gle.com>,
	"pfalcato@...e.de" <pfalcato@...e.de>
CC: "linux-mm@...ck.org" <linux-mm@...ck.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Wangkefeng (OS Kernel Lab)"
	<wangkefeng.wang@...wei.com>, Sunnanyong <sunnanyong@...wei.com>
Subject: Re: [RFC PATCH 3/3] mm/mremap: Use can_pte_batch_count() instead of
 folio_pte_batch() for pte batch

On 27.10.25 15:03, Zhang Qilong wrote:
> > In current mremap_folio_pte_batch(), 1) pte_batch_hint() always return
> > one pte in non-ARM64 machine, it is not efficient. 2) Next, it need to
> > acquire a folio to call the folio_pte_batch().
> >
> > Due to new added can_pte_batch_count(), we just call it instead of
> > folio_pte_batch(). And then rename mremap_folio_pte_batch() to
> > mremap_pte_batch().
> >
> > Signed-off-by: Zhang Qilong <zhangqilong3@...wei.com>
> > ---
> >   mm/mremap.c | 16 +++-------------
> >   1 file changed, 3 insertions(+), 13 deletions(-)
> >
> > diff --git a/mm/mremap.c b/mm/mremap.c index
> > bd7314898ec5..d11f93f1622f 100644
> > --- a/mm/mremap.c
> > +++ b/mm/mremap.c
> > @@ -169,27 +169,17 @@ static pte_t move_soft_dirty_pte(pte_t pte)
> >   		pte = pte_swp_mksoft_dirty(pte);
> >   #endif
> >   	return pte;
> >   }
> >
> > -static int mremap_folio_pte_batch(struct vm_area_struct *vma,
> > unsigned long addr,
> > +static int mremap_pte_batch(struct vm_area_struct *vma, unsigned long
> > +addr,
> >   		pte_t *ptep, pte_t pte, int max_nr)
> >   {
> > -	struct folio *folio;
> > -
> >   	if (max_nr == 1)
> >   		return 1;
> >
> > -	/* Avoid expensive folio lookup if we stand no chance of benefit. */
> > -	if (pte_batch_hint(ptep, pte) == 1)
> > -		return 1;
> > -
> > -	folio = vm_normal_folio(vma, addr, pte);
> > -	if (!folio || !folio_test_large(folio))
> > -		return 1;
> > -
> > -	return folio_pte_batch(folio, ptep, pte, max_nr);
> > +	return can_pte_batch_count(vma, ptep, &pte, max_nr, 0);
> >   }
> >
> >   static int move_ptes(struct pagetable_move_control *pmc,
> >   		unsigned long extent, pmd_t *old_pmd, pmd_t *new_pmd)
> >   {
> > @@ -278,11 +268,11 @@ static int move_ptes(struct
> pagetable_move_control *pmc,
> >   		 * make sure the physical page stays valid until
> >   		 * the TLB entry for the old mapping has been
> >   		 * flushed.
> >   		 */
> >   		if (pte_present(old_pte)) {
> > -			nr_ptes = mremap_folio_pte_batch(vma, old_addr,
> old_ptep,
> > +			nr_ptes = mremap_pte_batch(vma, old_addr, old_ptep,
> >   							 old_pte,
> max_nr_ptes);
> >   			force_flush = true;
> >   		}
> >   		pte = get_and_clear_ptes(mm, old_addr, old_ptep, nr_ptes);
> 
> get_and_clear_ptes() documents: "Clear present PTEs that map consecutive
> pages of the same folio, collecting dirty/accessed bits."

Oh, good catch. My focus was solely on the implementations of get_and_clear_ptes()
and set_ptes() and regarding their multi-folio PTEs handling, and I missed this comment.
get_and_clear_ptes() will collect dirty/accessed bits in batch ranges, and set in later.

> 
> And as can_pte_batch_count() will merge access/dirty bits, you would silently
> set ptes dirty/accessed that belong to other folios, which sounds very wrong.

Year, your analysis is very thorough. The access/dirty bit will be merged between
neighboring batched folios due to get_and_clear_ptes().

If caller don't want to merge access/dirty bits between folios(means not ignore any bits), 
they should call folio_pte_batch() or the new interface with 'flag | FPB_RESPECT_DIRTY '
(access bit is default be respected).

> 
> Staring at the code, I wonder if there is also a problem with the write bit, have
> to dig into that.

The write bit is handled similarly to dirty bit. If called don't want merge write between folios, we
could call the new interface with 'flag | FPB_RESPECT_WRITE', and pte_same() will compare the write bit
with the next neighboring folio pte.  If it's different, it will break.

Thanks for the in-depth review.
 
> --
> Cheers
> 
> David / dhildenb
>