[PATCH 9/9] tracing: Remove locking trace_types_lock from tracing_reset_all_online_cpus()

[PATCH 9/9] tracing: Remove locking trace_types_lock from tracing_reset_all_online_cpus()

[Steven Rostedt]

[-- Attachment #1: 0009-tracing-Remove-locking-trace_types_lock-from-tracing.patch --]
[-- Type: text/plain, Size: 1943 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Commit a82274151af "tracing: Protect ftrace_trace_arrays list in trace_events.c"
added taking the trace_types_lock mutex in trace_events.c as there were
several locations that needed it for protection. Unfortunately, it also
encapsulated a call to tracing_reset_all_online_cpus() which also takes
the trace_types_lock, causing a deadlock.

This happens when a module has tracepoints and has been traced. When the
module is removed, the trace events module notifier will grab the
trace_types_lock, do a bunch of clean ups, and also clears the buffer
by calling tracing_reset_all_online_cpus. This doesn't happen often
which explains why it wasn't caught right away.

Commit a82274151af was marked for stable, which means this must be
sent to stable too.

Link: http://lkml.kernel.org/r/51EEC646.7070306@broadcom.com

Reported-by: Arend van Spril <arend@broadcom.com>
Tested-by: Arend van Spriel <arend@broadcom.com>
Cc: Alexander Z Lam <azl@google.com>
Cc: Vaibhav Nagarnaik <vnagarnaik@google.com>
Cc: David Sharp <dhsharp@google.com>
Cc: stable@vger.kernel.org # 3.10
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 69cba47..882ec1d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1224,18 +1224,17 @@ void tracing_reset_current(int cpu)
 	tracing_reset(&global_trace.trace_buffer, cpu);
 }
 
+/* Must have trace_types_lock held */
 void tracing_reset_all_online_cpus(void)
 {
 	struct trace_array *tr;
 
-	mutex_lock(&trace_types_lock);
 	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
 		tracing_reset_online_cpus(&tr->trace_buffer);
 #ifdef CONFIG_TRACER_MAX_TRACE
 		tracing_reset_online_cpus(&tr->max_buffer);
 #endif
 	}
-	mutex_unlock(&trace_types_lock);
 }
 
 #define SAVED_CMDLINES 128
-- 
1.7.10.4

Re: [PATCH 9/9] tracing: Remove locking trace_types_lock from tracing_reset_all_online_cpus()

[Arend van Spriel]

On 07/26/2013 03:03 PM, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)"<rostedt@goodmis.org>
>
> Commit a82274151af "tracing: Protect ftrace_trace_arrays list in trace_events.c"
> added taking the trace_types_lock mutex in trace_events.c as there were
> several locations that needed it for protection. Unfortunately, it also
> encapsulated a call to tracing_reset_all_online_cpus() which also takes
> the trace_types_lock, causing a deadlock.
>
> This happens when a module has tracepoints and has been traced. When the
> module is removed, the trace events module notifier will grab the
> trace_types_lock, do a bunch of clean ups, and also clears the buffer
> by calling tracing_reset_all_online_cpus. This doesn't happen often
> which explains why it wasn't caught right away.
>
> Commit a82274151af was marked for stable, which means this must be
> sent to stable too.
>
> Link:http://lkml.kernel.org/r/51EEC646.7070306@broadcom.com
>
> Reported-by: Arend van *Spril*<arend@broadcom.com>

Nasty dutch names, huh. If we ever meet on a summit you may try to 
pronounce it :-) Way easier than Finnish.

Regards,
Arend

> Tested-by: Arend van *Spriel*<arend@broadcom.com>
> Cc: Alexander Z Lam<azl@google.com>
> Cc: Vaibhav Nagarnaik<vnagarnaik@google.com>
> Cc: David Sharp<dhsharp@google.com>
> Cc:stable@vger.kernel.org  # 3.10
> Signed-off-by: Steven Rostedt<rostedt@goodmis.org>
> ---
>   kernel/trace/trace.c |    3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)

Re: [PATCH 9/9] tracing: Remove locking trace_types_lock from tracing_reset_all_online_cpus()

[Steven Rostedt]

On Fri, 2013-07-26 at 18:15 +0200, Arend van Spriel wrote:

> > Link:http://lkml.kernel.org/r/51EEC646.7070306@broadcom.com
> >
> > Reported-by: Arend van *Spril*<arend@broadcom.com>
> 
> Nasty dutch names, huh. If we ever meet on a summit you may try to 
> pronounce it :-) Way easier than Finnish.

Eek! How did that happen?? You would think cut and paste would work. I
did cut off the quotes that I had from the place that I cut it from. I
must have cropped off the 'e' there somehow too?

Sorry about that.

> 
> Regards,
> Arend
> 
> > Tested-by: Arend van *Spriel*<arend@broadcom.com>

Well, at least I got this part right.

-- Steve