| From 6224beb12e190ff11f3c7d4bf50cb2922878f600 Mon Sep 17 00:00:00 2001 |
| From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org> |
| Date: Tue, 7 Jul 2015 15:05:03 -0400 |
| Subject: tracing: Have branch tracer use recursive field of task struct |
| |
| From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org> |
| |
| commit 6224beb12e190ff11f3c7d4bf50cb2922878f600 upstream. |
| |
| Fengguang Wu's tests triggered a bug in the branch tracer's start up |
| test when CONFIG_DEBUG_PREEMPT set. This was because that config |
| adds some debug logic in the per cpu field, which calls back into |
| the branch tracer. |
| |
| The branch tracer has its own recursive checks, but uses a per cpu |
| variable to implement it. If retrieving the per cpu variable calls |
| back into the branch tracer, you can see how things will break. |
| |
| Instead of using a per cpu variable, use the trace_recursion field |
| of the current task struct. Simply set a bit when entering the |
| branch tracing and clear it when leaving. If the bit is set on |
| entry, just don't do the tracing. |
| |
| There's also the case with lockdep, as the local_irq_save() called |
| before the recursion can also trigger code that can call back into |
| the function. Changing that to a raw_local_irq_save() will protect |
| that as well. |
| |
| This prevents the recursion and the inevitable crash that follows. |
| |
| Link: http://lkml.kernel.org/r/20150630141803.GA28071@wfg-t540p.sh.intel.com |
| |
| Reported-by: Fengguang Wu <fengguang.wu@intel.com> |
| Tested-by: Fengguang Wu <fengguang.wu@intel.com> |
| Signed-off-by: Steven Rostedt <rostedt@goodmis.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| kernel/trace/trace.h | 1 + |
| kernel/trace/trace_branch.c | 17 ++++++++++------- |
| 2 files changed, 11 insertions(+), 7 deletions(-) |
| |
| --- a/kernel/trace/trace.h |
| +++ b/kernel/trace/trace.h |
| @@ -428,6 +428,7 @@ enum { |
| |
| TRACE_CONTROL_BIT, |
| |
| + TRACE_BRANCH_BIT, |
| /* |
| * Abuse of the trace_recursion. |
| * As we need a way to maintain state if we are tracing the function |
| --- a/kernel/trace/trace_branch.c |
| +++ b/kernel/trace/trace_branch.c |
| @@ -37,9 +37,12 @@ probe_likely_condition(struct ftrace_bra |
| struct trace_branch *entry; |
| struct ring_buffer *buffer; |
| unsigned long flags; |
| - int cpu, pc; |
| + int pc; |
| const char *p; |
| |
| + if (current->trace_recursion & TRACE_BRANCH_BIT) |
| + return; |
| + |
| /* |
| * I would love to save just the ftrace_likely_data pointer, but |
| * this code can also be used by modules. Ugly things can happen |
| @@ -50,10 +53,10 @@ probe_likely_condition(struct ftrace_bra |
| if (unlikely(!tr)) |
| return; |
| |
| - local_irq_save(flags); |
| - cpu = raw_smp_processor_id(); |
| - data = per_cpu_ptr(tr->trace_buffer.data, cpu); |
| - if (atomic_inc_return(&data->disabled) != 1) |
| + raw_local_irq_save(flags); |
| + current->trace_recursion |= TRACE_BRANCH_BIT; |
| + data = this_cpu_ptr(tr->trace_buffer.data); |
| + if (atomic_read(&data->disabled)) |
| goto out; |
| |
| pc = preempt_count(); |
| @@ -82,8 +85,8 @@ probe_likely_condition(struct ftrace_bra |
| __buffer_unlock_commit(buffer, event); |
| |
| out: |
| - atomic_dec(&data->disabled); |
| - local_irq_restore(flags); |
| + current->trace_recursion &= ~TRACE_BRANCH_BIT; |
| + raw_local_irq_restore(flags); |
| } |
| |
| static inline |