perf: Fix contexted inheritance
Commit-ID: c5ed5145591774bd9a2960ba4ca45a02fc70aad1 Gitweb: http://git.kernel.org/tip/c5ed5145591774bd9a2960ba4ca45a02fc70aad1 Author: Peter Zijlstra <[email protected]> AuthorDate: Mon, 17 Jan 2011 13:45:37 +0100 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 18 Jan 2011 15:10:35 +0100
perf: Fix contexted inheritance
Linus reported that the RCU lockdep annotation bits triggered for this rcu_dereference() because we're not holding rcu_read_lock().
Going over the code I cannot convince myself its correct:
- holding a ref on the parent_ctx, doesn't avoid it being uncloned concurrently (as the comment says), so we can race with a free.
- holding parent_ctx->mutex doesn't avoid the above free from taking place either, it would at best avoid parent_ctx from being freed.
I.e. the warning is correct. To fix the bug, serialize against the unclone_ctx() call by extending the reach of the parent_ctx->lock.
Reported-by Linus Torvalds <[email protected]>
Signed-off-by Peter Zijlstra <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Paul E. McKenney <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by Ingo Molnar <[email protected]>
--- kernel/perf_event.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)
kernel/perf_event.c +5 -6
@@ -6494,7 +6494,6 @@ int perf_event_init_context(struct task_struct *child, int ctxn)raw_spin_lock_irqsave (&parent_ctx->lock, flags); parent_ctx->rotate_disable =0 ; -raw_spin_unlock_irqrestore (&parent_ctx->lock, flags); child_ctx = child->perf_event_ctxp[ctxn];
@@ -6502,12 +6501,11 @@ int perf_event_init_context(struct task_struct *child, int ctxn)/* * Mark the child context as a clone of the parent * context, or of whatever the parent is a clone of. -* Note that if the parent is a clone, it could get -* uncloned at any point, but that doesn't matter -* because the list of events and the generation -* count can't have changed since we took the mutex. +* +* Note that if the parent is a clone, the holding of +* parent_ctx->lock avoids it from being uncloned. */ - cloned_ctx =rcu_dereference (parent_ctx->parent_ctx); + cloned_ctx = parent_ctx->parent_ctx;if (cloned_ctx) { child_ctx->parent_ctx = cloned_ctx; child_ctx->parent_gen = parent_ctx->parent_gen;
@@ -6518,6 +6516,7 @@ int perf_event_init_context(struct task_struct *child, int ctxn)get_ctx (child_ctx->parent_ctx); } +raw_spin_unlock_irqrestore (&parent_ctx->lock, flags);mutex_unlock (&parent_ctx->mutex);perf_unpin_context (parent_ctx);