[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <14e4407c-11b2-42bf-aa63-343762018877@lucifer.local>
Date: Wed, 2 Jul 2025 10:08:14 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: David Hildenbrand <david@...hat.com>
Cc: Oscar Salvador <osalvador@...e.de>, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>, Jann Horn <jannh@...gle.com>,
Mike Rapoport <rppt@...nel.org>,
Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
Zi Yan <ziy@...dia.com>, Matthew Brost <matthew.brost@...el.com>,
Joshua Hahn <joshua.hahnjy@...il.com>, Rakie Kim <rakie.kim@...com>,
Byungchul Park <byungchul@...com>, Gregory Price <gourry@...rry.net>,
Ying Huang <ying.huang@...ux.alibaba.com>,
Alistair Popple <apopple@...dia.com>, Pedro Falcato <pfalcato@...e.de>,
Rik van Riel <riel@...riel.com>, Harry Yoo <harry.yoo@...cle.com>
Subject: Re: [PATCH v1 2/4] mm: smaller folio_pte_batch() improvements
On Wed, Jul 02, 2025 at 11:00:48AM +0200, David Hildenbrand wrote:
> On 02.07.25 10:51, Lorenzo Stoakes wrote:
> > On Wed, Jul 02, 2025 at 10:48:20AM +0200, David Hildenbrand wrote:
> > > On 02.07.25 10:42, Oscar Salvador wrote:
> > > > On Fri, Jun 27, 2025 at 01:55:08PM +0200, David Hildenbrand wrote:
> > > > > Let's clean up a bit:
> > > > >
> > > > > (1) No need for start_ptep vs. ptep anymore, we can simply use ptep
> > > > >
> > > > > (2) Let's switch to "unsigned int" for everything
> > > > >
> > > > > (3) We can simplify the code by leaving the pte unchanged after the
> > > > > pte_same() check.
> > > > >
> > > > > (4) Clarify that we should never exceed a single VMA; it indicates a
> > > > > problem in the caller.
> > > > >
> > > > > No functional change intended.
> > > > >
> > > > > Signed-off-by: David Hildenbrand <david@...hat.com>
> > > >
> > > > Hi David :-),
> > > >
> > > > I have to confess that I fell in the same trap as Lorenzo wrt.
> > > > __pte_batch_clear_ignored changing the pte value.
> > > > So I'm not sure if it would be nice to place a little comment in
> > > > __pte_batch_clear_ignored claryfing that pte's value remains unchanged ?
> > >
> > > I mean, that's how all our pte modification functions work, really? :)
> > >
> > > Thanks!
> >
> > I mean, it might be that me and Oscar are similarly 'challenged' in this
> > respect :P (high 5 Oscar!) but I think the issue here is that it's sort of
> > a compounded use, and in fact some functions do modify stuff, which is why
> > we end up with all the ptep ptent etc. fun.
> >
> > Up to you re: comment, but I think maybe in cases where it's a reallly
> > compounded set of stuff it's potentially useful.
> >
> > But obviously we still do do this all over the place elsewhere with no
> > comment...
>
> Well, if you are not passing in a *value* and not a pointer to a function,
> you would not expect for that *value* to change? :)
>
> Yes, once we pass pointers it's different. Or when we're using weird macros.
>
> Adding a comment that a function will not modify a value that is ...
> passed-by-value? Maybe it's just me that doesn't get why that should be
> particularly helpful :)
I think the issue is that we've passed around 'pte' as value and pointer (and of
course, via macros...) previously so that's the cause of the confusion, often.
This is why I really am a fan of us consistently saying ptep when passing a
pointer.
Anyway, I think on balance a comment isn't useful here, agreed! ;)
>
> --
> Cheers,
>
> David / dhildenb
>
Powered by blists - more mailing lists