On Tue, 10 Dec 2024 11:09:26 +0900 "Masami Hiramatsu (Google)" <mhiramat@xxxxxxxxxx> wrote: > From: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> > > Use ftrace_regs instead of fgraph_ret_regs for tracing return value > on function_graph tracer because of simplifying the callback interface. > > The CONFIG_HAVE_FUNCTION_GRAPH_RETVAL is also replaced by > CONFIG_HAVE_FUNCTION_GRAPH_FREGS. > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> > Acked-by: Heiko Carstens <hca@xxxxxxxxxxxxx> Did Heiko ack this? I don't see it in my inbox. > --- > Changes in v18: > - Use PTREGS_SIZE instead of redefining FRAME_SIZE on i386. > Changes in v17: > - Fixes s390 return_to_handler according to Heiko's advice. I see the comments, but I want to make sure Heiko is happy with this. > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig > index 0077969170e8..102029e56cf0 100644 > --- a/arch/s390/Kconfig > +++ b/arch/s390/Kconfig > @@ -192,7 +192,7 @@ config S390 > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_ARG_ACCESS_API > select HAVE_FUNCTION_ERROR_INJECTION > - select HAVE_FUNCTION_GRAPH_RETVAL > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h > index fc97d75dc752..5c94c1fc1bc1 100644 > --- a/arch/s390/include/asm/ftrace.h > +++ b/arch/s390/include/asm/ftrace.h > @@ -62,23 +62,6 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs * > return NULL; > } > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long gpr2; > - unsigned long fp; > -}; > - > -static __always_inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->gpr2; > -} > - > -static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > -#endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > - > static __always_inline void > ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > unsigned long ip) > @@ -86,6 +69,13 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > arch_ftrace_regs(fregs)->regs.psw.addr = ip; > } > > +#undef ftrace_regs_get_frame_pointer > +static __always_inline unsigned long > +ftrace_regs_get_frame_pointer(struct ftrace_regs *fregs) > +{ > + return ftrace_regs_get_stack_pointer(fregs); > +} > + > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > /* > * When an ftrace registered caller is tracing a function that is > diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c > index 862a9140528e..36709112ae7a 100644 > --- a/arch/s390/kernel/asm-offsets.c > +++ b/arch/s390/kernel/asm-offsets.c > @@ -175,12 +175,6 @@ int main(void) > DEFINE(OLDMEM_SIZE, PARMAREA + offsetof(struct parmarea, oldmem_size)); > DEFINE(COMMAND_LINE, PARMAREA + offsetof(struct parmarea, command_line)); > DEFINE(MAX_COMMAND_LINE_SIZE, PARMAREA + offsetof(struct parmarea, max_command_line_size)); > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > - /* function graph return value tracing */ > - OFFSET(__FGRAPH_RET_GPR2, fgraph_ret_regs, gpr2); > - OFFSET(__FGRAPH_RET_FP, fgraph_ret_regs, fp); > - DEFINE(__FGRAPH_RET_SIZE, sizeof(struct fgraph_ret_regs)); > -#endif > OFFSET(__FTRACE_REGS_PT_REGS, __arch_ftrace_regs, regs); > DEFINE(__FTRACE_REGS_SIZE, sizeof(struct __arch_ftrace_regs)); > > diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S > index 7e267ef63a7f..2b628aa3d809 100644 > --- a/arch/s390/kernel/mcount.S > +++ b/arch/s390/kernel/mcount.S > @@ -134,14 +134,14 @@ SYM_CODE_END(ftrace_common) > SYM_FUNC_START(return_to_handler) > stmg %r2,%r5,32(%r15) > lgr %r1,%r15 > - aghi %r15,-(STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE) > + # allocate ftrace_regs and stack frame for ftrace_return_to_handler > + aghi %r15,-STACK_FRAME_SIZE_FREGS > stg %r1,__SF_BACKCHAIN(%r15) > - la %r3,STACK_FRAME_OVERHEAD(%r15) > - stg %r1,__FGRAPH_RET_FP(%r3) > - stg %r2,__FGRAPH_RET_GPR2(%r3) > - lgr %r2,%r3 > + stg %r2,(STACK_FREGS_PTREGS_GPRS+2*8)(%r15) > + stg %r1,(STACK_FREGS_PTREGS_GPRS+15*8)(%r15) > + la %r2,STACK_FRAME_OVERHEAD(%r15) > brasl %r14,ftrace_return_to_handler > - aghi %r15,STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE > + aghi %r15,STACK_FRAME_SIZE_FREGS > lgr %r14,%r2 > lmg %r2,%r5,32(%r15) > BR_EX %r14 Heiko, Does the above look OK to you? Thanks, -- Steve