Re: [PATCH v1 2/4] mm: smaller folio_pte_batch() improvements

[Lorenzo Stoakes]

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