Messages in this thread | | | From | Guo Ren <> | Date | Thu, 17 Nov 2022 09:49:06 +0800 | Subject | Re: [PATCH v2 3/3] riscv/ftrace: cleanup ftrace_caller and ftrace_regs_caller |
| |
On Wed, Nov 16, 2022 at 11:13 AM Song Shuai <suagrfillet@gmail.com> wrote: > > ftrace_caller and ftrace_regs_caller save their regs with the respective > option of SAVE_ALL, then call the tracing function, especially graph_ops's > ftrace_graph_func. So the ftrace_graph_[regs]_call labels aren't needed > anymore if FTRACE_WITH_REGS is defined. > > If FTRACE_WITH_REGS isn't defined, the !FTRACE_WITH_REGS version > ftrace_caller remains with the ftrace_graph_call. So the enable/disable > helpers are revised for serving only this ftrace_graph_call. > > Signed-off-by: Song Shuai <suagrfillet@gmail.com> > --- > arch/riscv/kernel/ftrace.c | 19 ++---------------- > arch/riscv/kernel/mcount-dyn.S | 35 +++++++++++++++------------------- > 2 files changed, 17 insertions(+), 37 deletions(-) > > diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c > index 148a5480076b..2f0bcedc6a2d 100644 > --- a/arch/riscv/kernel/ftrace.c > +++ b/arch/riscv/kernel/ftrace.c > @@ -211,30 +211,15 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > extern void ftrace_graph_call(void); > -extern void ftrace_graph_regs_call(void); > int ftrace_enable_ftrace_graph_caller(void) > { > - int ret; > - > - ret = __ftrace_modify_call((unsigned long)&ftrace_graph_call, > - (unsigned long)&prepare_ftrace_return, true); > - if (ret) > - return ret; > - > - return __ftrace_modify_call((unsigned long)&ftrace_graph_regs_call, > + return __ftrace_modify_call((unsigned long)&ftrace_graph_call, > (unsigned long)&prepare_ftrace_return, true); > } > > int ftrace_disable_ftrace_graph_caller(void) > { > - int ret; > - > - ret = __ftrace_modify_call((unsigned long)&ftrace_graph_call, > - (unsigned long)&prepare_ftrace_return, false); > - if (ret) > - return ret; > - > - return __ftrace_modify_call((unsigned long)&ftrace_graph_regs_call, > + return __ftrace_modify_call((unsigned long)&ftrace_graph_call, > (unsigned long)&prepare_ftrace_return, false); > } > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S > index 2f0a280bd7a0..9e4097c6793d 100644 > --- a/arch/riscv/kernel/mcount-dyn.S > +++ b/arch/riscv/kernel/mcount-dyn.S > @@ -215,6 +215,7 @@ > .endm > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > > +#ifndef CONFIG_DYNAMIC_FTRACE_WITH_REGS > ENTRY(ftrace_caller) > SAVE_ABI > > @@ -243,33 +244,27 @@ ftrace_graph_call: > ret > ENDPROC(ftrace_caller) > > -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS > +#else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > ENTRY(ftrace_regs_caller) > - SAVE_ALL > - > - addi a0, ra, -FENTRY_RA_OFFSET > - la a1, function_trace_op > - REG_L a2, 0(a1) > - REG_L a1, PT_SIZE_ON_STACK(sp) > - mv a3, sp > + SAVE_ALL 1 I think the patch is broken, we shouldn't split mcount-dyn.S modification from the patch.
> > ftrace_regs_call: > .global ftrace_regs_call > call ftrace_stub > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > - addi a0, sp, PT_RA > - REG_L a1, PT_EPC(sp) > - addi a1, a1, -FENTRY_RA_OFFSET > -#ifdef HAVE_FUNCTION_GRAPH_FP_TEST > - mv a2, s0 > -#endif > -ftrace_graph_regs_call: > - .global ftrace_graph_regs_call > - call ftrace_stub > -#endif > > - RESTORE_ALL > + RESTORE_ALL 1 > ret > ENDPROC(ftrace_regs_caller) > + > +ENTRY(ftrace_caller) > + SAVE_ALL 0 > + > +ftrace_call: > + .global ftrace_call > + call ftrace_stub > + > + RESTORE_ALL 0 > + ret > +ENDPROC(ftrace_caller) > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ > -- > 2.20.1 >
-- Best Regards Guo Ren
| |