From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754061AbdBPJrw (ORCPT ); Thu, 16 Feb 2017 04:47:52 -0500 Received: from merlin.infradead.org ([205.233.59.134]:40316 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753747AbdBPJrs (ORCPT ); Thu, 16 Feb 2017 04:47:48 -0500 Date: Thu, 16 Feb 2017 10:47:44 +0100 From: Peter Zijlstra To: Ben Hutchings Cc: Alexander Shishkin , Ingo Molnar , linux-kernel@vger.kernel.org Subject: [PATCH] perf: Remove confusing comment Message-ID: <20170216094744.GY6515@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 Signed-off-by: Peter Zijlstra (Intel) --- 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); } /*