Messages in this thread | | | Date | Thu, 26 May 2011 20:38:21 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH -tip ] [BUGFIX]tracing/kprobes: Fix kprobe-tracer to support stack trace |
| |
(2011/05/26 11:07), Steven Rostedt wrote: > On Thu, 2011-05-12 at 19:22 +0900, Masami Hiramatsu wrote: >> Fix to support kernel stack trace correctly on kprobe-tracer. >> Since the execution path of kprobe-based dynamic events is different >> from other tracepoint-based events, normal ftrace_trace_stack() doesn't >> work correctly. To fix that, this introduces ftrace_trace_stack_regs() >> which traces stack via pt_regs instead of current stack register. >> > > Hi Masami, > > I'm working to get a "fix" patch set push out, and I included this > patch. But when I ran it on my arch test (compiling on other archs), > this patch broke the following: > > arm, blackfin, powerpc, s390. > > (Note, due to other bugs, UML and mips does not build to test)
Oh... I see. Perhaps, we can refer the systemtap backtrace implementation for some of them. As far as I know, it supports arm, ppc, and s390.
> > The problem is that these archs do not implement a: > > save_stack_trace_regs() function. > > I'm stripping this patch out of my next pull request, and it may need to > wait till 2.6.41/2.8.1/3.1.
OK, thanks for the test!
> > -- Steve > >> e.g. >> >> # echo p schedule > /sys/kernel/debug/tracing/kprobe_events >> # echo 1 > /sys/kernel/debug/tracing/options/stacktrace >> # echo 1 > /sys/kernel/debug/tracing/events/kprobes/enable >> # head -n 20 /sys/kernel/debug/tracing/trace >> # tracer: nop >> # >> # TASK-PID CPU# TIMESTAMP FUNCTION >> # | | | | | >> sshd-1549 [001] 857.326335: p_schedule_0: (schedule+0x0/0x900) >> sshd-1549 [001] 857.326337: <stack trace> >> => schedule_hrtimeout_range >> => poll_schedule_timeout >> => do_select >> => core_sys_select >> => sys_select >> => system_call_fastpath >> tee-1751 [000] 857.326394: p_schedule_0: (schedule+0x0/0x900) >> tee-1751 [000] 857.326395: <stack trace> >> => do_group_exit >> => sys_exit_group >> => system_call_fastpath >> <idle>-0 [001] 857.326406: p_schedule_0: (schedule+0x0/0x900) >> <idle>-0 [001] 857.326407: <stack trace> >> => start_secondary >> >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >> Cc: Steven Rostedt <rostedt@goodmis.org> >> Cc: Frederic Weisbecker <fweisbec@gmail.com> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> >> Cc: linux-kernel@vger.kernel.org >> --- >> include/linux/ftrace_event.h | 4 ++++ >> kernel/trace/trace.c | 35 ++++++++++++++++++++++++++++++----- >> kernel/trace/trace.h | 9 +++++++++ >> kernel/trace/trace_kprobe.c | 6 ++++-- >> 4 files changed, 47 insertions(+), 7 deletions(-) >> >> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h >> index b5a550a..bca2499 100644 >> --- a/include/linux/ftrace_event.h >> +++ b/include/linux/ftrace_event.h >> @@ -117,6 +117,10 @@ void trace_current_buffer_unlock_commit(struct ring_buffer *buffer, >> void trace_nowake_buffer_unlock_commit(struct ring_buffer *buffer, >> struct ring_buffer_event *event, >> unsigned long flags, int pc); >> +void trace_nowake_buffer_unlock_commit_regs(struct ring_buffer *buffer, >> + struct ring_buffer_event *event, >> + unsigned long flags, int pc, >> + struct pt_regs *regs); >> void trace_current_buffer_discard_commit(struct ring_buffer *buffer, >> struct ring_buffer_event *event); >> >> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c >> index ee9c921..bf12209 100644 >> --- a/kernel/trace/trace.c >> +++ b/kernel/trace/trace.c >> @@ -1191,6 +1191,18 @@ void trace_nowake_buffer_unlock_commit(struct ring_buffer *buffer, >> } >> EXPORT_SYMBOL_GPL(trace_nowake_buffer_unlock_commit); >> >> +void trace_nowake_buffer_unlock_commit_regs(struct ring_buffer *buffer, >> + struct ring_buffer_event *event, >> + unsigned long flags, int pc, >> + struct pt_regs *regs) >> +{ >> + ring_buffer_unlock_commit(buffer, event); >> + >> + ftrace_trace_stack_regs(buffer, flags, 0, pc, regs); >> + ftrace_trace_userstack(buffer, flags, pc); >> +} >> +EXPORT_SYMBOL_GPL(trace_nowake_buffer_unlock_commit_regs); >> + >> void trace_current_buffer_discard_commit(struct ring_buffer *buffer, >> struct ring_buffer_event *event) >> { >> @@ -1236,7 +1248,7 @@ ftrace(struct trace_array *tr, struct trace_array_cpu *data, >> #ifdef CONFIG_STACKTRACE >> static void __ftrace_trace_stack(struct ring_buffer *buffer, >> unsigned long flags, >> - int skip, int pc) >> + int skip, int pc, struct pt_regs *regs) >> { >> struct ftrace_event_call *call = &event_kernel_stack; >> struct ring_buffer_event *event; >> @@ -1255,24 +1267,36 @@ static void __ftrace_trace_stack(struct ring_buffer *buffer, >> trace.skip = skip; >> trace.entries = entry->caller; >> >> - save_stack_trace(&trace); >> + if (regs) >> + save_stack_trace_regs(&trace, regs); >> + else >> + save_stack_trace(&trace); >> if (!filter_check_discard(call, entry, buffer, event)) >> ring_buffer_unlock_commit(buffer, event); >> } >> >> +void ftrace_trace_stack_regs(struct ring_buffer *buffer, unsigned long flags, >> + int skip, int pc, struct pt_regs *regs) >> +{ >> + if (!(trace_flags & TRACE_ITER_STACKTRACE)) >> + return; >> + >> + __ftrace_trace_stack(buffer, flags, skip, pc, regs); >> +} >> + >> void ftrace_trace_stack(struct ring_buffer *buffer, unsigned long flags, >> int skip, int pc) >> { >> if (!(trace_flags & TRACE_ITER_STACKTRACE)) >> return; >> >> - __ftrace_trace_stack(buffer, flags, skip, pc); >> + __ftrace_trace_stack(buffer, flags, skip, pc, NULL); >> } >> >> void __trace_stack(struct trace_array *tr, unsigned long flags, int skip, >> int pc) >> { >> - __ftrace_trace_stack(tr->buffer, flags, skip, pc); >> + __ftrace_trace_stack(tr->buffer, flags, skip, pc, NULL); >> } >> >> /** >> @@ -1288,7 +1312,8 @@ void trace_dump_stack(void) >> local_save_flags(flags); >> >> /* skipping 3 traces, seems to get us at the caller of this function */ >> - __ftrace_trace_stack(global_trace.buffer, flags, 3, preempt_count()); >> + __ftrace_trace_stack(global_trace.buffer, flags, 3, preempt_count(), >> + NULL); >> } >> >> static DEFINE_PER_CPU(int, user_stack_count); >> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h >> index 5e9dfc6..cbce5e4 100644 >> --- a/kernel/trace/trace.h >> +++ b/kernel/trace/trace.h >> @@ -389,6 +389,9 @@ void update_max_tr_single(struct trace_array *tr, >> void ftrace_trace_stack(struct ring_buffer *buffer, unsigned long flags, >> int skip, int pc); >> >> +void ftrace_trace_stack_regs(struct ring_buffer *buffer, unsigned long flags, >> + int skip, int pc, struct pt_regs *regs); >> + >> void ftrace_trace_userstack(struct ring_buffer *buffer, unsigned long flags, >> int pc); >> >> @@ -400,6 +403,12 @@ static inline void ftrace_trace_stack(struct ring_buffer *buffer, >> { >> } >> >> +static inline void ftrace_trace_stack_regs(struct ring_buffer *buffer, >> + unsigned long flags, int skip, >> + int pc, struct pt_regs *regs) >> +{ >> +} >> + >> static inline void ftrace_trace_userstack(struct ring_buffer *buffer, >> unsigned long flags, int pc) >> { >> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c >> index f925c45..7053ef3 100644 >> --- a/kernel/trace/trace_kprobe.c >> +++ b/kernel/trace/trace_kprobe.c >> @@ -1397,7 +1397,8 @@ static __kprobes void kprobe_trace_func(struct kprobe *kp, struct pt_regs *regs) >> store_trace_args(sizeof(*entry), tp, regs, (u8 *)&entry[1], dsize); >> >> if (!filter_current_check_discard(buffer, call, entry, event)) >> - trace_nowake_buffer_unlock_commit(buffer, event, irq_flags, pc); >> + trace_nowake_buffer_unlock_commit_regs(buffer, event, >> + irq_flags, pc, regs); >> } >> >> /* Kretprobe handler */ >> @@ -1429,7 +1430,8 @@ static __kprobes void kretprobe_trace_func(struct kretprobe_instance *ri, >> store_trace_args(sizeof(*entry), tp, regs, (u8 *)&entry[1], dsize); >> >> if (!filter_current_check_discard(buffer, call, entry, event)) >> - trace_nowake_buffer_unlock_commit(buffer, event, irq_flags, pc); >> + trace_nowake_buffer_unlock_commit_regs(buffer, event, >> + irq_flags, pc, regs); >> } >> >> /* Event entry printers */ > >
| |