Skip to contentSkip to conversation

arm64:ftrace: add save_stack_trace_regs()

PATCH Superseded Pratyush Anand 4 participants 5 messages Sep 1, 2016 – Sep 2, 2016 arm64

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Pratyush Anand

author
#0 lore raw

PATCH v2 1/1 arm64:ftrace: add save_stack_trace_regs()

Implement save_stack_trace_regs, so that stacktrace of a kprobe events can be obtained.

Signed-off-by Pratyush Anand <[email protected]>

---
V1 to V2: Rebased to latest kernel.

I see following stack trace for kprobe at xhci_irq():

echo stacktrace > /sys/kernel/debug/tracing/trace_options echo "p xhci_irq" > /sys/kernel/debug/tracing/kprobe_events echo 1 > /sys/kernel/debug/tracing/events/kprobes/enable more /sys/kernel/debug/tracing/trace

          <idle>-0     [004] d.h.  1356.000496: p_xhci_irq_0:(xhci_irq+0x0/0x9ac)
          <idle>-0     [004] d.h.  1356.000497: <stack trace>

=> xhci_irq
=> __handle_irq_event_percpu
=> handle_irq_event_percpu
=> handle_irq_event
=> handle_fasteoi_irq
=> generic_handle_irq
=> __handle_domain_irq
=> gic_handle_irq
=> el1_irq
=> arch_cpu_idle
=> default_idle_call
=> cpu_startup_entry
=> secondary_start_kernel
=>

 arch/arm64/kernel/stacktrace.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
arch/arm64/kernel/stacktrace.c +18 -0
index d9751a4769e7..bd6d9d8087f0 100644
@@ -152,6 +152,24 @@ static int save_trace(struct stackframe *frame, void *d)
 	return trace->nr_entries >= trace->max_entries;
 }
 
+void save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)
+{
+	struct stack_trace_data data;
+	struct stackframe frame;
+
+	data.trace = trace;
+	data.skip = trace->skip;
+	data.no_sched_functions = 0;
+
+	frame.fp = regs->regs[29];
+	frame.sp = regs->sp;
+	frame.pc = regs->pc;
+
+	walk_stackframe(current, &frame, save_trace, &data);
+	if (trace->nr_entries < trace->max_entries)
+		trace->entries[trace->nr_entries++] = ULONG_MAX;
+}
+
 void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 {
 	struct stack_trace_data data;
Signature · 1 line
2.7.4
#1 lore raw

PATCH v2 1/1 arm64:ftrace: add save_stack_trace_regs()

Quoted 44 lines · Pratyush Anand

Implement save_stack_trace_regs, so that stacktrace of a kprobe events can be obtained.

Signed-off-by Pratyush Anand <[email protected]>

---
V1 to V2: Rebased to latest kernel.

I see following stack trace for kprobe at xhci_irq():

echo stacktrace > /sys/kernel/debug/tracing/trace_options echo "p xhci_irq" > /sys/kernel/debug/tracing/kprobe_events echo 1 > /sys/kernel/debug/tracing/events/kprobes/enable more /sys/kernel/debug/tracing/trace

           <idle>-0     [004] d.h.  1356.000496: p_xhci_irq_0:(xhci_irq+0x0/0x9ac)
           <idle>-0     [004] d.h.  1356.000497: <stack trace>

=> xhci_irq
=> __handle_irq_event_percpu
=> handle_irq_event_percpu
=> handle_irq_event
=> handle_fasteoi_irq
=> generic_handle_irq
=> __handle_domain_irq
=> gic_handle_irq
=> el1_irq
=> arch_cpu_idle
=> default_idle_call
=> cpu_startup_entry
=> secondary_start_kernel
=>

  arch/arm64/kernel/stacktrace.c | 18 ++++++++++++++++++
  1 file changed, 18 insertions(+)
arch/arm64/kernel/stacktrace.c +2 -0
index d9751a4769e7..bd6d9d8087f0 100644
@@ -152,6 +152,24 @@ static int save_trace(struct stackframe *frame, void *d)
  	return trace->nr_entries >= trace->max_entries;
  }
 
+void save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)
+{

arch/arm64/kernel/stacktrace.c static int save_trace(struct stackframe *frame, void *d) · line 166

Quoted 10 lines
arch/arm64/kernel/stacktrace.c +10 -0
+	struct stack_trace_data data;
+	struct stackframe frame;
+
+	data.trace = trace;
+	data.skip = trace->skip;
+	data.no_sched_functions = 0;
+
+	frame.fp = regs->regs[29];
+	frame.sp = regs->sp;
+	frame.pc = regs->pc;

Interesting that arm64 does not define an "lr" field in struct stackframe like 32-bit arm does.

arch/arm64/kernel/stacktrace.c static int save_trace(struct stackframe *frame, void *d) · line 175

Quoted 10 lines
arch/arm64/kernel/stacktrace.c +6 -0
+
+	walk_stackframe(current, &frame, save_trace, &data);
+	if (trace->nr_entries < trace->max_entries)
+		trace->entries[trace->nr_entries++] = ULONG_MAX;
+}
+
  void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
  {
  	struct stack_trace_data data;

Looks OK to me. I successfully (if briefly) tested it with v4.8-rc4 on hikey.

Thanks,
-dl

#2 lore raw

PATCH v2 1/1 arm64:ftrace: add save_stack_trace_regs()

In reply to Pratyush Anand

Hi Pratyush,

Quoted 2 lines · Pratyush Anand

Implement save_stack_trace_regs, so that stacktrace of a kprobe events can be obtained.

Hmm, kconfig enables 'CONFIG_KPROBE_EVENT' if we select KPROBES and FTRACE, this calls save_stack_trace_regs(), which we haven't implemented, giving:

[  373.406982] save_stack_trace_regs() not implemented yet.
[  373.413020] ------------[ cut here ]------------
[  373.417601] WARNING: CPU: 1 PID: 0 at ../kernel/stacktrace.c:74 save_stack_tr
ace_regs+0x3c/0x48
[  373.426212] Modules linked in:
[  373.429238]
[  373.430716] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0-rc4-dirty #5128
[  373.437606] Hardware name: ARM Juno development board (r1) (DT)
[  373.443466] task: ffff800975dd1900 task.stack: ffff800975ddc000
[  373.449327] PC is at save_stack_trace_regs+0x3c/0x48
[  373.454242] LR is at save_stack_trace_regs+0x3c/0x48
[  373.459156] pc : [<ffff000008126c64>] lr : [<ffff000008126c64>] pstate: 600003c5
[  373.466476] sp : ffff80097ef52c00
[  373.554836] Call trace:
[  373.641153] [<ffff000008126c64>] save_stack_trace_regs+0x3c/0x48
[  373.647104] [<ffff00000817a0b0>] __ftrace_trace_stack+0x168/0x208
[  373.653138] [<ffff00000817ad40>] trace_buffer_unlock_commit_regs+0x5c/0x7c
[  373.659946] [<ffff00000818d57c>] kprobe_trace_func+0x308/0x3d8
[  373.665721] [<ffff00000818defc>] kprobe_dispatcher+0x58/0x60
[  373.671327] [<ffff0000088d8cfc>] kprobe_breakpoint_handler+0xbc/0x18c
[  373.677705] [<ffff0000080851b8>] brk_handler+0x50/0x90
[  373.682792] [<ffff000008081300>] do_debug_exception+0x50/0xbc

I guess hitting a WARN_ONCE() while using a 'kernel hacking' feature isn't serious enough to make this a fix.

Quoted 10 lines
arch/arm64/kernel/stacktrace.c +2 -0
index d9751a4769e7..bd6d9d8087f0 100644
@@ -152,6 +152,24 @@ static int save_trace(struct stackframe *frame, void *d)
 	return trace->nr_entries >= trace->max_entries;
 }
 
+void save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)
+{

arch/arm64/kernel/stacktrace.c static int save_trace(struct stackframe *frame, void *d) · line 166

Quoted 10 lines
arch/arm64/kernel/stacktrace.c +10 -0
+	struct stack_trace_data data;
+	struct stackframe frame;
+
+	data.trace = trace;
+	data.skip = trace->skip;
+	data.no_sched_functions = 0;
+
+	frame.fp = regs->regs[29];
+	frame.sp = regs->sp;
+	frame.pc = regs->pc;

What happens when a kprobe-event runs over a frame the function graph tracer has meddled with? You need:

#ifdef CONFIG_FUNCTION_GRAPH_TRACER
	frame.graph = current->curr_ret_stack;

#endif

arch/arm64/kernel/stacktrace.c static int save_trace(struct stackframe *frame, void *d) · line 175

Quoted 10 lines
arch/arm64/kernel/stacktrace.c +6 -0
+
+	walk_stackframe(current, &frame, save_trace, &data);
+	if (trace->nr_entries < trace->max_entries)
+		trace->entries[trace->nr_entries++] = ULONG_MAX;
+}
+
 void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 {
 	struct stack_trace_data data;
Reviewed-by James Morse <[email protected]>

Thanks,

James

#3 lore raw

PATCH v2 1/1 arm64:ftrace: add save_stack_trace_regs()

Quoted 33 lines · James Morse

Hi Pratyush,

Quoted 2 lines · Pratyush Anand

Implement save_stack_trace_regs, so that stacktrace of a kprobe events can be obtained.

Hmm, kconfig enables 'CONFIG_KPROBE_EVENT' if we select KPROBES and FTRACE, this calls save_stack_trace_regs(), which we haven't implemented, giving:

[  373.406982] save_stack_trace_regs() not implemented yet.
[  373.413020] ------------[ cut here ]------------
[  373.417601] WARNING: CPU: 1 PID: 0 at ../kernel/stacktrace.c:74 save_stack_tr
ace_regs+0x3c/0x48
[  373.426212] Modules linked in:
[  373.429238]
[  373.430716] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0-rc4-dirty #5128
[  373.437606] Hardware name: ARM Juno development board (r1) (DT)
[  373.443466] task: ffff800975dd1900 task.stack: ffff800975ddc000
[  373.449327] PC is at save_stack_trace_regs+0x3c/0x48
[  373.454242] LR is at save_stack_trace_regs+0x3c/0x48
[  373.459156] pc : [<ffff000008126c64>] lr : [<ffff000008126c64>] pstate: 600003c5
[  373.466476] sp : ffff80097ef52c00
[  373.554836] Call trace:
[  373.641153] [<ffff000008126c64>] save_stack_trace_regs+0x3c/0x48
[  373.647104] [<ffff00000817a0b0>] __ftrace_trace_stack+0x168/0x208
[  373.653138] [<ffff00000817ad40>] trace_buffer_unlock_commit_regs+0x5c/0x7c
[  373.659946] [<ffff00000818d57c>] kprobe_trace_func+0x308/0x3d8
[  373.665721] [<ffff00000818defc>] kprobe_dispatcher+0x58/0x60
[  373.671327] [<ffff0000088d8cfc>] kprobe_breakpoint_handler+0xbc/0x18c
[  373.677705] [<ffff0000080851b8>] brk_handler+0x50/0x90
[  373.682792] [<ffff000008081300>] do_debug_exception+0x50/0xbc
Quoted 3 lines

I guess hitting a WARN_ONCE() while using a 'kernel hacking' feature isn't serious enough to make this a fix.

I wasn't clear enough with the above: this patch is fixing this WARN_ON(), it may be worth mentioning that in the commit message...

Thanks,

James

#4 lore raw

PATCH v2 1/1 arm64:ftrace: add save_stack_trace_regs()

Quoted 34 lines · James Morse
Quoted 33 lines · James Morse

Hi Pratyush,

Quoted 2 lines · Pratyush Anand

Implement save_stack_trace_regs, so that stacktrace of a kprobe events can be obtained.

Hmm, kconfig enables 'CONFIG_KPROBE_EVENT' if we select KPROBES and FTRACE, this calls save_stack_trace_regs(), which we haven't implemented, giving:

[  373.406982] save_stack_trace_regs() not implemented yet.
[  373.413020] ------------[ cut here ]------------
[  373.417601] WARNING: CPU: 1 PID: 0 at ../kernel/stacktrace.c:74 save_stack_tr
ace_regs+0x3c/0x48
[  373.426212] Modules linked in:
[  373.429238]
[  373.430716] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0-rc4-dirty #5128
[  373.437606] Hardware name: ARM Juno development board (r1) (DT)
[  373.443466] task: ffff800975dd1900 task.stack: ffff800975ddc000
[  373.449327] PC is at save_stack_trace_regs+0x3c/0x48
[  373.454242] LR is at save_stack_trace_regs+0x3c/0x48
[  373.459156] pc : [<ffff000008126c64>] lr : [<ffff000008126c64>] pstate: 600003c5
[  373.466476] sp : ffff80097ef52c00
[  373.554836] Call trace:
[  373.641153] [<ffff000008126c64>] save_stack_trace_regs+0x3c/0x48
[  373.647104] [<ffff00000817a0b0>] __ftrace_trace_stack+0x168/0x208
[  373.653138] [<ffff00000817ad40>] trace_buffer_unlock_commit_regs+0x5c/0x7c
[  373.659946] [<ffff00000818d57c>] kprobe_trace_func+0x308/0x3d8
[  373.665721] [<ffff00000818defc>] kprobe_dispatcher+0x58/0x60
[  373.671327] [<ffff0000088d8cfc>] kprobe_breakpoint_handler+0xbc/0x18c
[  373.677705] [<ffff0000080851b8>] brk_handler+0x50/0x90
[  373.682792] [<ffff000008081300>] do_debug_exception+0x50/0xbc
Quoted 6 lines
Quoted 3 lines

I guess hitting a WARN_ONCE() while using a 'kernel hacking' feature isn't serious enough to make this a fix.

I wasn't clear enough with the above: this patch is fixing this WARN_ON(), it may be worth mentioning that in the commit message...

Yup, please stick that in the commit message without the timestamps and the addresses, then repost with the CONFIG_FUNCTION_GRAPH_TRACER fixup, Dave's tested-by and James' ack.

Will