[PATCH] mips/ftrace: Fix stack backtrace in unwind_stack_by_address()
Calling the unwind_stack_by_address() function for stack backtrace
will fail, when we use "echo function: stacktrace > set_ftrace_filter".
The stack backtrace as follows:
<...>-3102 [001] ...263.557737:
=> 0
=> 0
=> 0
=> 0
=> 0
=> 0
=> 0
=> 0
=>
-0 [000] .N.263.558793:
The reason is that when performing stack backtrace, the "ftrace_call"
and "ftrace_graph_call" global symbols in ftrace_caller() are
treated as functions.
If CONFIG_FUNCTION_GRAPH_TRACER is defined, the value in the "ra"
register is the address of ftrace_graph_call when the stack
backtrace back to ftrace_caller(). ”ftrace_graph_call“ is a global
symbol, and the value of "ofs" is set to zero when the
kallsyms_lookup_size_offset() is called. Otherwise, the value
in the "ra" register is the address of ftrace_call+8. "ftrace_call"
is the global symbol, and return one when the get_frame_info() is called.
Signed-off-by: YuanJunQing
---
arch/mips/kernel/process.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index b2a797557825..ac4fe79bc5bc 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -53,6 +53,8 @@
#include
#include
#include
+#include
+#include
#ifdef CONFIG_HOTPLUG_CPU
void arch_cpu_idle_dead(void)
@@ -569,6 +571,13 @@ unsigned long notrace unwind_stack_by_address(unsigned
long stack_page,
* Return ra if an exception occurred at the first instruction
*/
if (unlikely(ofs == 0)) {
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ extern void ftrace_graph_call(void);
+ if ((pc == (unsigned long)ftrace_graph_call)) {
+ pc = ((unsigned long *)(*sp))[PT_R31/sizeof(long)];
+ *sp += PT_SIZE;
+ } else
+#endif
pc = *ra;
*ra = 0;
return pc;
@@ -583,16 +592,23 @@ unsigned long notrace unwind_stack_by_address(unsigned
long stack_page,
if (*sp < low || *sp + info.frame_size > high)
return 0;
- if (leaf)
+ if (leaf) {
/*
* For some extreme cases, get_frame_info() can
* consider wrongly a nested function as a leaf
* one. In that cases avoid to return always the
* same value.
*/
+#ifdef CONFIG_DYNAMIC_FTRACE
+ if (info.func == (void *)ftrace_call) {
+ pc = ((unsigned long *)(*sp))[PT_R31/sizeof(long)];
+ info.frame_size = PT_SIZE;
+ } else
+#endif
pc = pc != *ra ? *ra : 0;
- else
+ } else {
pc = ((unsigned long *)(*sp))[info.pc_offset];
+ }
*sp += info.frame_size;
*ra = 0;
--
2.17.1
Re: [PATCH] function:stacktrace/mips: Fix function:stacktrace for mips
在 2020/6/4 上午9:17, Maciej W. Rozycki 写道: > On Fri, 29 May 2020, WANG Xuerui wrote: > >> On 2020/5/29 17:29, yuanjunqing wrote: >> >>>> diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S >>>> index cff52b283e03..cd5545764e5f 100644 >>>> --- a/arch/mips/kernel/mcount.S >>>> +++ b/arch/mips/kernel/mcount.S >>>> @@ -87,8 +87,15 @@ EXPORT_SYMBOL(_mcount) >>>>PTR_LA t1, _etext >>>>sltu t3, t1, a0 /* t3 = (a0 > _etext) */ >>>>or t1, t2, t3 >>>> + PTR_LA t2, stlab-4/* t2: "function:stacktrace" return address */ >>>> + move a1, AT /* arg2: parent's return address */ >>>>beqz t1, ftrace_call >>>> - nop >>>> + nop/* "function:stacktrace" return address */ >>>> +stlab: >>>> + PTR_LA t2, stlab-4 >>>> + /* ftrace_call_end: ftrace_call return address */ >>>> + beq t2,ra, ftrace_call_end >>>> + nop > Broken delay slot indentation. Thank you for your reply. For this question that you mentioned about the delay slot, I will modify my patch again. > >>>> #if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT) >>>>PTR_SUBU a0, a0, 16 /* arg1: adjust to module's recorded callsite */ >>>> #else >>>> @@ -98,7 +105,9 @@ EXPORT_SYMBOL(_mcount) >>>>.globl ftrace_call >>>> ftrace_call: >>>>nop /* a placeholder for the call to a real tracing function */ >>>> - move a1, AT /* arg2: parent's return address */ >>>> + movera, t2 /* t2: "function:stacktrace" return address */ > Likewise. NB I haven't investigated if the change makes sense. A more > detailed explanation in the change description is certainly needed. I will attach a specific description for further explanation about the second patch later. > > Maciej
Re: [PATCH] function:stacktrace/mips: Fix function:stacktrace for mips
May I ask if you have received this email. 在 2020/5/28 下午8:36, YuanJunQing 写道: > ftrace_call as global symbol in ftrace_caller(), this > will cause function:stacktrace can not work well. > i.e. echo do_IRQ:stacktrace > set_ftrace_filte > > Signed-off-by: YuanJunQing > --- > arch/mips/kernel/mcount.S | 13 +++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S > index cff52b283e03..cd5545764e5f 100644 > --- a/arch/mips/kernel/mcount.S > +++ b/arch/mips/kernel/mcount.S > @@ -87,8 +87,15 @@ EXPORT_SYMBOL(_mcount) > PTR_LA t1, _etext > sltu t3, t1, a0 /* t3 = (a0 > _etext) */ > or t1, t2, t3 > + PTR_LA t2, stlab-4/* t2: "function:stacktrace" return address */ > + move a1, AT /* arg2: parent's return address */ > beqz t1, ftrace_call > - nop > + nop/* "function:stacktrace" return address */ > +stlab: > + PTR_LA t2, stlab-4 > + /* ftrace_call_end: ftrace_call return address */ > + beq t2,ra, ftrace_call_end > + nop > #if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT) > PTR_SUBU a0, a0, 16 /* arg1: adjust to module's recorded callsite */ > #else > @@ -98,7 +105,9 @@ EXPORT_SYMBOL(_mcount) > .globl ftrace_call > ftrace_call: > nop /* a placeholder for the call to a real tracing function */ > - move a1, AT /* arg2: parent's return address */ > + movera, t2 /* t2: "function:stacktrace" return address */ > + > +ftrace_call_end: > > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > .globl ftrace_graph_call
Re: [PATCH] MIPS: Fix IRQ tracing when call handle_fpe() and handle_msa_fpe()
sorry! 在 2020/5/28 下午8:35, YuanJunQing 写道: > Register "a1" is unsaved in this function, > when CONFIG_TRACE_IRQFLAGS is enabled, > the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), > and this may change register "a1". > The changed register "a1" as argument will be send > to do_fpe() and do_msa_fpe(). > > Signed-off-by: YuanJunQing > --- > arch/mips/kernel/genex.S | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S > index 8236fb291e3f..a1b966f3578e 100644 > --- a/arch/mips/kernel/genex.S > +++ b/arch/mips/kernel/genex.S > @@ -476,20 +476,20 @@ NESTED(nmi_handler, PT_SIZE, sp) > .endm > > .macro __build_clear_fpe > + CLI > + TRACE_IRQS_OFF > .setpush > /* gas fails to assemble cfc1 for some archs (octeon).*/ \ > .setmips1 > SET_HARDFLOAT > cfc1a1, fcr31 > .setpop > - CLI > - TRACE_IRQS_OFF > .endm > > .macro __build_clear_msa_fpe > - _cfcmsa a1, MSA_CSR > CLI > TRACE_IRQS_OFF > + _cfcmsa a1, MSA_CSR > .endm > > .macro __build_clear_ade
[PATCH] function:stacktrace/mips: Fix function:stacktrace for mips
ftrace_call as global symbol in ftrace_caller(), this will cause function:stacktrace can not work well. i.e. echo do_IRQ:stacktrace > set_ftrace_filte Signed-off-by: YuanJunQing --- arch/mips/kernel/mcount.S | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S index cff52b283e03..cd5545764e5f 100644 --- a/arch/mips/kernel/mcount.S +++ b/arch/mips/kernel/mcount.S @@ -87,8 +87,15 @@ EXPORT_SYMBOL(_mcount) PTR_LA t1, _etext sltu t3, t1, a0 /* t3 = (a0 > _etext) */ or t1, t2, t3 + PTR_LA t2, stlab-4/* t2: "function:stacktrace" return address */ + move a1, AT /* arg2: parent's return address */ beqz t1, ftrace_call -nop +nop/* "function:stacktrace" return address */ +stlab: + PTR_LA t2, stlab-4 + /* ftrace_call_end: ftrace_call return address */ + beq t2,ra, ftrace_call_end + nop #if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT) PTR_SUBU a0, a0, 16 /* arg1: adjust to module's recorded callsite */ #else @@ -98,7 +105,9 @@ EXPORT_SYMBOL(_mcount) .globl ftrace_call ftrace_call: nop /* a placeholder for the call to a real tracing function */ -move a1, AT /* arg2: parent's return address */ + movera, t2 /* t2: "function:stacktrace" return address */ + +ftrace_call_end: #ifdef CONFIG_FUNCTION_GRAPH_TRACER .globl ftrace_graph_call -- 2.17.1
[PATCH] MIPS: Fix IRQ tracing when call handle_fpe() and handle_msa_fpe()
Register "a1" is unsaved in this function, when CONFIG_TRACE_IRQFLAGS is enabled, the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), and this may change register "a1". The changed register "a1" as argument will be send to do_fpe() and do_msa_fpe(). Signed-off-by: YuanJunQing --- arch/mips/kernel/genex.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S index 8236fb291e3f..a1b966f3578e 100644 --- a/arch/mips/kernel/genex.S +++ b/arch/mips/kernel/genex.S @@ -476,20 +476,20 @@ NESTED(nmi_handler, PT_SIZE, sp) .endm .macro __build_clear_fpe + CLI + TRACE_IRQS_OFF .setpush /* gas fails to assemble cfc1 for some archs (octeon).*/ \ .setmips1 SET_HARDFLOAT cfc1a1, fcr31 .setpop - CLI - TRACE_IRQS_OFF .endm .macro __build_clear_msa_fpe - _cfcmsa a1, MSA_CSR CLI TRACE_IRQS_OFF + _cfcmsa a1, MSA_CSR .endm .macro __build_clear_ade -- 2.17.1
[PATCH] MIPS: Fix IRQ tracing when call handle_fpe() and handle_msa_fpe()
Register "a1" is unsaved in this function, when CONFIG_TRACE_IRQFLAGS is enabled, the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), and this may change register "a1". The changed register "a1" as argument will be send to do_fpe() and do_msa_fpe(). Signed-off-by: YuanJunQing --- arch/mips/kernel/genex.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S index 8236fb291e3f..a1b966f3578e 100644 --- a/arch/mips/kernel/genex.S +++ b/arch/mips/kernel/genex.S @@ -476,20 +476,20 @@ NESTED(nmi_handler, PT_SIZE, sp) .endm .macro __build_clear_fpe + CLI + TRACE_IRQS_OFF .setpush /* gas fails to assemble cfc1 for some archs (octeon).*/ \ .setmips1 SET_HARDFLOAT cfc1a1, fcr31 .setpop - CLI - TRACE_IRQS_OFF .endm .macro __build_clear_msa_fpe - _cfcmsa a1, MSA_CSR CLI TRACE_IRQS_OFF + _cfcmsa a1, MSA_CSR .endm .macro __build_clear_ade -- 2.17.1
Re: [PATCH] MIPS: Fix IRQ tracing when call handle_fpe()
yes, I will re-send email for this patch. 在 2020/5/26 下午9:04, Thomas Bogendoerfer 写道: > On Tue, May 26, 2020 at 03:07:16PM +0800, yuanjunqing wrote: >> 在 2020/5/25 下午4:42, Thomas Bogendoerfer 写道: >>> On Mon, May 25, 2020 at 11:31:23AM +0800, YuanJunQing wrote: >>>> Register "a1" is unsaved in this function, >>>> when CONFIG_TRACE_IRQFLAGS is enabled, >>>> the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), >>>> and this may change register "a1". >>>> The variment of register "a1" may send SIGFPE signal >>>> to task when call do_fpe(),and this may kill the task. >>>> >>>> Signed-off-by: YuanJunQing >>>> --- >>>> arch/mips/kernel/genex.S | 6 -- >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S >>>> index 8236fb291e3f..956a76429773 100644 >>>> --- a/arch/mips/kernel/genex.S >>>> +++ b/arch/mips/kernel/genex.S >>>> @@ -480,16 +480,18 @@ NESTED(nmi_handler, PT_SIZE, sp) >>>>/* gas fails to assemble cfc1 for some archs (octeon).*/ \ >>>>.setmips1 >>>>SET_HARDFLOAT >>>> - cfc1a1, fcr31 >>>> + cfc1s0, fcr31 >>>>.setpop >>>>CLI >>>>TRACE_IRQS_OFF >>>> + movea1,s0 >>>>.endm >>> do we realy need to read fcr31 that early ? Wouldn't it work to >>> just move the cfc1 below TRACE_IRQS_OFF ? >>> >> yes, it can work when we just move the cfc1 below TRACE_IRQS_OFF, >> and the code is written as follows. >> >> CLI >> TRACE_IRQS_OFF >> .setmips1 >> SET_HARDFLOAT >> cfc1a1, fcr31 >> .setpop >> .endm > good, could we do the same with _cfcmsa a1, MSA_CSR in the msa case ? > > Thomas. >
Re: [PATCH] MIPS: Fix IRQ tracing when call handle_fpe()
在 2020/5/25 下午4:42, Thomas Bogendoerfer 写道: > On Mon, May 25, 2020 at 11:31:23AM +0800, YuanJunQing wrote: >> Register "a1" is unsaved in this function, >> when CONFIG_TRACE_IRQFLAGS is enabled, >> the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), >> and this may change register "a1". >> The variment of register "a1" may send SIGFPE signal >> to task when call do_fpe(),and this may kill the task. >> >> Signed-off-by: YuanJunQing >> --- >> arch/mips/kernel/genex.S | 6 -- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S >> index 8236fb291e3f..956a76429773 100644 >> --- a/arch/mips/kernel/genex.S >> +++ b/arch/mips/kernel/genex.S >> @@ -480,16 +480,18 @@ NESTED(nmi_handler, PT_SIZE, sp) >> /* gas fails to assemble cfc1 for some archs (octeon).*/ \ >> .setmips1 >> SET_HARDFLOAT >> -cfc1a1, fcr31 >> +cfc1s0, fcr31 >> .setpop >> CLI >> TRACE_IRQS_OFF >> +movea1,s0 >> .endm > do we realy need to read fcr31 that early ? Wouldn't it work to > just move the cfc1 below TRACE_IRQS_OFF ? > > Thomas. yes, it can work when we just move the cfc1 below TRACE_IRQS_OFF, and the code is written as follows. CLI TRACE_IRQS_OFF .setmips1 SET_HARDFLOAT cfc1a1, fcr31 .setpop .endm
[PATCH] MIPS: Fix IRQ tracing when call handle_fpe()
Register "a1" is unsaved in this function, when CONFIG_TRACE_IRQFLAGS is enabled, the TRACE_IRQS_OFF macro will call trace_hardirqs_off(), and this may change register "a1". The variment of register "a1" may send SIGFPE signal to task when call do_fpe(),and this may kill the task. Signed-off-by: YuanJunQing --- arch/mips/kernel/genex.S | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S index 8236fb291e3f..956a76429773 100644 --- a/arch/mips/kernel/genex.S +++ b/arch/mips/kernel/genex.S @@ -480,16 +480,18 @@ NESTED(nmi_handler, PT_SIZE, sp) /* gas fails to assemble cfc1 for some archs (octeon).*/ \ .setmips1 SET_HARDFLOAT - cfc1a1, fcr31 + cfc1s0, fcr31 .setpop CLI TRACE_IRQS_OFF + movea1,s0 .endm .macro __build_clear_msa_fpe - _cfcmsa a1, MSA_CSR + _cfcmsa s0, MSA_CSR CLI TRACE_IRQS_OFF + movea1,s0 .endm .macro __build_clear_ade -- 2.17.1
