ftrace/x86: function_graph stack dump fixes
Subsystems (14)
cover ftrace/x86: function_graph stack dump fixes
Some stack dump fixes related to function_graph tracing.
Josh Poimboeuf (8):
ftrace: remove CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST from config
ftrace: only allocate the ret_stack 'fp' field when needed
ftrace: add return address pointer to ftrace_ret_stack
ftrace: add ftrace_graph_ret_addr() stack unwinding helpers x86/dumpstack/ftrace: convert dump_trace() callbacks to use
ftrace_graph_ret_addr()
ftrace/x86: implement HAVE_FUNCTION_GRAPH_RET_ADDR_PTR x86/dumpstack/ftrace: mark function graph handler function as
unreliable
x86/dumpstack/ftrace: don't print unreliable addresses in
print_context_stack_bp()
Documentation/trace/ftrace-design.txt | 11 ++++++ arch/arm/kernel/ftrace.c | 2 +- arch/arm64/kernel/entry-ftrace.S | 2 +- arch/arm64/kernel/ftrace.c | 2 +- arch/blackfin/kernel/ftrace-entry.S | 4 +- arch/blackfin/kernel/ftrace.c | 2 +- arch/microblaze/kernel/ftrace.c | 2 +- arch/mips/kernel/ftrace.c | 4 +- arch/parisc/kernel/ftrace.c | 2 +- arch/powerpc/kernel/ftrace.c | 3 +- arch/s390/kernel/ftrace.c | 3 +- arch/sh/kernel/ftrace.c | 2 +- arch/sparc/Kconfig | 1 - arch/sparc/include/asm/ftrace.h | 4 ++ arch/sparc/kernel/ftrace.c | 2 +- arch/tile/kernel/ftrace.c | 2 +- arch/x86/Kconfig | 1 - arch/x86/include/asm/ftrace.h | 3 ++ arch/x86/kernel/dumpstack.c | 73 ++++++++++++++--------------------- arch/x86/kernel/ftrace.c | 2 +- include/linux/ftrace.h | 17 +++++++- kernel/trace/Kconfig | 5 --- kernel/trace/trace_functions_graph.c | 67 +++++++++++++++++++++++++++++++- 23 files changed, 146 insertions(+), 70 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/8 ftrace: remove CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST from config
Make HAVE_FUNCTION_GRAPH_FP_TEST a normal define, independent from kconfig. This removes some config file pollution and simplifies the checking for the fp test.
--- arch/arm64/kernel/entry-ftrace.S | 2 +- arch/blackfin/kernel/ftrace-entry.S | 4 ++-- arch/sparc/Kconfig | 1 - arch/sparc/include/asm/ftrace.h | 4 ++++ arch/x86/Kconfig | 1 - arch/x86/include/asm/ftrace.h | 1 + kernel/trace/Kconfig | 5 ----- kernel/trace/trace_functions_graph.c | 2 +- 8 files changed, 9 insertions(+), 11 deletions(-)
arch/arm64/kernel/entry-ftrace.S +1 -1
@@ -219,7 +219,7 @@ ENDPROC(ftrace_graph_caller) * * Run ftrace_return_to_handler() before going back to parent. * @fp is checked against the value passed by ftrace_graph_caller() - * only when CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST is enabled. + * only when HAVE_FUNCTION_GRAPH_FP_TEST is enabled. */ ENTRY(return_to_handler) save_return_regs
arch/blackfin/kernel/ftrace-entry.S +2 -2
@@ -169,7 +169,7 @@ ENTRY(_ftrace_graph_caller) r0 = sp; /* unsigned long *parent */ r1 = [sp]; /* unsigned long self_addr */ # endif -# ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST +# ifdef HAVE_FUNCTION_GRAPH_FP_TEST r2 = fp; /* unsigned long frame_pointer */ # endif r0 += 16; /* skip the 4 local regs on stack */
@@ -190,7 +190,7 @@ ENTRY(_return_to_handler) [--sp] = r1; /* get original return address */ -# ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST +# ifdef HAVE_FUNCTION_GRAPH_FP_TEST r0 = fp; /* Blackfin is sane, so omit this */ # endif call _ftrace_return_to_handler;
arch/sparc/Kconfig +0 -1
@@ -56,7 +56,6 @@ config SPARC64 def_bool 64BIT select HAVE_FUNCTION_TRACER select HAVE_FUNCTION_GRAPH_TRACER - select HAVE_FUNCTION_GRAPH_FP_TEST select HAVE_KRETPROBES select HAVE_KPROBES select HAVE_RCU_TABLE_FREE if SMP
arch/sparc/include/asm/ftrace.h +4 -0
@@ -9,6 +9,10 @@void _mcount (void );#endif +#endif /* CONFIG_MCOUNT */ + +#if defined(CONFIG_SPARC64 ) && !defined(CC_USE_FENTRY ) +#define HAVE_FUNCTION_GRAPH_FP_TEST #endif #ifdef CONFIG_DYNAMIC_FTRACE
arch/x86/Kconfig +0 -1
@@ -110,7 +110,6 @@ config X86 select HAVE_EXIT_THREAD select HAVE_FENTRY if X86_64 select HAVE_FTRACE_MCOUNT_RECORD - select HAVE_FUNCTION_GRAPH_FP_TEST select HAVE_FUNCTION_GRAPH_TRACER select HAVE_FUNCTION_TRACER select HAVE_GCC_PLUGINS
arch/x86/include/asm/ftrace.h +1 -0
@@ -6,6 +6,7 @@# define MCOUNT_ADDR ((unsigned long)(__fentry__))#else # define MCOUNT_ADDR ((unsigned long)(mcount)) +# define HAVE_FUNCTION_GRAPH_FP_TEST #endif #define MCOUNT_INSN_SIZE 5/* sizeof mcount call */
kernel/trace/Kconfig +0 -5
@@ -24,11 +24,6 @@ config HAVE_FUNCTION_GRAPH_TRACER help See Documentation/trace/ftrace-design.txt -config HAVE_FUNCTION_GRAPH_FP_TEST - bool - help - See Documentation/trace/ftrace-design.txt - config HAVE_DYNAMIC_FTRACE bool help
kernel/trace/trace_functions_graph.c +1 -1
@@ -204,7 +204,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,return ; } -#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST ) && !defined(CC_USING_FENTRY ) +#ifdef HAVE_FUNCTION_GRAPH_FP_TEST /* * The arch may choose to record the frame pointer used * and check it here to make sure that it is what we expect it
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] ftrace: Remove CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST from config
ftrace: Remove CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST from config
Commit-ID: e4a744ef2fef5c803348b650a3a2d01da7797a9b Gitweb: http://git.kernel.org/tip/e4a744ef2fef5c803348b650a3a2d01da7797a9b Author: Josh Poimboeuf <[email protected]> AuthorDate: Fri, 19 Aug 2016 06:52:55 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:15:13 +0200
ftrace: Remove CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST from config
Make HAVE_FUNCTION_GRAPH_FP_TEST a normal define, independent from kconfig. This removes some config file pollution and simplifies the checking for the fp test.
--- arch/arm64/kernel/entry-ftrace.S | 2 +- arch/blackfin/kernel/ftrace-entry.S | 4 ++-- arch/sparc/Kconfig | 1 - arch/sparc/include/asm/ftrace.h | 4 ++++ arch/x86/Kconfig | 1 - arch/x86/include/asm/ftrace.h | 1 + kernel/trace/Kconfig | 5 ----- kernel/trace/trace_functions_graph.c | 2 +- 8 files changed, 9 insertions(+), 11 deletions(-)
arch/arm64/kernel/entry-ftrace.S +1 -1
@@ -219,7 +219,7 @@ ENDPROC(ftrace_graph_caller) * * Run ftrace_return_to_handler() before going back to parent. * @fp is checked against the value passed by ftrace_graph_caller() - * only when CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST is enabled. + * only when HAVE_FUNCTION_GRAPH_FP_TEST is enabled. */ ENTRY(return_to_handler) save_return_regs
arch/blackfin/kernel/ftrace-entry.S +2 -2
@@ -169,7 +169,7 @@ ENTRY(_ftrace_graph_caller) r0 = sp; /* unsigned long *parent */ r1 = [sp]; /* unsigned long self_addr */ # endif -# ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST +# ifdef HAVE_FUNCTION_GRAPH_FP_TEST r2 = fp; /* unsigned long frame_pointer */ # endif r0 += 16; /* skip the 4 local regs on stack */
@@ -190,7 +190,7 @@ ENTRY(_return_to_handler) [--sp] = r1; /* get original return address */ -# ifdef CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST +# ifdef HAVE_FUNCTION_GRAPH_FP_TEST r0 = fp; /* Blackfin is sane, so omit this */ # endif call _ftrace_return_to_handler;
arch/sparc/Kconfig +0 -1
@@ -56,7 +56,6 @@ config SPARC64 def_bool 64BIT select HAVE_FUNCTION_TRACER select HAVE_FUNCTION_GRAPH_TRACER - select HAVE_FUNCTION_GRAPH_FP_TEST select HAVE_KRETPROBES select HAVE_KPROBES select HAVE_RCU_TABLE_FREE if SMP
arch/sparc/include/asm/ftrace.h +4 -0
@@ -9,6 +9,10 @@void _mcount (void );#endif +#endif /* CONFIG_MCOUNT */ + +#if defined(CONFIG_SPARC64 ) && !defined(CC_USE_FENTRY ) +#define HAVE_FUNCTION_GRAPH_FP_TEST #endif #ifdef CONFIG_DYNAMIC_FTRACE
arch/x86/Kconfig +0 -1
@@ -111,7 +111,6 @@ config X86 select HAVE_EXIT_THREAD select HAVE_FENTRY if X86_64 select HAVE_FTRACE_MCOUNT_RECORD - select HAVE_FUNCTION_GRAPH_FP_TEST select HAVE_FUNCTION_GRAPH_TRACER select HAVE_FUNCTION_TRACER select HAVE_GCC_PLUGINS
arch/x86/include/asm/ftrace.h +1 -0
@@ -6,6 +6,7 @@# define MCOUNT_ADDR ((unsigned long)(__fentry__))#else # define MCOUNT_ADDR ((unsigned long)(mcount)) +# define HAVE_FUNCTION_GRAPH_FP_TEST #endif #define MCOUNT_INSN_SIZE 5/* sizeof mcount call */
kernel/trace/Kconfig +0 -5
@@ -24,11 +24,6 @@ config HAVE_FUNCTION_GRAPH_TRACER help See Documentation/trace/ftrace-design.txt -config HAVE_FUNCTION_GRAPH_FP_TEST - bool - help - See Documentation/trace/ftrace-design.txt - config HAVE_DYNAMIC_FTRACE bool help
kernel/trace/trace_functions_graph.c +1 -1
@@ -204,7 +204,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,return ; } -#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST ) && !defined(CC_USING_FENTRY ) +#ifdef HAVE_FUNCTION_GRAPH_FP_TEST /* * The arch may choose to record the frame pointer used * and check it here to make sure that it is what we expect it
PATCH 2/8 ftrace: only allocate the ret_stack 'fp' field when needed
In reply to Josh PoimboeufThis saves some memory when HAVE_FUNCTION_GRAPH_FP_TEST isn't defined. On x86_64 with newer versions of gcc which have -mfentry, it saves 400 bytes per task.
--- include/linux/ftrace.h | 2 ++ kernel/trace/trace_functions_graph.c | 2 ++ 2 files changed, 4 insertions(+)
include/linux/ftrace.h +2 -0
@@ -795,7 +795,9 @@ struct ftrace_ret_stack {unsigned long func;unsigned long long calltime;unsigned long long subtime; +#ifdef HAVE_FUNCTION_GRAPH_FP_TEST unsigned long fp; +#endif };/*
kernel/trace/trace_functions_graph.c +2 -0
@@ -171,7 +171,9 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth, current->ret_stack[index].func = func; current->ret_stack[index].calltime = calltime; current->ret_stack[index].subtime =0 ; +#ifdef HAVE_FUNCTION_GRAPH_FP_TEST current->ret_stack[index].fp = frame_pointer; +#endif *depth = current->curr_ret_stack;return 0 ;
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] ftrace: Only allocate the ret_stack 'fp' field when needed
ftrace: Only allocate the ret_stack 'fp' field when needed
Commit-ID: daa460a88c09b26b68e8b017de589c217e901afb Gitweb: http://git.kernel.org/tip/daa460a88c09b26b68e8b017de589c217e901afb Author: Josh Poimboeuf <[email protected]> AuthorDate: Fri, 19 Aug 2016 06:52:56 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:15:14 +0200
ftrace: Only allocate the ret_stack 'fp' field when needed
This saves some memory when HAVE_FUNCTION_GRAPH_FP_TEST isn't defined. On x86_64 with newer versions of gcc which have -mfentry, it saves 400 bytes per task.
--- include/linux/ftrace.h | 2 ++ kernel/trace/trace_functions_graph.c | 2 ++ 2 files changed, 4 insertions(+)
include/linux/ftrace.h +2 -0
@@ -795,7 +795,9 @@ struct ftrace_ret_stack {unsigned long func;unsigned long long calltime;unsigned long long subtime; +#ifdef HAVE_FUNCTION_GRAPH_FP_TEST unsigned long fp; +#endif };/*
kernel/trace/trace_functions_graph.c +2 -0
@@ -171,7 +171,9 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth, current->ret_stack[index].func = func; current->ret_stack[index].calltime = calltime; current->ret_stack[index].subtime =0 ; +#ifdef HAVE_FUNCTION_GRAPH_FP_TEST current->ret_stack[index].fp = frame_pointer; +#endif *depth = current->curr_ret_stack;return 0 ;
PATCH 3/8 ftrace: add return address pointer to ftrace_ret_stack
In reply to Josh PoimboeufStoring this value will help prevent unwinders from getting out of sync with the function graph tracer ret_stack. Now instead of needing a stateful iterator, they can compare the return address pointer to find the right ret_stack entry.
Note that an array of 50 ftrace_ret_stack structs is allocated for every task. So when an arch implements this, it will add either 200 or 400 bytes of memory usage per task (depending on whether it's a 32-bit or 64-bit platform).
--- Documentation/trace/ftrace-design.txt | 11 +++++++++++ arch/arm/kernel/ftrace.c | 2 +- arch/arm64/kernel/ftrace.c | 2 +- arch/blackfin/kernel/ftrace.c | 2 +- arch/microblaze/kernel/ftrace.c | 2 +- arch/mips/kernel/ftrace.c | 4 ++-- arch/parisc/kernel/ftrace.c | 2 +- arch/powerpc/kernel/ftrace.c | 3 ++- arch/s390/kernel/ftrace.c | 3 ++- arch/sh/kernel/ftrace.c | 2 +- arch/sparc/kernel/ftrace.c | 2 +- arch/tile/kernel/ftrace.c | 2 +- arch/x86/kernel/ftrace.c | 2 +- include/linux/ftrace.h | 5 ++++- kernel/trace/trace_functions_graph.c | 5 ++++- 15 files changed, 34 insertions(+), 15 deletions(-)
Documentation/trace/ftrace-design.txt +11 -0
@@ -203,6 +203,17 @@ along to ftrace_push_return_trace() instead of a stub value of 0. Similarly, when you call ftrace_return_to_handler(), pass it the frame pointer. +HAVE_FUNCTION_GRAPH_RET_ADDR_PTR +-------------------------------- + +An arch may pass in a pointer to the return address on the stack. This +prevents potential stack unwinding issues where the unwinder gets out of +sync with ret_stack and the wrong addresses are reported by +ftrace_graph_ret_addr(). + +Adding support for it is easy: just define the macro in asm/ftrace.h and +pass the return address pointer as the 'retp' argument to +ftrace_push_return_trace(). HAVE_FTRACE_NMI_ENTER ---------------------
arch/arm/kernel/ftrace.c +1 -1
@@ -218,7 +218,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr, } err =ftrace_push_return_trace (old, self_addr, &trace.depth, - frame_pointer); + frame_pointer,NULL );if (err == -EBUSY ) { *parent = old;return ;
arch/arm64/kernel/ftrace.c +1 -1
@@ -138,7 +138,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,return ; err =ftrace_push_return_trace (old, self_addr, &trace.depth, - frame_pointer); + frame_pointer,NULL );if (err == -EBUSY )return ;else
arch/blackfin/kernel/ftrace.c +1 -1
@@ -107,7 +107,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,return ;if (ftrace_push_return_trace (*parent, self_addr, &trace.depth, - frame_pointer) == -EBUSY ) + frame_pointer,NULL ) == -EBUSY )return ; trace.func = self_addr;
arch/microblaze/kernel/ftrace.c +1 -1
@@ -63,7 +63,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)return ; } - err =ftrace_push_return_trace (old, self_addr, &trace.depth,0 ); + err =ftrace_push_return_trace (old, self_addr, &trace.depth,0 ,NULL );if (err == -EBUSY ) { *parent = old;return ;
arch/mips/kernel/ftrace.c +2 -2
@@ -382,8 +382,8 @@ void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra,if (unlikely (faulted)) goto out; -if (ftrace_push_return_trace (old_parent_ra, self_ra, &trace.depth, fp) - == -EBUSY ) { +if (ftrace_push_return_trace (old_parent_ra, self_ra, &trace.depth, fp, +NULL ) == -EBUSY ) { *parent_ra_addr = old_parent_ra;return ; }
arch/parisc/kernel/ftrace.c +1 -1
@@ -48,7 +48,7 @@ static void __hot prepare_ftrace_return(unsigned long *parent,return ;if (ftrace_push_return_trace (old, self_addr, &trace.depth, -0 ) == -EBUSY ) +0 ,NULL ) == -EBUSY )return ;/* activate parisc_return_to_handler() as return point */
arch/powerpc/kernel/ftrace.c +2 -1
@@ -593,7 +593,8 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip)if (!ftrace_graph_entry (&trace)) goto out; -if (ftrace_push_return_trace (parent, ip, &trace.depth,0 ) == -EBUSY ) +if (ftrace_push_return_trace (parent, ip, &trace.depth,0 , +NULL ) == -EBUSY ) goto out; parent = return_hooker;
arch/s390/kernel/ftrace.c +2 -1
@@ -209,7 +209,8 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip)/* Only trace if the calling function expects to. */ if (!ftrace_graph_entry (&trace)) goto out; -if (ftrace_push_return_trace (parent, ip, &trace.depth,0 ) == -EBUSY ) +if (ftrace_push_return_trace (parent, ip, &trace.depth,0 , +NULL ) == -EBUSY ) goto out; parent = (unsigned long ) return_to_handler; out:
arch/sh/kernel/ftrace.c +1 -1
@@ -382,7 +382,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)return ; } - err =ftrace_push_return_trace (old, self_addr, &trace.depth,0 ); + err =ftrace_push_return_trace (old, self_addr, &trace.depth,0 ,NULL );if (err == -EBUSY ) {__raw_writel (old, parent);return ;
arch/sparc/kernel/ftrace.c +1 -1
@@ -131,7 +131,7 @@ unsigned long prepare_ftrace_return(unsigned long parent,return parent +8UL ;if (ftrace_push_return_trace (parent, self_addr, &trace.depth, - frame_pointer) == -EBUSY ) + frame_pointer,NULL ) == -EBUSY )return parent +8UL ; trace.func = self_addr;
arch/tile/kernel/ftrace.c +1 -1
@@ -184,7 +184,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr, *parent = return_hooker; err =ftrace_push_return_trace (old, self_addr, &trace.depth, - frame_pointer); + frame_pointer,NULL );if (err == -EBUSY ) { *parent = old;return ;
arch/x86/kernel/ftrace.c +1 -1
@@ -1029,7 +1029,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, }if (ftrace_push_return_trace (old, self_addr, &trace.depth, - frame_pointer) == -EBUSY ) { + frame_pointer,NULL ) == -EBUSY ) { *parent = old;return ; }
include/linux/ftrace.h +4 -1
@@ -798,6 +798,9 @@ struct ftrace_ret_stack {#ifdef HAVE_FUNCTION_GRAPH_FP_TEST unsigned long fp;#endif +#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR +unsigned long *retp; +#endif };/*
@@ -809,7 +812,7 @@ extern void return_to_handler(void);extern int ftrace_push_return_trace (unsigned long ret,unsigned long func,int *depth, -unsigned long frame_pointer); +unsigned long frame_pointer,unsigned long *retp);/* * Sometimes we don't want to trace a function with the function
kernel/trace/trace_functions_graph.c +4 -1
@@ -119,7 +119,7 @@ print_graph_duration(struct trace_array *tr, unsigned long long duration,/* Add a function return address to the trace stack on thread info.*/ int ftrace_push_return_trace (unsigned long ret,unsigned long func,int *depth, -unsigned long frame_pointer) +unsigned long frame_pointer,unsigned long *retp) {unsigned long long calltime;int index;
@@ -174,6 +174,9 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth,#ifdef HAVE_FUNCTION_GRAPH_FP_TEST current->ret_stack[index].fp = frame_pointer;#endif +#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR + current->ret_stack[index].retp = retp; +#endif *depth = current->curr_ret_stack;return 0 ;
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] ftrace: Add return address pointer to ftrace_ret_stack
ftrace: Add return address pointer to ftrace_ret_stack
Commit-ID: 9a7c348ba6a46f6270d4fe49577649dad5664fe7 Gitweb: http://git.kernel.org/tip/9a7c348ba6a46f6270d4fe49577649dad5664fe7 Author: Josh Poimboeuf <[email protected]> AuthorDate: Fri, 19 Aug 2016 06:52:57 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:15:14 +0200
ftrace: Add return address pointer to ftrace_ret_stack
Storing this value will help prevent unwinders from getting out of sync with the function graph tracer ret_stack. Now instead of needing a stateful iterator, they can compare the return address pointer to find the right ret_stack entry.
Note that an array of 50 ftrace_ret_stack structs is allocated for every task. So when an arch implements this, it will add either 200 or 400 bytes of memory usage per task (depending on whether it's a 32-bit or 64-bit platform).
--- Documentation/trace/ftrace-design.txt | 11 +++++++++++ arch/arm/kernel/ftrace.c | 2 +- arch/arm64/kernel/ftrace.c | 2 +- arch/blackfin/kernel/ftrace.c | 2 +- arch/microblaze/kernel/ftrace.c | 2 +- arch/mips/kernel/ftrace.c | 4 ++-- arch/parisc/kernel/ftrace.c | 2 +- arch/powerpc/kernel/ftrace.c | 3 ++- arch/s390/kernel/ftrace.c | 3 ++- arch/sh/kernel/ftrace.c | 2 +- arch/sparc/kernel/ftrace.c | 2 +- arch/tile/kernel/ftrace.c | 2 +- arch/x86/kernel/ftrace.c | 2 +- include/linux/ftrace.h | 5 ++++- kernel/trace/trace_functions_graph.c | 5 ++++- 15 files changed, 34 insertions(+), 15 deletions(-)
Documentation/trace/ftrace-design.txt +11 -0
@@ -203,6 +203,17 @@ along to ftrace_push_return_trace() instead of a stub value of 0. Similarly, when you call ftrace_return_to_handler(), pass it the frame pointer. +HAVE_FUNCTION_GRAPH_RET_ADDR_PTR +-------------------------------- + +An arch may pass in a pointer to the return address on the stack. This +prevents potential stack unwinding issues where the unwinder gets out of +sync with ret_stack and the wrong addresses are reported by +ftrace_graph_ret_addr(). + +Adding support for it is easy: just define the macro in asm/ftrace.h and +pass the return address pointer as the 'retp' argument to +ftrace_push_return_trace(). HAVE_FTRACE_NMI_ENTER ---------------------
arch/arm/kernel/ftrace.c +1 -1
@@ -218,7 +218,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr, } err =ftrace_push_return_trace (old, self_addr, &trace.depth, - frame_pointer); + frame_pointer,NULL );if (err == -EBUSY ) { *parent = old;return ;
arch/arm64/kernel/ftrace.c +1 -1
@@ -138,7 +138,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,return ; err =ftrace_push_return_trace (old, self_addr, &trace.depth, - frame_pointer); + frame_pointer,NULL );if (err == -EBUSY )return ;else
arch/blackfin/kernel/ftrace.c +1 -1
@@ -107,7 +107,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,return ;if (ftrace_push_return_trace (*parent, self_addr, &trace.depth, - frame_pointer) == -EBUSY ) + frame_pointer,NULL ) == -EBUSY )return ; trace.func = self_addr;
arch/microblaze/kernel/ftrace.c +1 -1
@@ -63,7 +63,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)return ; } - err =ftrace_push_return_trace (old, self_addr, &trace.depth,0 ); + err =ftrace_push_return_trace (old, self_addr, &trace.depth,0 ,NULL );if (err == -EBUSY ) { *parent = old;return ;
arch/mips/kernel/ftrace.c +2 -2
@@ -382,8 +382,8 @@ void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra,if (unlikely (faulted)) goto out; -if (ftrace_push_return_trace (old_parent_ra, self_ra, &trace.depth, fp) - == -EBUSY ) { +if (ftrace_push_return_trace (old_parent_ra, self_ra, &trace.depth, fp, +NULL ) == -EBUSY ) { *parent_ra_addr = old_parent_ra;return ; }
arch/parisc/kernel/ftrace.c +1 -1
@@ -48,7 +48,7 @@ static void __hot prepare_ftrace_return(unsigned long *parent,return ;if (ftrace_push_return_trace (old, self_addr, &trace.depth, -0 ) == -EBUSY ) +0 ,NULL ) == -EBUSY )return ;/* activate parisc_return_to_handler() as return point */
arch/powerpc/kernel/ftrace.c +2 -1
@@ -593,7 +593,8 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip)if (!ftrace_graph_entry (&trace)) goto out; -if (ftrace_push_return_trace (parent, ip, &trace.depth,0 ) == -EBUSY ) +if (ftrace_push_return_trace (parent, ip, &trace.depth,0 , +NULL ) == -EBUSY ) goto out; parent = return_hooker;
arch/s390/kernel/ftrace.c +2 -1
@@ -209,7 +209,8 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip)/* Only trace if the calling function expects to. */ if (!ftrace_graph_entry (&trace)) goto out; -if (ftrace_push_return_trace (parent, ip, &trace.depth,0 ) == -EBUSY ) +if (ftrace_push_return_trace (parent, ip, &trace.depth,0 , +NULL ) == -EBUSY ) goto out; parent = (unsigned long ) return_to_handler; out:
arch/sh/kernel/ftrace.c +1 -1
@@ -382,7 +382,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)return ; } - err =ftrace_push_return_trace (old, self_addr, &trace.depth,0 ); + err =ftrace_push_return_trace (old, self_addr, &trace.depth,0 ,NULL );if (err == -EBUSY ) {__raw_writel (old, parent);return ;
arch/sparc/kernel/ftrace.c +1 -1
@@ -131,7 +131,7 @@ unsigned long prepare_ftrace_return(unsigned long parent,return parent +8UL ;if (ftrace_push_return_trace (parent, self_addr, &trace.depth, - frame_pointer) == -EBUSY ) + frame_pointer,NULL ) == -EBUSY )return parent +8UL ; trace.func = self_addr;
arch/tile/kernel/ftrace.c +1 -1
@@ -184,7 +184,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr, *parent = return_hooker; err =ftrace_push_return_trace (old, self_addr, &trace.depth, - frame_pointer); + frame_pointer,NULL );if (err == -EBUSY ) { *parent = old;return ;
arch/x86/kernel/ftrace.c +1 -1
@@ -1029,7 +1029,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, }if (ftrace_push_return_trace (old, self_addr, &trace.depth, - frame_pointer) == -EBUSY ) { + frame_pointer,NULL ) == -EBUSY ) { *parent = old;return ; }
include/linux/ftrace.h +4 -1
@@ -798,6 +798,9 @@ struct ftrace_ret_stack {#ifdef HAVE_FUNCTION_GRAPH_FP_TEST unsigned long fp;#endif +#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR +unsigned long *retp; +#endif };/*
@@ -809,7 +812,7 @@ extern void return_to_handler(void);extern int ftrace_push_return_trace (unsigned long ret,unsigned long func,int *depth, -unsigned long frame_pointer); +unsigned long frame_pointer,unsigned long *retp);/* * Sometimes we don't want to trace a function with the function
kernel/trace/trace_functions_graph.c +4 -1
@@ -119,7 +119,7 @@ print_graph_duration(struct trace_array *tr, unsigned long long duration,/* Add a function return address to the trace stack on thread info.*/ int ftrace_push_return_trace (unsigned long ret,unsigned long func,int *depth, -unsigned long frame_pointer) +unsigned long frame_pointer,unsigned long *retp) {unsigned long long calltime;int index;
@@ -174,6 +174,9 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth,#ifdef HAVE_FUNCTION_GRAPH_FP_TEST current->ret_stack[index].fp = frame_pointer;#endif +#ifdef HAVE_FUNCTION_GRAPH_RET_ADDR_PTR + current->ret_stack[index].retp = retp; +#endif *depth = current->curr_ret_stack;return 0 ;
PATCH 4/8 ftrace: add ftrace_graph_ret_addr() stack unwinding helpers
In reply to Josh PoimboeufWhen function graph tracing is enabled for a function, ftrace modifies the stack by replacing the original return address with the address of a hook function (return_to_handler).
Stack unwinders need a way to get the original return address. Add an arch-independent helper function for that named ftrace_graph_ret_addr().
This adds two variations of the function: one depends on HAVE_FUNCTION_GRAPH_RET_ADDR_PTR, and the other relies on an index state variable.
The former is recommended because, in some cases, the latter can cause problems when the unwinder skips stack frames. It can get out of sync with the ret_stack index and wrong addresses can be reported for the stack trace.
Once all arches have been ported to use HAVE_FUNCTION_GRAPH_RET_ADDR_PTR, we can get rid of the distinction.
--- include/linux/ftrace.h | 10 +++++++ kernel/trace/trace_functions_graph.c | 58 ++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+)
include/linux/ftrace.h +10 -0
@@ -814,6 +814,9 @@ extern int ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth, unsigned long frame_pointer, unsigned long *retp); +unsigned long ftrace_graph_ret_addr (struct task_struct *task,int *idx, +unsigned long ret,unsigned long *retp); +/* * Sometimes we don't want to trace a function with the function * graph tracer but we want them to keep traced by the usual function
@@ -875,6 +878,13 @@ static inline int task_curr_ret_stack(struct task_struct *tsk) return -1; } +static inline unsigned long +ftrace_graph_ret_addr (struct task_struct *task,int *idx,unsigned long ret, +unsigned long *retp) +{ +return ret; +} +static inline void pause_graph_tracing (void ) { }static inline void unpause_graph_tracing (void ) { }#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
kernel/trace/trace_functions_graph.c +58 -0
64 lines not on this page · raw message
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] ftrace: Add ftrace_graph_ret_addr() stack unwinding helpers
ftrace: Add ftrace_graph_ret_addr() stack unwinding helpers
Commit-ID: 223918e32a87c79ac55ca4aa513ba405ba4d57cd Gitweb: http://git.kernel.org/tip/223918e32a87c79ac55ca4aa513ba405ba4d57cd Author: Josh Poimboeuf <[email protected]> AuthorDate: Fri, 19 Aug 2016 06:52:58 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:15:14 +0200
ftrace: Add ftrace_graph_ret_addr() stack unwinding helpers
When function graph tracing is enabled for a function, ftrace modifies the stack by replacing the original return address with the address of a hook function (return_to_handler).
Stack unwinders need a way to get the original return address. Add an arch-independent helper function for that named ftrace_graph_ret_addr().
This adds two variations of the function: one depends on HAVE_FUNCTION_GRAPH_RET_ADDR_PTR, and the other relies on an index state variable.
The former is recommended because, in some cases, the latter can cause problems when the unwinder skips stack frames. It can get out of sync with the ret_stack index and wrong addresses can be reported for the stack trace.
Once all arches have been ported to use HAVE_FUNCTION_GRAPH_RET_ADDR_PTR, we can get rid of the distinction.
--- include/linux/ftrace.h | 10 +++++++ kernel/trace/trace_functions_graph.c | 58 ++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+)
include/linux/ftrace.h +10 -0
@@ -814,6 +814,9 @@ extern int ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth, unsigned long frame_pointer, unsigned long *retp); +unsigned long ftrace_graph_ret_addr (struct task_struct *task,int *idx, +unsigned long ret,unsigned long *retp); +/* * Sometimes we don't want to trace a function with the function * graph tracer but we want them to keep traced by the usual function
@@ -875,6 +878,13 @@ static inline int task_curr_ret_stack(struct task_struct *tsk) return -1; } +static inline unsigned long +ftrace_graph_ret_addr (struct task_struct *task,int *idx,unsigned long ret, +unsigned long *retp) +{ +return ret; +} +static inline void pause_graph_tracing (void ) { }static inline void unpause_graph_tracing (void ) { }#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
kernel/trace/trace_functions_graph.c +58 -0
64 lines not on this page · raw message
PATCH 5/8 x86/dumpstack/ftrace: convert dump_trace() callbacks to use ftrace_graph_ret_addr()
In reply to Josh PoimboeufConvert print_context_stack() and print_context_stack_bp() to use the arch-independent ftrace_graph_ret_addr() helper.
--- arch/x86/kernel/dumpstack.c | 65 +++++++++++++++------------------------------ 1 file changed, 22 insertions(+), 43 deletions(-)
arch/x86/kernel/dumpstack.c +22 -43
@@ -38,38 +38,6 @@ void printk_address(unsigned long address)pr_cont (" [<%p>] %pS\n" , (void *)address, (void *)address); } -#ifdef CONFIG_FUNCTION_GRAPH_TRACER -static void -print_ftrace_graph_addr (unsigned long addr,void *data, -const struct stacktrace_ops *ops, -struct task_struct *task,int *graph) -{ -unsigned long ret_addr; -int index; - -if (addr != (unsigned long )return_to_handler) -return ; - - index = task->curr_ret_stack; - -if (!task->ret_stack || index < *graph) -return ; - - index -= *graph; - ret_addr = task->ret_stack[index].ret; - - ops->address (data, ret_addr,1 ); - - (*graph)++; -} -#else -static inline void -print_ftrace_graph_addr (unsigned long addr,void *data, -const struct stacktrace_ops *ops, -struct task_struct *task,int *graph) -{ } -#endif -/* * x86-64 can have up to three kernel stacks: * process stack
@@ -107,18 +75,24 @@ print_context_stack(struct task_struct *task, stack = (unsigned long *)task_stack_page (task);while (valid_stack_ptr (task, stack,sizeof (*stack), end)) { -unsigned long addr; +unsigned long addr = *stack; - addr = *stack;if (__kernel_text_address (addr)) { +unsigned long real_addr; +int reliable =0 ; +if ((unsigned long ) stack == bp +sizeof (long )) { - ops->address (data, addr,1 ); + reliable =1 ; frame = frame->next_frame; bp = (unsigned long ) frame; - }else { - ops->address (data, addr,0 ); } -print_ftrace_graph_addr (addr, data, ops, task, graph); + + ops->address (data, addr, reliable); + + real_addr =ftrace_graph_ret_addr (task, graph, addr, + stack); +if (real_addr != addr) + ops->address (data, real_addr,1 ); } stack++; }
@@ -133,19 +107,24 @@ print_context_stack_bp(struct task_struct *task, unsigned long *end, int *graph) { struct stack_frame *frame = (struct stack_frame *)bp; -unsigned long *ret_addr = &frame->return_address; +unsigned long *retp = &frame->return_address; -while (valid_stack_ptr (task, ret_addr,sizeof (*ret_addr), end)) { -unsigned long addr = *ret_addr; +while (valid_stack_ptr (task, retp,sizeof (*retp), end)) { +unsigned long addr = *retp; +unsigned long real_addr;if (!__kernel_text_address (addr))break ;if (ops->address (data, addr,1 ))break ; + + real_addr =ftrace_graph_ret_addr (task, graph, addr, retp); +if (real_addr != addr) + ops->address (data, real_addr,1 ); + frame = frame->next_frame; - ret_addr = &frame->return_address; -print_ftrace_graph_addr (addr, data, ops, task, graph); + retp = &frame->return_address; }return (unsigned long )frame;
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] x86/dumpstack/ftrace: Convert dump_trace() callbacks to use ftrace_graph_ret_addr()
x86/dumpstack/ftrace: Convert dump_trace() callbacks to use ftrace_graph_ret_addr()
Commit-ID: 408fe5de2f2767059a9561e0ae6d4385d1b39dac Gitweb: http://git.kernel.org/tip/408fe5de2f2767059a9561e0ae6d4385d1b39dac Author: Josh Poimboeuf <[email protected]> AuthorDate: Fri, 19 Aug 2016 06:52:59 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:15:14 +0200
x86/dumpstack/ftrace: Convert dump_trace() callbacks to use ftrace_graph_ret_addr()
Convert print_context_stack() and print_context_stack_bp() to use the arch-independent ftrace_graph_ret_addr() helper.
--- arch/x86/kernel/dumpstack.c | 65 +++++++++++++++------------------------------ 1 file changed, 22 insertions(+), 43 deletions(-)
arch/x86/kernel/dumpstack.c +22 -43
@@ -38,38 +38,6 @@ void printk_address(unsigned long address)pr_cont (" [<%p>] %pS\n" , (void *)address, (void *)address); } -#ifdef CONFIG_FUNCTION_GRAPH_TRACER -static void -print_ftrace_graph_addr (unsigned long addr,void *data, -const struct stacktrace_ops *ops, -struct task_struct *task,int *graph) -{ -unsigned long ret_addr; -int index; - -if (addr != (unsigned long )return_to_handler) -return ; - - index = task->curr_ret_stack; - -if (!task->ret_stack || index < *graph) -return ; - - index -= *graph; - ret_addr = task->ret_stack[index].ret; - - ops->address (data, ret_addr,1 ); - - (*graph)++; -} -#else -static inline void -print_ftrace_graph_addr (unsigned long addr,void *data, -const struct stacktrace_ops *ops, -struct task_struct *task,int *graph) -{ } -#endif -/* * x86-64 can have up to three kernel stacks: * process stack
@@ -107,18 +75,24 @@ print_context_stack(struct task_struct *task, stack = (unsigned long *)task_stack_page (task);while (valid_stack_ptr (task, stack,sizeof (*stack), end)) { -unsigned long addr; +unsigned long addr = *stack; - addr = *stack;if (__kernel_text_address (addr)) { +unsigned long real_addr; +int reliable =0 ; +if ((unsigned long ) stack == bp +sizeof (long )) { - ops->address (data, addr,1 ); + reliable =1 ; frame = frame->next_frame; bp = (unsigned long ) frame; - }else { - ops->address (data, addr,0 ); } -print_ftrace_graph_addr (addr, data, ops, task, graph); + + ops->address (data, addr, reliable); + + real_addr =ftrace_graph_ret_addr (task, graph, addr, + stack); +if (real_addr != addr) + ops->address (data, real_addr,1 ); } stack++; }
@@ -133,19 +107,24 @@ print_context_stack_bp(struct task_struct *task, unsigned long *end, int *graph) { struct stack_frame *frame = (struct stack_frame *)bp; -unsigned long *ret_addr = &frame->return_address; +unsigned long *retp = &frame->return_address; -while (valid_stack_ptr (task, ret_addr,sizeof (*ret_addr), end)) { -unsigned long addr = *ret_addr; +while (valid_stack_ptr (task, retp,sizeof (*retp), end)) { +unsigned long addr = *retp; +unsigned long real_addr;if (!__kernel_text_address (addr))break ;if (ops->address (data, addr,1 ))break ; + + real_addr =ftrace_graph_ret_addr (task, graph, addr, retp); +if (real_addr != addr) + ops->address (data, real_addr,1 ); + frame = frame->next_frame; - ret_addr = &frame->return_address; -print_ftrace_graph_addr (addr, data, ops, task, graph); + retp = &frame->return_address; }return (unsigned long )frame;
PATCH 6/8 ftrace/x86: implement HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
In reply to Josh PoimboeufUse the more reliable version of ftrace_graph_ret_addr() so we no longer have to worry about the unwinder getting out of sync with the function graph ret_stack index, which can happen if the unwinder skips any frames before calling ftrace_graph_ret_addr().
This fixes this issue (and several others like it):
$ cat /proc/self/stack [<ffffffff810489a2>] save_stack_trace_tsk+0x22/0x40 [<ffffffff81311a89>] proc_pid_stack+0xb9/0x110 [<ffffffff813127c4>] proc_single_show+0x54/0x80 [<ffffffff812be088>] seq_read+0x108/0x3e0 [<ffffffff812923d7>] __vfs_read+0x37/0x140 [<ffffffff812929d9>] vfs_read+0x99/0x140 [<ffffffff81293f28>] SyS_read+0x58/0xc0 [<ffffffff818af97c>] entry_SYSCALL_64_fastpath+0x1f/0xbd [<ffffffffffffffff>] 0xffffffffffffffff
$ echo function_graph > /sys/kernel/debug/tracing/current_tracer
$ cat /proc/self/stack [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff810394cc>] print_context_stack+0xfc/0x100 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff8103891b>] dump_trace+0x12b/0x350 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff810489a2>] save_stack_trace_tsk+0x22/0x40 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff81311a89>] proc_pid_stack+0xb9/0x110 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff813127c4>] proc_single_show+0x54/0x80 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff812be088>] seq_read+0x108/0x3e0 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff812923d7>] __vfs_read+0x37/0x140 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff812929d9>] vfs_read+0x99/0x140 [<ffffffffffffffff>] 0xffffffffffffffff
Enabling function graph tracing causes the stack trace to change in two ways:
First, the real call addresses are confusingly interspersed with 'return_to_handler' addresses. This issue will be fixed by the next patch.
Second, the stack trace is offset by two frames, because the unwinder skipped the first two frames and got out of sync with the ret_stack index. This patch fixes this issue.
--- arch/x86/include/asm/ftrace.h | 2 ++ arch/x86/kernel/ftrace.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)
arch/x86/include/asm/ftrace.h +2 -0
@@ -14,6 +14,8 @@#define ARCH_SUPPORTS_FTRACE_OPS 1#endif +#define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR +#ifndef __ASSEMBLY__extern void mcount (void );extern atomic_t modifying_ftrace_code;
arch/x86/kernel/ftrace.c +1 -1
@@ -1029,7 +1029,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, }if (ftrace_push_return_trace (old, self_addr, &trace.depth, - frame_pointer,NULL ) == -EBUSY ) { + frame_pointer, parent) == -EBUSY ) { *parent = old;return ; }
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] ftrace/x86: Implement HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
ftrace/x86: Implement HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
Commit-ID: 471bd10f5e2880bd91a2627d887f6062494cfe9c Gitweb: http://git.kernel.org/tip/471bd10f5e2880bd91a2627d887f6062494cfe9c Author: Josh Poimboeuf <[email protected]> AuthorDate: Fri, 19 Aug 2016 06:53:00 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:15:15 +0200
ftrace/x86: Implement HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
Use the more reliable version of ftrace_graph_ret_addr() so we no longer have to worry about the unwinder getting out of sync with the function graph ret_stack index, which can happen if the unwinder skips any frames before calling ftrace_graph_ret_addr().
This fixes this issue (and several others like it):
$ cat /proc/self/stack [<ffffffff810489a2>] save_stack_trace_tsk+0x22/0x40 [<ffffffff81311a89>] proc_pid_stack+0xb9/0x110 [<ffffffff813127c4>] proc_single_show+0x54/0x80 [<ffffffff812be088>] seq_read+0x108/0x3e0 [<ffffffff812923d7>] __vfs_read+0x37/0x140 [<ffffffff812929d9>] vfs_read+0x99/0x140 [<ffffffff81293f28>] SyS_read+0x58/0xc0 [<ffffffff818af97c>] entry_SYSCALL_64_fastpath+0x1f/0xbd [<ffffffffffffffff>] 0xffffffffffffffff
$ echo function_graph > /sys/kernel/debug/tracing/current_tracer
$ cat /proc/self/stack [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff810394cc>] print_context_stack+0xfc/0x100 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff8103891b>] dump_trace+0x12b/0x350 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff810489a2>] save_stack_trace_tsk+0x22/0x40 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff81311a89>] proc_pid_stack+0xb9/0x110 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff813127c4>] proc_single_show+0x54/0x80 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff812be088>] seq_read+0x108/0x3e0 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff812923d7>] __vfs_read+0x37/0x140 [<ffffffff818b2428>] return_to_handler+0x0/0x27 [<ffffffff812929d9>] vfs_read+0x99/0x140 [<ffffffffffffffff>] 0xffffffffffffffff
Enabling function graph tracing causes the stack trace to change in two ways:
First, the real call addresses are confusingly interspersed with 'return_to_handler' addresses. This issue will be fixed by the next patch.
Second, the stack trace is offset by two frames, because the unwinder skipped the first two frames and got out of sync with the ret_stack index. This patch fixes this issue.
--- arch/x86/include/asm/ftrace.h | 2 ++ arch/x86/kernel/ftrace.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)
arch/x86/include/asm/ftrace.h +2 -0
@@ -14,6 +14,8 @@#define ARCH_SUPPORTS_FTRACE_OPS 1#endif +#define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR +#ifndef __ASSEMBLY__extern void mcount (void );extern atomic_t modifying_ftrace_code;
arch/x86/kernel/ftrace.c +1 -1
@@ -1029,7 +1029,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, }if (ftrace_push_return_trace (old, self_addr, &trace.depth, - frame_pointer,NULL ) == -EBUSY ) { + frame_pointer, parent) == -EBUSY ) { *parent = old;return ; }
PATCH 7/8 x86/dumpstack/ftrace: mark function graph handler function as unreliable
In reply to Josh PoimboeufWhen function graph tracing is enabled for a function, its return address on the stack is replaced with the address of an ftrace handler (return_to_handler).
Currently 'return_to_handler' can be reported as reliable. That's not ideal, and can actually be misleading. When saving or dumping the stack, you normally only care about what led up to that point (the call path), rather than what will happen in the future (the return path).
That's especially true in the non-oops stack trace case, which isn't used for debugging. For example, in a perf profiling operation, reporting return_to_handler() in the trace would just be confusing.
And in the oops case, where debugging is important, "unreliable" is also more appropriate there because it serves as a hint that graph tracing was involved, instead of trying to imply that return_to_handler() was the real caller.
--- arch/x86/kernel/dumpstack.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)
arch/x86/kernel/dumpstack.c +16 -8
@@ -87,12 +87,21 @@ print_context_stack(struct task_struct *task, bp = (unsigned long ) frame; } - ops->address (data, addr, reliable); - +/* +* When function graph tracing is enabled for a +* function, its return address on the stack is +* replaced with the address of an ftrace handler +* (return_to_handler). In that case, before printing +* the "real" address, we want to print the handler +* address as an "unreliable" hint that function graph +* tracing was involved. +*/ real_addr =ftrace_graph_ret_addr (task, graph, addr, stack);if (real_addr != addr) - ops->address (data, real_addr,1 ); + ops->address (data, addr,0 ); + + ops->address (data, real_addr, reliable); } stack++; }
@@ -116,12 +125,11 @@ print_context_stack_bp(struct task_struct *task,if (!__kernel_text_address (addr))break ; -if (ops->address (data, addr,1 )) -break ; - real_addr =ftrace_graph_ret_addr (task, graph, addr, retp); -if (real_addr != addr) - ops->address (data, real_addr,1 ); +if (real_addr != addr && ops->address (data, addr,0 )) +break ; +if (ops->address (data, real_addr,1 )) +break ; frame = frame->next_frame; retp = &frame->return_address;
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] x86/dumpstack/ftrace: Mark function graph handler function as unreliable
x86/dumpstack/ftrace: Mark function graph handler function as unreliable
Commit-ID: 6f727b84e23421721025f4eb1b4f6cea1d4d723a Gitweb: http://git.kernel.org/tip/6f727b84e23421721025f4eb1b4f6cea1d4d723a Author: Josh Poimboeuf <[email protected]> AuthorDate: Fri, 19 Aug 2016 06:53:01 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:15:15 +0200
x86/dumpstack/ftrace: Mark function graph handler function as unreliable
When function graph tracing is enabled for a function, its return address on the stack is replaced with the address of an ftrace handler (return_to_handler).
Currently 'return_to_handler' can be reported as reliable. That's not ideal, and can actually be misleading. When saving or dumping the stack, you normally only care about what led up to that point (the call path), rather than what will happen in the future (the return path).
That's especially true in the non-oops stack trace case, which isn't used for debugging. For example, in a perf profiling operation, reporting return_to_handler() in the trace would just be confusing.
And in the oops case, where debugging is important, "unreliable" is also more appropriate there because it serves as a hint that graph tracing was involved, instead of trying to imply that return_to_handler() was the real caller.
--- arch/x86/kernel/dumpstack.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)
arch/x86/kernel/dumpstack.c +16 -8
@@ -87,12 +87,21 @@ print_context_stack(struct task_struct *task, bp = (unsigned long ) frame; } - ops->address (data, addr, reliable); - +/* +* When function graph tracing is enabled for a +* function, its return address on the stack is +* replaced with the address of an ftrace handler +* (return_to_handler). In that case, before printing +* the "real" address, we want to print the handler +* address as an "unreliable" hint that function graph +* tracing was involved. +*/ real_addr =ftrace_graph_ret_addr (task, graph, addr, stack);if (real_addr != addr) - ops->address (data, real_addr,1 ); + ops->address (data, addr,0 ); + + ops->address (data, real_addr, reliable); } stack++; }
@@ -116,12 +125,11 @@ print_context_stack_bp(struct task_struct *task,if (!__kernel_text_address (addr))break ; -if (ops->address (data, addr,1 )) -break ; - real_addr =ftrace_graph_ret_addr (task, graph, addr, retp); -if (real_addr != addr) - ops->address (data, real_addr,1 ); +if (real_addr != addr && ops->address (data, addr,0 )) +break ; +if (ops->address (data, real_addr,1 )) +break ; frame = frame->next_frame; retp = &frame->return_address;
PATCH 8/8 x86/dumpstack/ftrace: don't print unreliable addresses in print_context_stack_bp()
In reply to Josh PoimboeufWhen function graph tracing is enabled, print_context_stack_bp() can report return_to_handler() as an unreliable address, which is confusing and misleading: return_to_handler() is really only useful as a hint for debugging, whereas print_context_stack_bp() users only care about the actual 'reliable' call path.
--- arch/x86/kernel/dumpstack.c | 2 -- 1 file changed, 2 deletions(-)
arch/x86/kernel/dumpstack.c +0 -2
@@ -126,8 +126,6 @@ print_context_stack_bp(struct task_struct *task,break ; real_addr =ftrace_graph_ret_addr (task, graph, addr, retp); -if (real_addr != addr && ops->address (data, addr,0 )) -break ;if (ops->address (data, real_addr,1 ))break ;
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] x86/dumpstack/ftrace: Don't print unreliable addresses in print_context_stack_bp()
x86/dumpstack/ftrace: Don't print unreliable addresses in print_context_stack_bp()
Commit-ID: 13e25bab7e51bdd4ba7df1ef2388961294bb565e Gitweb: http://git.kernel.org/tip/13e25bab7e51bdd4ba7df1ef2388961294bb565e Author: Josh Poimboeuf <[email protected]> AuthorDate: Fri, 19 Aug 2016 06:53:02 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 24 Aug 2016 12:15:15 +0200
x86/dumpstack/ftrace: Don't print unreliable addresses in print_context_stack_bp()
When function graph tracing is enabled, print_context_stack_bp() can report return_to_handler() as an unreliable address, which is confusing and misleading: return_to_handler() is really only useful as a hint for debugging, whereas print_context_stack_bp() users only care about the actual 'reliable' call path.
--- arch/x86/kernel/dumpstack.c | 2 -- 1 file changed, 2 deletions(-)
arch/x86/kernel/dumpstack.c +0 -2
@@ -126,8 +126,6 @@ print_context_stack_bp(struct task_struct *task,break ; real_addr =ftrace_graph_ret_addr (task, graph, addr, retp); -if (real_addr != addr && ops->address (data, addr,0 )) -break ;if (ops->address (data, real_addr,1 ))break ;
Quoted 14 lines · On Fri, 19 Aug 2016 06:52:54 -0500 Josh Poimboeuf <[email protected]> wrote:
Some stack dump fixes related to
function_graphtracing.Josh Poimboeuf (8):
ftrace: removeCONFIG_HAVE_FUNCTION_GRAPH_FP_TESTfrom config
ftrace: only allocate theret_stack'fp' field when needed
ftrace: add return address pointer toftrace_ret_stack
ftrace: addftrace_graph_ret_addr()stack unwinding helpers x86/dumpstack/ftrace: convertdump_trace()callbacks to use
ftrace_graph_ret_addr()
ftrace/x86: implementHAVE_FUNCTION_GRAPH_RET_ADDR_PTRx86/dumpstack/ftrace: mark function graph handler function as
unreliable
x86/dumpstack/ftrace: don't print unreliable addresses in
print_context_stack_bp()
For the series..
Ingo,
If you want, please take it through tip.
Thanks!
-- Steve
Quoted 26 lines
Documentation/trace/ftrace-design.txt | 11 ++++++ arch/arm/kernel/ftrace.c | 2 +- arch/arm64/kernel/entry-ftrace.S | 2 +- arch/arm64/kernel/ftrace.c | 2 +- arch/blackfin/kernel/ftrace-entry.S | 4 +- arch/blackfin/kernel/ftrace.c | 2 +- arch/microblaze/kernel/ftrace.c | 2 +- arch/mips/kernel/ftrace.c | 4 +- arch/parisc/kernel/ftrace.c | 2 +- arch/powerpc/kernel/ftrace.c | 3 +- arch/s390/kernel/ftrace.c | 3 +- arch/sh/kernel/ftrace.c | 2 +- arch/sparc/Kconfig | 1 - arch/sparc/include/asm/ftrace.h | 4 ++ arch/sparc/kernel/ftrace.c | 2 +- arch/tile/kernel/ftrace.c | 2 +- arch/x86/Kconfig | 1 - arch/x86/include/asm/ftrace.h | 3 ++ arch/x86/kernel/dumpstack.c | 73 ++++++++++++++--------------------- arch/x86/kernel/ftrace.c | 2 +- include/linux/ftrace.h | 17 +++++++- kernel/trace/Kconfig | 5 --- kernel/trace/trace_functions_graph.c | 67 +++++++++++++++++++++++++++++++- 23 files changed, 146 insertions(+), 70 deletions(-)
Quoted 18 lines · Steven Rostedt
Quoted 14 lines · On Fri, 19 Aug 2016 06:52:54 -0500 Josh Poimboeuf <[email protected]> wrote:
Some stack dump fixes related to
function_graphtracing.Josh Poimboeuf (8):
ftrace: removeCONFIG_HAVE_FUNCTION_GRAPH_FP_TESTfrom config
ftrace: only allocate theret_stack'fp' field when needed
ftrace: add return address pointer toftrace_ret_stack
ftrace: addftrace_graph_ret_addr()stack unwinding helpers x86/dumpstack/ftrace: convertdump_trace()callbacks to use
ftrace_graph_ret_addr()
ftrace/x86: implementHAVE_FUNCTION_GRAPH_RET_ADDR_PTRx86/dumpstack/ftrace: mark function graph handler function as
unreliable
x86/dumpstack/ftrace: don't print unreliable addresses in
print_context_stack_bp()
Quoted 7 lines
For the series..
Acked-by Steven Rostedt <[email protected]>Ingo,
If you want, please take it through tip.
Thanks Steve!
Ingo