On Wed, Oct 16, 2024 at 09:57:55AM +0900, Masami Hiramatsu (Google) wrote: > From: Masami Hiramatsu (Google) <[email protected]> > > Pass ftrace_regs to the fgraph_ops::entryfunc(). If ftrace_regs is not > available, it passes a NULL instead. User callback function can access > some registers (including return address) via this ftrace_regs. > > Note that the ftrace_regs can be NULL when the arch does NOT define: > HAVE_DYNAMIC_FTRACE_WITH_ARGS or HAVE_DYNAMIC_FTRACE_WITH_REGS. > More specifically, if HAVE_DYNAMIC_FTRACE_WITH_REGS is defined but > not the HAVE_DYNAMIC_FTRACE_WITH_ARGS, and the ftrace ops used to > register the function callback does not set FTRACE_OPS_FL_SAVE_REGS. > In this case, ftrace_regs can be NULL in user callback. > > Signed-off-by: Masami Hiramatsu (Google) <[email protected]> > Cc: Steven Rostedt <[email protected]> > Cc: Mark Rutland <[email protected]> > Cc: Catalin Marinas <[email protected]> > Cc: Will Deacon <[email protected]> > Cc: Huacai Chen <[email protected]> > Cc: WANG Xuerui <[email protected]> > Cc: Michael Ellerman <[email protected]> > Cc: Nicholas Piggin <[email protected]> > Cc: Christophe Leroy <[email protected]> > Cc: Naveen N Rao <[email protected]> > Cc: Madhavan Srinivasan <[email protected]> > Cc: Paul Walmsley <[email protected]> > Cc: Palmer Dabbelt <[email protected]> > Cc: Albert Ou <[email protected]> > Cc: Thomas Gleixner <[email protected]> > Cc: Ingo Molnar <[email protected]> > Cc: Borislav Petkov <[email protected]> > Cc: Dave Hansen <[email protected]> > Cc: [email protected] > Cc: "H. Peter Anvin" <[email protected]> > Cc: Mathieu Desnoyers <[email protected]> > > --- > Changes in v16: > - Add a note when the ftrace_regs can be NULL. > - Update against for the latest kernel. > Changes in v11: > - Update for the latest for-next branch. > Changes in v8: > - Just pass ftrace_regs to the handler instead of adding a new > entryregfunc. > - Update riscv ftrace_graph_func(). > Changes in v3: > - Update for new multiple fgraph. > --- > arch/arm64/kernel/ftrace.c | 20 +++++++++++- > arch/loongarch/kernel/ftrace_dyn.c | 10 +++++- > arch/powerpc/kernel/trace/ftrace.c | 2 + > arch/powerpc/kernel/trace/ftrace_64_pg.c | 10 ++++-- > arch/riscv/kernel/ftrace.c | 17 ++++++++++ > arch/x86/kernel/ftrace.c | 50 > +++++++++++++++++++++--------- > include/linux/ftrace.h | 17 ++++++++-- > kernel/trace/fgraph.c | 25 +++++++++------ > kernel/trace/ftrace.c | 3 +- > kernel/trace/trace.h | 3 +- > kernel/trace/trace_functions_graph.c | 3 +- > kernel/trace/trace_irqsoff.c | 3 +- > kernel/trace/trace_sched_wakeup.c | 3 +- > kernel/trace/trace_selftest.c | 8 +++-- > 14 files changed, 129 insertions(+), 45 deletions(-) > > diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c > index b2d947175cbe..a5a285f8a7ef 100644 > --- a/arch/arm64/kernel/ftrace.c > +++ b/arch/arm64/kernel/ftrace.c > @@ -481,7 +481,25 @@ void prepare_ftrace_return(unsigned long self_addr, > unsigned long *parent, > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs) > { > - prepare_ftrace_return(ip, &arch_ftrace_regs(fregs)->lr, > arch_ftrace_regs(fregs)->fp); > + unsigned long return_hooker = (unsigned long)&return_to_handler; > + unsigned long frame_pointer = arch_ftrace_regs(fregs)->fp; > + unsigned long *parent = &arch_ftrace_regs(fregs)->lr; > + unsigned long old; > + > + if (unlikely(atomic_read(¤t->tracing_graph_pause))) > + return; > + > + /* > + * Note: > + * No protection against faulting at *parent, which may be seen > + * on other archs. It's unlikely on AArch64. > + */ > + old = *parent;
Sorry to pick on this line again, but the comment is very non-committal] and I think this is something on which we need to be definitive. Either the access can fault, and we should handle it, or it will never fault and we don't need to handle it. Saying it's unlikely means we need to handle it :) Will
