Re: [PATCH v6 6/8] x86/module: prepare module loading for ROX allocations of text

Peter Zijlstra
2024-10-17T09:35:19Z

On Wed, Oct 16, 2024 at 05:01:28PM -0400, Steven Rostedt wrote:
> On Wed, 16 Oct 2024 15:24:22 +0300
> Mike Rapoport <rppt@kernel.org> wrote:
>
> > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> > index 8da0e66ca22d..b498897b213c 100644
> > --- a/arch/x86/kernel/ftrace.c
> > +++ b/arch/x86/kernel/ftrace.c
> > @@ -118,10 +118,13 @@ ftrace_modify_code_direct(unsigned long ip, const char *old_code,
> > return ret;
> >
> > /* replace the text with the new text */
> > - if (ftrace_poke_late)
> > + if (ftrace_poke_late) {
> > text_poke_queue((void *)ip, new_code, MCOUNT_INSN_SIZE, NULL);
> > - else
> > - text_poke_early((void *)ip, new_code, MCOUNT_INSN_SIZE);
> > + } else {
> > + mutex_lock(&text_mutex);
> > + text_poke((void *)ip, new_code, MCOUNT_INSN_SIZE);
> > + mutex_unlock(&text_mutex);
> > + }
> > return 0;
> > }
>
> So this slows down the boot by over 30ms. That may not sound like much, but
> we care very much about boot times. This code is serialized with boot and
> runs whenever ftrace is configured in the kernel. The way I measured this,
> was that I added:
>

> If this is only needed for module load, can we at least still use the
> text_poke_early() at boot up?

Right, so I don't understand why this is needed at all.
ftrace_module_init() runs before complete_formation() which normally
switches to ROX, as such ftrace should be able to continue to do direct
modifications here.

Which reminds me, at some point I did patches adding a
MODULE_STATE_UNFORMED callback in order for static_call / jump_label to
be able to avoid the expensive patching on module load as well (arguably
ftrace should be using that too, instead of a custom callback).