linux-kernel - Re: [PATCH v3 2/2] mm: Optimize mremap() by PTE batching

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: <53a39d5f-6ea2-403f-8837-fb00d7f8d6b8@lucifer.local>
Date: Wed, 28 May 2025 05:49:50 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Dev Jain <dev.jain@....com>
Cc: akpm@...ux-foundation.org, Liam.Howlett@...cle.com, vbabka@...e.cz,
        jannh@...gle.com, pfalcato@...e.de, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, david@...hat.com, peterx@...hat.com,
        ryan.roberts@....com, mingo@...nel.org, libang.li@...group.com,
        maobibo@...ngson.cn, zhengqi.arch@...edance.com, baohua@...nel.org,
        anshuman.khandual@....com, willy@...radead.org, ioworker0@...il.com,
        yang@...amperecomputing.com, baolin.wang@...ux.alibaba.com,
        ziy@...dia.com, hughd@...gle.com
Subject: Re: [PATCH v3 2/2] mm: Optimize mremap() by PTE batching

On Wed, May 28, 2025 at 09:02:26AM +0530, Dev Jain wrote:
>
> On 27/05/25 10:16 pm, Lorenzo Stoakes wrote:
> > On Tue, May 27, 2025 at 10:08:59PM +0530, Dev Jain wrote:
> > > On 27/05/25 9:59 pm, Lorenzo Stoakes wrote:
> > [snip]
> > > > If I invoke split_huge_pmd(), I end up with a bunch of PTEs mapping the same
> > > > large folio. The folio itself is not split, so nr_ptes surely will be equal to
> > > > something >1 here right?
> > >
> > > Thanks for elaborating.
> > >
> > > So,
> > >
> > > Case 1: folio splitting => nr_ptes = 1 => the question of a/d bit smearing
> > > disappears.
> > >
> > > Case 2: page table splitting => consec PTEs point to the same large folio
> > > => nr_ptes > 1 => get_and_clear_full_ptes() will smear a/d bits on the
> > > new ptes, which is correct because we are still pointing to the same large
> > > folio.
> > >
> > OK awesome, I thought as much, just wanted to make sure :) we are good then.
> >
> > The accessed/dirty bits really matter at a folio granularity (and especially
> > with respect to reclaim/writeback which both operate at folio level) so the
> > smearing as you say is fine.
> >
> > This patch therefore looks fine, only the trivial comment fixup.
> >
> > I ran the series on my x86-64 setup (fwiw) with no build/mm selftest errors.
>
> Thanks!
>
>
> >
> > Sorry to be a pain but could you respin with the commit message for this patch
> > updated to explicitly mention that the logic applies for the non-contPTE split
> > PTE case (and therefore also helps performance there)? That and the trivial
> > thing of dropping that comment.
>
> What do you mean by the non-contpte case? In that case the PTEs do not point
> to the same folio or are misaligned, and there will be no optimization. This

Split page table large folio.

> patch is optimizing two things: 1) ptep_get() READ_ONCE accesses 2) reduction
> in number of TLBIs for contig blocks, both of which happen in the contpte case.
>

But it impacts split huge pages. Your code changes this behaviour. We need to
make this clear :)

> In general, the patch should have a minor improvement on other arches because
> we are detecting a batch and processing it together, thus saving on a few
> function calls, but the main benefit is for arm64.

Ack, but you are changing this behaviour. The commit message doesn't make this
clear and seems to imply this only impacts contPTE cases. Or at least isn't
clear enough

A simple additional paragraph like:

'Transparent huge pages which have been split into PTEs will also be impacted,
however the performance gain in this case is expected to be modest'

Will sort this out.

Thanks!

>
> >
> > Then we should be good for a tag unless somebody else spots something
> > egregious :)
> >
> > Thanks for this! Good improvement.
> >
> > [snip]
> >
> > Cheers, Lorenzo