| Message ID | 20221116031305.286634-4-suagrfillet@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | riscv/ftrace: make function graph use ftrace directly | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/patch_count | success | Link |
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/build_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 86 lines checked |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
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
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 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 */
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(-)