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

[David Hildenbrand]

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 :)

--
Cheers,

David / dhildenb