Commit 279b5165 authored by Peter Zijlstra's avatar Peter Zijlstra Committed by Ingo Molnar

perf/core: Remove confusing comment and move put_ctx()

Since commit:

  321027c1 ("perf/core: Fix concurrent sys_perf_event_open() vs. 'move_group' race")

... the code looks like (assuming move_group==1):

  gctx = __perf_event_ctx_lock_double(group_leader, ctx);

  perf_remove_from_context(group_leader, 0);
  list_for_each_entry(sibling, &group_leader->sibling_list, group_entry) {
	perf_remove_from_context(sibling, 0);
	put_ctx(gctx);
  }

  /* ... */

  /* misleading comment about how this is the last reference */
  put_ctx(gctx);

  perf_event_ctx_unlock(group_leader, gctx);

What that 'last' put_ctx() does is drop @group_leader's reference on
gctx after having dropped all its potential sibling references.

But the thing is that __perf_event_ctx_lock_double() returns with a
reference _and_ a held lock, and perf_event_ctx_unlock() unlocks that
lock and drops that reference. Therefore that put_ctx() cannot be the
'last' of anything, nor is there an unbalance in puts.

To reduce confusion, remove the comment and place the put_ctx() next
to the remove_from_context() call.
Reported-by: default avatarBen Hutchings <ben@decadent.org.uk>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 8a5897fe
...@@ -9955,6 +9955,7 @@ SYSCALL_DEFINE5(perf_event_open, ...@@ -9955,6 +9955,7 @@ SYSCALL_DEFINE5(perf_event_open,
* of swizzling perf_event::ctx. * of swizzling perf_event::ctx.
*/ */
perf_remove_from_context(group_leader, 0); perf_remove_from_context(group_leader, 0);
put_ctx(gctx);
list_for_each_entry(sibling, &group_leader->sibling_list, list_for_each_entry(sibling, &group_leader->sibling_list,
group_entry) { group_entry) {
...@@ -9993,13 +9994,6 @@ SYSCALL_DEFINE5(perf_event_open, ...@@ -9993,13 +9994,6 @@ SYSCALL_DEFINE5(perf_event_open,
perf_event__state_init(group_leader); perf_event__state_init(group_leader);
perf_install_in_context(ctx, group_leader, group_leader->cpu); perf_install_in_context(ctx, group_leader, group_leader->cpu);
get_ctx(ctx); get_ctx(ctx);
/*
* Now that all events are installed in @ctx, nothing
* references @gctx anymore, so drop the last reference we have
* on it.
*/
put_ctx(gctx);
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment