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

[Lorenzo Stoakes (Oracle)]

On Fri, Mar 20, 2026 at 11:07:26AM +0100, David Hildenbrand (Arm) wrote:
> On 3/20/26 11:04, Pedro Falcato wrote:
> > 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@xxxxxxx>
> >>> ---
> >>>  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,
>
> Yes, please. It's easier to argue about __always_inline and constant
> propagation than "this code is too scary big for my CPU so I better do
> an expensive function call if the code is actually needed".

It would make this much more acceptable as a change in general for sure, as it
is already good from a code readability point of view.

Sometimes that and perf align nicely it seems... :)

I do worry about some of these things that might be fine on x86-64, but will
somehow be a total problem on other real architectures (i.e. 64 bit).

>
> --
> Cheers,
>
> David

Cheers, Lorenzo