Re: [RFC][PATCH] ppc: Implement save_stack_trace_regs()
On Mon, 11 Jan 2016 14:30:31 +1100 Michael Ellermanwrote: > Sorry, yep I'll take it. > > I trimmed the change log a bit, final version below. > > cheers > Thanks, appreciate it! -- Steve ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] ppc: Implement save_stack_trace_regs()
On Fri, 2016-01-08 at 17:50 -0500, Steven Rostedt wrote: > On Wed, 16 Dec 2015 12:24:19 -0500 > Steven Rostedtwrote: > > On Wed, 09 Dec 2015 12:03:05 +1100 > > Michael Ellerman wrote: > > > > > Should I take this via powerpc or do you want it to go in via tracing? > > > > > > > > You can take it. And you can replace the PT_R1 if you want. I just > > > > noticed that it was defined, and I try to use macro names instead of > > > > hard coded numbers. I was actually looking for a "PT_SP" :-) > > > > > > OK thanks. > > > > > > Looks like we actually have: > > > > > > #define kernel_stack_pointer(regs) ((regs)->gpr[1]) > > > > > > So that would be the most self documenting way to do it I guess, though > > > I've > > > never actually seen that macro used anywhere before :) > > > > Hi, > > > > Are you going to take this, or do you want me to? Sorry, yep I'll take it. I trimmed the change log a bit, final version below. cheers powerpc: Implement save_stack_trace_regs() to enable kprobe stack tracing It has come to my attention that kprobe event stack tracing does not work on powerpc. You can see with the following: # cd /sys/kernel/debug/tracing # echo stacktrace > trace_options # echo 'p kfree' > kprobe_events # echo 1 > events/kprobes/enable Will print the following warning: save_stack_trace_regs() not implemented yet. Although save_stack_trace() (which normal event stack traces use) is implemented, save_stack_trace_regs() which kprobe events use is not. This is a cheap attempt to implement that function. Note, This may have issues if a task tries to get a stack trace from another task with its regs, because it just passes in "current" to save_context_stack(). But this does solve the issue with stack tracing kprobe events. Reported-by: Chunyu Hu Signed-off-by: Steven Rostedt Signed-off-by: Michael Ellerman diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c index ea43a347a104..4f24606afc3f 100644 --- a/arch/powerpc/kernel/stacktrace.c +++ b/arch/powerpc/kernel/stacktrace.c @@ -61,3 +61,10 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) save_context_stack(trace, tsk->thread.ksp, tsk, 0); } EXPORT_SYMBOL_GPL(save_stack_trace_tsk); + +void +save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace) +{ + save_context_stack(trace, regs->gpr[1], current, 0); +} +EXPORT_SYMBOL_GPL(save_stack_trace_regs); ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] ppc: Implement save_stack_trace_regs()
On Wed, 16 Dec 2015 12:24:19 -0500 Steven Rostedtwrote: > On Wed, 09 Dec 2015 12:03:05 +1100 > Michael Ellerman wrote: > > > > > Should I take this via powerpc or do you want it to go in via tracing? > > > > > > > > > > You can take it. And you can replace the PT_R1 if you want. I just > > > noticed that it was defined, and I try to use macro names instead of > > > hard coded numbers. I was actually looking for a "PT_SP" :-) > > > > OK thanks. > > > > Looks like we actually have: > > > > #define kernel_stack_pointer(regs) ((regs)->gpr[1]) > > > > So that would be the most self documenting way to do it I guess, though I've > > never actually seen that macro used anywhere before :) > > Hi, > > Are you going to take this, or do you want me to? > > -- Steve Ping? -- Steve ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] ppc: Implement save_stack_trace_regs()
On Wed, 09 Dec 2015 12:03:05 +1100 Michael Ellermanwrote: > > > Should I take this via powerpc or do you want it to go in via tracing? > > > > You can take it. And you can replace the PT_R1 if you want. I just > > noticed that it was defined, and I try to use macro names instead of > > hard coded numbers. I was actually looking for a "PT_SP" :-) > > OK thanks. > > Looks like we actually have: > > #define kernel_stack_pointer(regs) ((regs)->gpr[1]) > > So that would be the most self documenting way to do it I guess, though I've > never actually seen that macro used anywhere before :) Hi, Are you going to take this, or do you want me to? -- Steve ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC][PATCH] ppc: Implement save_stack_trace_regs()
It has come to my attention that kprobe event stack tracing does not work on powerpc. You can see with the following: # cd /sys/kernel/debug/tracing # echo stacktrace > trace_options # echo 'p kfree' > kprobe_events # echo 'r exit_mmap' >> kprobe_events # echo 1 > events/kprobes/enable Gives the following splat: save_stack_trace_regs() not implemented yet. [ cut here ] WARNING: at kernel/stacktrace.c:74 Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.3.0-test #89 task: c0a98660 ti: c0003efe8000 task.ti: c0b98000 NIP: c00fa72c LR: c00fa728 CTR: c04a8b80 REGS: c0003efeaf40 TRAP: 0700 Not tainted (4.3.0-test) MSR: 90021032CR: 28002042 XER: 2000 SOFTE: 0 GPR00: c00fa728 c0003efeb1c0 c0b9e400 002c GPR04: 013b c0b5e790 6e6f7420696d706c GPR08: 656d656e c0aad148 c0ba3878 01ef GPR12: 22002044 c000 0100 GPR16: c0003800f01c c0003800f02c 0001a849 c00038008100 GPR20: c0003800f010 c0003efe8000 GPR24: 0100 c000389ad120 c000389ad1b0 GPR28: 0100 c0003efeb920 c0a1d88c c0a93529 NIP [c00fa72c] .save_stack_trace_regs+0x3c/0x70 LR [c00fa728] .save_stack_trace_regs+0x38/0x70 Call Trace: [c0003efeb1c0] [c00fa728] .save_stack_trace_regs+0x38/0x70 (unreliable) [c0003efeb240] [c0145b2c] .__ftrace_trace_stack+0x15c/0x210 [c0003efeb310] [c0145cc4] .ftrace_trace_stack_regs+0x24/0x40 [c0003efeb380] [c01467c4] .trace_buffer_unlock_commit_regs+0x44/0x70 [c0003efeb420] [c0165b38] .kprobe_trace_func+0x278/0x400 [c0003efeb530] [c0165d38] .kprobe_dispatcher+0x78/0xa0 [c0003efeb5c0] [c07b0ca0] .kprobe_exceptions_notify+0x2e0/0x520 [c0003efeb670] [c00b4934] .notifier_call_chain+0x94/0xf0 [c0003efeb710] [c00b4a2c] .atomic_notifier_call_chain+0x3c/0x50 [c0003efeb7a0] [c00b4c48] .notify_die+0x38/0x50 [c0003efeb830] [c07aff90] .program_check_exception+0x1a0/0x260 [c0003efeb8b0] [c000621c] program_check_common+0x11c/0x180 --- interrupt: 700 at .kfree+0x0/0x220 LR = .skb_release_data+0xe8/0x160 [c0003efebba0] [c0003efebc30] 0xc0003efebc30 (unreliable) [c0003efebc30] [c0678bd8] .__kfree_skb+0x38/0xe0 [c0003efebcb0] [c0686a20] .net_tx_action+0xe0/0x330 [c0003efebd70] [c0090e94] .__do_softirq+0x194/0x3d0 [c0003efebe90] [c0091428] .irq_exit+0xb8/0x100 [c0003efebf00] [c000f2dc] .__do_irq+0xac/0x1b0 [c0003efebf90] [c001f1b8] .call_do_irq+0x14/0x24 [c0b9b870] [c000f478] .do_IRQ+0x98/0x110 [c0b9b920] [c00020b8] hardware_interrupt_common+0x138/0x180 --- interrupt: 501 at .arch_local_irq_restore+0x64/0x90 LR = .arch_local_irq_restore+0x64/0x90 [c0b9bc10] [c0b98000] 0xc0b98000 (unreliable) [c0b9bc80] [c00133e8] .arch_cpu_idle+0xe8/0x160 [c0b9bd00] [c00d9204] .default_idle_call+0x44/0x70 [c0b9bd70] [c00d95f4] .cpu_startup_entry+0x2f4/0x460 [c0b9be80] [c000ab4c] .rest_init+0x9c/0xb0 [c0b9bef0] [c097742c] .start_kernel+0x520/0x540 [c0b9bf90] [c0008cf0] start_here_common+0x20/0x3b0 Instruction dump: f821ff81 3fe2ffef 6000 6000 3bff5129 881f0001 2f81 41be0028 3c62ffd0 38636b38 486bb975 6000 <0fe0> 3801 981f0001 6000 ---[ end trace e224cc02c4ea7f78 ]--- Although save_stack_trace() (which normal event stack traces use) is implemented, save_stack_trace_regs() which kprobe events use is not. This is a cheap attempt to implement that function. Note, This may have issues if a task tries to get a stack trace from another task with its regs, because it just passes in "current" to save_context_stack(). But this does solve the issue with stack tracing kprobe events. Reported-by: Chunyu Hu Signed-off-by: Steven Rostedt --- diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c index ea43a347a104..0142c86801ba 100644 --- a/arch/powerpc/kernel/stacktrace.c +++ b/arch/powerpc/kernel/stacktrace.c @@ -61,3 +61,10 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) save_context_stack(trace, tsk->thread.ksp, tsk, 0); } EXPORT_SYMBOL_GPL(save_stack_trace_tsk); + +void +save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace) +{ + save_context_stack(trace, regs->gpr[PT_R1], current, 0); +} +EXPORT_SYMBOL_GPL(save_stack_trace_regs); ___ Linuxppc-dev mailing list
Re: [RFC][PATCH] ppc: Implement save_stack_trace_regs()
On Wed, 09 Dec 2015 11:20:22 +1100 Michael Ellermanwrote: > On Tue, 2015-12-08 at 13:50 -0500, Steven Rostedt wrote: > > > It has come to my attention that kprobe event stack tracing does not > > work on powerpc. > > Yep looks like you're right. I didn't realise it was separate from the regular > stack trace stuff. > > > diff --git a/arch/powerpc/kernel/stacktrace.c > > b/arch/powerpc/kernel/stacktrace.c > > index ea43a347a104..0142c86801ba 100644 > > --- a/arch/powerpc/kernel/stacktrace.c > > +++ b/arch/powerpc/kernel/stacktrace.c > > @@ -61,3 +61,10 @@ void save_stack_trace_tsk(struct task_struct *tsk, > > struct stack_trace *trace) > > save_context_stack(trace, tsk->thread.ksp, tsk, 0); > > } > > EXPORT_SYMBOL_GPL(save_stack_trace_tsk); > > + > > +void > > +save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace) > > +{ > > + save_context_stack(trace, regs->gpr[PT_R1], current, 0); > > In the kernel we would normally use just '1' here rather than 'PT_R1', but > it's > not a huge deal. > > Should I take this via powerpc or do you want it to go in via tracing? > You can take it. And you can replace the PT_R1 if you want. I just noticed that it was defined, and I try to use macro names instead of hard coded numbers. I was actually looking for a "PT_SP" :-) -- Steve ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] ppc: Implement save_stack_trace_regs()
On Tue, 2015-12-08 at 19:28 -0500, Steven Rostedt wrote: > On Wed, 09 Dec 2015 11:20:22 +1100 > Michael Ellermanwrote: > > On Tue, 2015-12-08 at 13:50 -0500, Steven Rostedt wrote: > > > It has come to my attention that kprobe event stack tracing does not > > > work on powerpc. > > > diff --git a/arch/powerpc/kernel/stacktrace.c > > > b/arch/powerpc/kernel/stacktrace.c > > > index ea43a347a104..0142c86801ba 100644 > > > --- a/arch/powerpc/kernel/stacktrace.c > > > +++ b/arch/powerpc/kernel/stacktrace.c > > > @@ -61,3 +61,10 @@ void save_stack_trace_tsk(struct task_struct *tsk, > > > struct stack_trace *trace) > > > save_context_stack(trace, tsk->thread.ksp, tsk, 0); > > > } > > > EXPORT_SYMBOL_GPL(save_stack_trace_tsk); > > > + > > > +void > > > +save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace) > > > +{ > > > + save_context_stack(trace, regs->gpr[PT_R1], current, 0); > > > > In the kernel we would normally use just '1' here rather than 'PT_R1', but > > it's > > not a huge deal. > > > > Should I take this via powerpc or do you want it to go in via tracing? > > You can take it. And you can replace the PT_R1 if you want. I just > noticed that it was defined, and I try to use macro names instead of > hard coded numbers. I was actually looking for a "PT_SP" :-) OK thanks. Looks like we actually have: #define kernel_stack_pointer(regs) ((regs)->gpr[1]) So that would be the most self documenting way to do it I guess, though I've never actually seen that macro used anywhere before :) cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] ppc: Implement save_stack_trace_regs()
On Tue, 2015-12-08 at 13:50 -0500, Steven Rostedt wrote: > It has come to my attention that kprobe event stack tracing does not > work on powerpc. Yep looks like you're right. I didn't realise it was separate from the regular stack trace stuff. > diff --git a/arch/powerpc/kernel/stacktrace.c > b/arch/powerpc/kernel/stacktrace.c > index ea43a347a104..0142c86801ba 100644 > --- a/arch/powerpc/kernel/stacktrace.c > +++ b/arch/powerpc/kernel/stacktrace.c > @@ -61,3 +61,10 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct > stack_trace *trace) > save_context_stack(trace, tsk->thread.ksp, tsk, 0); > } > EXPORT_SYMBOL_GPL(save_stack_trace_tsk); > + > +void > +save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace) > +{ > + save_context_stack(trace, regs->gpr[PT_R1], current, 0); In the kernel we would normally use just '1' here rather than 'PT_R1', but it's not a huge deal. Should I take this via powerpc or do you want it to go in via tracing? cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] ppc: Implement save_stack_trace_regs()
On Wed, 09 Dec 2015 12:03:05 +1100 Michael Ellermanwrote: > Looks like we actually have: > > #define kernel_stack_pointer(regs) ((regs)->gpr[1]) > > So that would be the most self documenting way to do it I guess, though I've > never actually seen that macro used anywhere before :) Cool, throw that in, and then it will be the first for you ;-) -- Steve ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev