Re: [PATCH 3/4] mm/mprotect: un-inline folio_pte_batch_flags()

["Lorenzo Stoakes (Oracle)" <ljs@kernel.org>]

On Thu, Mar 19, 2026 at 10:41:54PM +0100, David Hildenbrand (Arm) wrote:
>
> >
> > This is all seems VERY delicate, and subject to somebody else coming along and
> > breaking it/causing some of these noinline/__always_inline invocations to make
> > things far worse.
> >
> > I also reserve the right to seriously rework this pile of crap software.
> >
> > I'd rather we try to find less fragile ways to optimise!
> >
> > Maybe there's some steps that are bigger wins than others?
>
> What we can do is, collect similar folio_pte_batch_*() variants and
> centralize them in mm/utils.c.

I'm not sure that addresses any of the comments above?

Putting logic specific to components of mm away from where those components
are and into mm/util.c seems like a complete regression in terms of
fragility and code separation.

And for what reason would you want to do that? To force a noinline of an
inline and people 'just have to know' that's why you randomly separated the
two?

Doesn't sound appealing overall.

I'd rather we find a way to implement the batching such that it doesn't
exhibit bad inlining decisions in the first place.

I mean mprotect_folio_batch() having !folio, !folio_test_large() checks
only there is already silly, we should have a _general_ function that does
optimisations like that.

Isn't the issue rather than folio_pte_batch_flags() shouldn't be an inline
function in internal.h but rather a function in mm/util.c?

>
> For
>
> 	nr_ptes = mprotect_folio_pte_batch(folio, pte, oldpte,
> 					   max_nr_ptes, /* flags = */ 0)
>
> We might just be able to use folio_pte_batch()?

Yup.

>
> For the other variant (soft-dirt+write) we'd have to create a helper like
>
> 	folio_pte_batch_sd_w() [better name suggestion welcome]
>
> That will reduce the code footprint overall I guess.

I mean yeah that's a terrible name so obviously it'd have to be something
better.

But again, this seems pretty stupid, now we're writing a bunch of duplicate
per-case code to force noinline because we made the central function inline
no?

That's also super fragile, because an engineer might later decide that
pattern is horrible and fix it, and we regress this.

But I mean overall, is the perf here really all that important? Are people
really that dependent on mprotect() et al. performing brilliantly fast?

Couldn't we do this with any mm interface and end up making efforts that
degrade code quality, increase fragility for dubious benefit?

>
> --
> Cheers,
>
> David

Thanks, Lorenzo