lkml.org 
[lkml]   [2010]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 10/11] tracing/perf: Fix lock events recursions in the fast path
On Wed, Feb 03, 2010 at 10:14:34AM +0100, Frederic Weisbecker wrote:
> There are rcu locked read side areas in the path where we submit
> a trace events. And these rcu_read_(un)lock() trigger lock events,
> which create recursive events.
>
> One pair in do_perf_sw_event:
>
> __lock_acquire
> |
> |--96.11%-- lock_acquire
> | |
> | |--27.21%-- do_perf_sw_event
> | | perf_tp_event
> | | |
> | | |--49.62%-- ftrace_profile_lock_release
> | | | lock_release
> | | | |
> | | | |--33.85%-- _raw_spin_unlock
>
> Another pair in perf_output_begin/end:
>
> __lock_acquire
> |--23.40%-- perf_output_begin
> | | __perf_event_overflow
> | | perf_swevent_overflow
> | | perf_swevent_add
> | | perf_swevent_ctx_event
> | | do_perf_sw_event
> | | perf_tp_event
> | | |
> | | |--55.37%-- ftrace_profile_lock_acquire
> | | | lock_acquire
> | | | |
> | | | |--37.31%-- _raw_spin_lock
>
> The problem is not that much the trace recursion itself, as we have a
> recursion protection already (though it's always wasteful to recurse).
> But the trace events are outside the lockdep recursion protection, then
> each lockdep event triggers a lock trace, which will trigger two
> other lockdep events. Here the recursive lock trace event won't
> be taken because of the trace recursion, so the recursion stops there
> but lockdep will still analyse these new events:
>
> To sum up, for each lockdep events we have:
>
> lock_*()
> |
> trace lock_acquire
> |
> ----- rcu_read_lock()
> | |
> | lock_acquire()
> | |
> | trace_lock_acquire() (stopped)
> | |
> | lockdep analyze
> |
> ----- rcu_read_unlock()
> |
> lock_release
> |
> trace_lock_release() (stopped)
> |
> lockdep analyze
>
> And you can repeat the above two times as we have two rcu read side
> sections when we submit an event.
>
> This is fixed in this pacth by using the non-lockdep versions of
> rcu_read_(un)lock.

Hmmm... Perhaps I should rename __rcu_read_lock() to something more
meaningful if it is to be used outside of the RCU files. In the
meantime:

Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
> Cc: Li Zefan <lizf@cn.fujitsu.com>
> Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
> Cc: Masami Hiramatsu <mhiramat@redhat.com>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> ---
> kernel/perf_event.c | 10 +++++-----
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index 280ae44..98fd360 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -2986,7 +2986,7 @@ int perf_output_begin(struct perf_output_handle *handle,
> u64 lost;
> } lost_event;
>
> - rcu_read_lock();
> + __rcu_read_lock();
> /*
> * For inherited events we send all the output towards the parent.
> */
> @@ -3051,7 +3051,7 @@ fail:
> atomic_inc(&data->lost);
> perf_output_unlock(handle);
> out:
> - rcu_read_unlock();
> + __rcu_read_unlock();
>
> return -ENOSPC;
> }
> @@ -3072,7 +3072,7 @@ void perf_output_end(struct perf_output_handle *handle)
> }
>
> perf_output_unlock(handle);
> - rcu_read_unlock();
> + __rcu_read_unlock();
> }
>
> static u32 perf_event_pid(struct perf_event *event, struct task_struct *p)
> @@ -4116,7 +4116,7 @@ static void do_perf_sw_event(enum perf_type_id type, u32 event_id,
> struct perf_event_context *ctx;
>
> cpuctx = &__get_cpu_var(perf_cpu_context);
> - rcu_read_lock();
> + __rcu_read_lock();
> perf_swevent_ctx_event(&cpuctx->ctx, type, event_id,
> nr, nmi, data, regs);
> /*
> @@ -4126,7 +4126,7 @@ static void do_perf_sw_event(enum perf_type_id type, u32 event_id,
> ctx = rcu_dereference(current->perf_event_ctxp);
> if (ctx)
> perf_swevent_ctx_event(ctx, type, event_id, nr, nmi, data, regs);
> - rcu_read_unlock();
> + __rcu_read_unlock();
> }
>
> void __perf_sw_event(u32 event_id, u64 nr, int nmi,
> --
> 1.6.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/


\
 
 \ /
  Last update: 2010-02-04 16:49    [from the cache]
©2003-2011 Jasper Spaans