* [PATCH] perf: Remove confusing comment
@ 2017-02-16 9:47 Peter Zijlstra
0 siblings, 0 replies; only message in thread
From: Peter Zijlstra @ 2017-02-16 9:47 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Alexander Shishkin, Ingo Molnar, linux-kernel
Since commit: 321027c1fe77 ("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: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/events/core.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -9957,6 +9957,7 @@ SYSCALL_DEFINE5(perf_event_open,
* of swizzling perf_event::ctx.
*/
perf_remove_from_context(group_leader, 0);
+ put_ctx(gctx);
list_for_each_entry(sibling, &group_leader->sibling_list,
group_entry) {
@@ -9995,13 +9996,6 @@ SYSCALL_DEFINE5(perf_event_open,
perf_event__state_init(group_leader);
perf_install_in_context(ctx, group_leader, group_leader->cpu);
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);
}
/*
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2017-02-16 9:47 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-16 9:47 [PATCH] perf: Remove confusing comment Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®