linux-kernel - Re: [PATCH v5 6/7] mm: Optimize mprotect() 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 for Android: free password hash cracker in your pocket

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

Message-ID: <9cbe9c6a-0013-4239-9347-bf5d43021fe3@lucifer.local>
Date: Wed, 6 Aug 2025 10:50:55 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Dev Jain <dev.jain@....com>
Cc: David Hildenbrand <david@...hat.com>, akpm@...ux-foundation.org,
        ryan.roberts@....com, willy@...radead.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, catalin.marinas@....com, will@...nel.org,
        Liam.Howlett@...cle.com, vbabka@...e.cz, jannh@...gle.com,
        anshuman.khandual@....com, peterx@...hat.com, joey.gouly@....com,
        ioworker0@...il.com, baohua@...nel.org, kevin.brodsky@....com,
        quic_zhenhuah@...cinc.com, christophe.leroy@...roup.eu,
        yangyicong@...ilicon.com, linux-arm-kernel@...ts.infradead.org,
        hughd@...gle.com, yang@...amperecomputing.com, ziy@...dia.com
Subject: Re: [PATCH v5 6/7] mm: Optimize mprotect() by PTE batching

On Wed, Aug 06, 2025 at 03:07:49PM +0530, Dev Jain wrote:
> > >
> > > You mean in _this_ PTE of the batch right? As we're invoking these
> > > on each part
> > > of the PTE table.
> > >
> > > I mean I guess we can simply do:
> > >
> > >     struct page *first_page = pte_page(ptent);
> > >
> > > Right?
> >
> > Yes, but we should forward the result from vm_normal_page(), which does
> > exactly that for you, and increment the page accordingly as required,
> > just like with the pte we are processing.
>
> Makes sense, so I guess I will have to change the signature of
> prot_numa_skip()
>
> to pass a double ptr to a page instead of folio and derive the folio in the
> caller,
>
> and pass down both the folio and the page to
> set_write_prot_commit_flush_ptes.

I already don't love how we psas the folio back from there for very dubious
benefit. I really hate the idea of having a struct **page parameter...

I wonder if we should just have a quick fixup for hotfix, and refine this more
later?

I foresee some debate otherwise...

>
>
> >
> > ...
> >
> > > >
> > > > > +            else
> > > > > +                prot_commit_flush_ptes(vma, addr, pte, oldpte, ptent,
> > > > > +                    nr_ptes, /* idx = */ 0, /* set_write =
> > > > > */ false, tlb);
> > > >
> > > > Semi-broken intendation.
> > >
> > > Because of else then 2 lines after?
> >
> > prot_commit_flush_ptes(vma, addr, pte, oldpte, ptent,
> >                nr_ptes, /* idx = */ 0, /* set_write = */ false, tlb);
> >
> > Is what I would have expected.
> >
> >
> > I think a smart man once said, that if you need more than one line per
> > statement in
> > an if/else clause, a set of {} can aid readability. But I don't
> > particularly care :)
> >

Can do this in follow up too.