Messages in this thread | | | Date | Wed, 1 Apr 2020 13:14:33 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH V4 08/13] ftrace: Add perf text poke events for ftrace trampolines |
| |
On Wed, Apr 01, 2020 at 01:42:50PM +0300, Adrian Hunter wrote: > On 1/04/20 1:09 pm, Peter Zijlstra wrote: > > On Wed, Mar 04, 2020 at 11:06:28AM +0200, Adrian Hunter wrote: > >> Add perf text poke events for ftrace trampolines when created and when > >> freed. > > > > If I'm not mistaken that ends up like so: > > > > static void ftrace_update_trampoline(struct ftrace_ops *ops) > > { > > + unsigned long trampoline = ops->trampoline; > > + > > arch_ftrace_update_trampoline(ops); > > + if (ops->trampoline && ops->trampoline != trampoline && > >> + (ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) { > >> + /* Add to kallsyms before the perf events */ > > + ftrace_add_trampoline_to_kallsyms(ops); > >> + perf_event_ksymbol(PERF_RECORD_KSYMBOL_TYPE_OOL, > >> + ops->trampoline, ops->trampoline_size, false, > >> + FTRACE_TRAMPOLINE_SYM); > >> + /* > >> + * Record the perf text poke event after the ksymbol register > >> + * event. > >> + */ > >> + perf_event_text_poke((void *)ops->trampoline, NULL, 0, > >> + (void *)ops->trampoline, > >> + ops->trampoline_size); > > } > > } > > > > And afaict, that is wrong. > > > > The thing is; arch_ftrace_update_trampoline() can actually *update* an > > existing trampoline, as per the name. Yes it also creates a trampoline > > if there isn't one already, but if there already is one, it will modify > > it in-place. > > > > I see the appeal of having this event in generic code; but I'm thinking > > you'll need the update even in arch code anyway, at which point it'd > > probably be easier to do all of this in arch code. > > For x86, we use text_poke_bp for updates which already does text_poke events > via text_poke_bp_batch.
Ah, indeed! Damn, I even wrote that code :/
> It might be reasonable to assume other architectures will also need to put > updates through a common text poker which will take care of text_poke events.
You'd better look, I recently rewrote the x86/ftrace code to be 'sane' and not re-implement all the text poking stuff itself.
But I suppose that any arch adding support for this can fix that up if needed.
| |