Re: [PATCH 2/4] mm/mprotect: move softleaf code out of the main function

[Pedro Falcato <pfalcato@suse.de>]

On Thu, Mar 19, 2026 at 10:33:47PM +0100, David Hildenbrand (Arm) wrote:
> On 3/19/26 19:31, Pedro Falcato wrote:
> > Move softleaf change_pte_range code into a separate function. This makes
> > the change_pte_range() function (or where it inlines) a good bit
> > smaller. Plus it lessens cognitive load when reading through the
> > function.
> > 
> > Signed-off-by: Pedro Falcato <pfalcato@suse.de>
> > ---
> >  mm/mprotect.c | 128 +++++++++++++++++++++++++++-----------------------
> >  1 file changed, 68 insertions(+), 60 deletions(-)
> > 
> > diff --git a/mm/mprotect.c b/mm/mprotect.c
> > index 1bd0d4aa07c2..8d4fa38a8a26 100644
> > --- a/mm/mprotect.c
> > +++ b/mm/mprotect.c
> > @@ -211,6 +211,73 @@ static void set_write_prot_commit_flush_ptes(struct vm_area_struct *vma,
> >  	commit_anon_folio_batch(vma, folio, page, addr, ptep, oldpte, ptent, nr_ptes, tlb);
> >  }
> >  
> > +static noinline long change_pte_softleaf(struct vm_area_struct *vma,
> 
> Why the noinline? This sounds like something that works good on some
> CPUs and bad on others, no?
>

If you don't like the noinline I can always remove it, but see my other email
for the justification for most code movement here. In this case, I don't see
how softleaf is the common case (at all) and it's a sizeable amount of code.

Note that the results don't show improvement here (in fact, the opposite),
but we are also spinning on the mprotect() system call, so it's hard for
the icache, branch predictors not to be primed.

> I like the cleanup, but the noinline really is odd.
> 
> > +	unsigned long addr, pte_t *pte, pte_t oldpte, unsigned long cp_flags)
> 
> I'd call that "change_softleaf_pte"
> 
> > +{
> > +	bool uffd_wp = cp_flags & MM_CP_UFFD_WP;
> > +	bool uffd_wp_resolve = cp_flags & MM_CP_UFFD_WP_RESOLVE;
> 
> both can be const.

ACK, will do!

-- 
Pedro