Skip to content

Commit

Permalink
perf: Fix contexted inheritance
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
Peter Zijlstra authored and Ingo Molnar committed Jan 18, 2011
1 parent ad7f4e3 commit c5ed514
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions kernel/perf_event.c
Original file line number Diff line number Diff line change
Expand Up @@ -6494,20 +6494,18 @@ 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];

if (child_ctx && inherited_all) {
/*
* 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;
Expand All @@ -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);
Expand Down

0 comments on commit c5ed514

Please sign in to comment.