From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753771AbbIJQV5 (ORCPT ); Thu, 10 Sep 2015 12:21:57 -0400 Received: from casper.infradead.org ([85.118.1.10]:41333 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752217AbbIJQVx (ORCPT ); Thu, 10 Sep 2015 12:21:53 -0400 Message-Id: <20150910162007.881117204@infradead.org> User-Agent: quilt/0.61-1 Date: Thu, 10 Sep 2015 18:16:22 +0200 From: Peter Zijlstra To: mingo@kernel.org Cc: linux-kernel@vger.kernel.org, vincent.weaver@maine.edu, acme@kernel.org, eranian@google.com, jolsa@redhat.com, alexander.shishkin@linux.intel.com, peterz@infradead.org Subject: [RFC][PATCH 1/3] perf: Restructure perf syscall point of no return References: <20150910161621.363774906@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Disposition: inline; filename=peterz-perf-rework-syscall-error-patch.patch Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The exclusive_event_installable() stuff only works because its exclusive with the grouping bits. Rework the code such that there is a sane place to error out before we go do things we cannot undo. Cc: Alexander Shishkin Signed-off-by: Peter Zijlstra (Intel) --- kernel/events/core.c | 108 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 78 insertions(+), 30 deletions(-) --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -8242,15 +8271,31 @@ SYSCALL_DEFINE5(perf_event_open, goto err_context; } - if (move_group) { - gctx = group_leader->ctx; + gctx = group_leader->ctx; + if (move_group) + mutex_lock_double(&gctx->mutex, &ctx->mutex); + else + mutex_lock(&ctx->mutex); + /* + * Must be under the same ctx::mutex as perf_install_in_context(), + * because we need to serialize with concurrent event creation. + */ + if (!exclusive_event_installable(event, ctx)) { + /* exclusive and group stuff are assumed mutually exclusive */ + WARN_ON_ONCE(move_group); + + err = -EBUSY; + goto err_locked; + } + + WARN_ON_ONCE(ctx->parent_ctx); + + if (move_group) { /* * See perf_event_ctx_lock() for comments on the details * of swizzling perf_event::ctx. */ - mutex_lock_double(&gctx->mutex, &ctx->mutex); - perf_remove_from_context(group_leader, false); list_for_each_entry(sibling, &group_leader->sibling_list, @@ -8258,13 +8308,7 @@ SYSCALL_DEFINE5(perf_event_open, perf_remove_from_context(sibling, false); put_ctx(gctx); } - } else { - mutex_lock(&ctx->mutex); - } - WARN_ON_ONCE(ctx->parent_ctx); - - if (move_group) { /* * Wait for everybody to stop referencing the events through * the old lists, before installing it on new lists. @@ -8296,22 +8340,20 @@ SYSCALL_DEFINE5(perf_event_open, perf_event__state_init(group_leader); perf_install_in_context(ctx, group_leader, group_leader->cpu); get_ctx(ctx); - } - if (!exclusive_event_installable(event, ctx)) { - err = -EBUSY; - mutex_unlock(&ctx->mutex); - fput(event_file); - goto err_context; + /* + * Now that all events are installed in @ctx, nothing + * references @gctx anymore, so drop the last reference we have + * on it. + */ + put_ctx(gctx); } perf_install_in_context(ctx, event, event->cpu); perf_unpin_context(ctx); - if (move_group) { + if (move_group) mutex_unlock(&gctx->mutex); - put_ctx(gctx); - } mutex_unlock(&ctx->mutex); put_online_cpus(); @@ -8338,6 +8380,12 @@ SYSCALL_DEFINE5(perf_event_open, fd_install(event_fd, event_file); return event_fd; +err_locked: + if (move_group) + mutex_unlock(&gctx->mutex); + mutex_unlock(&ctx->mutex); +/* err_file: */ + fput(event_file); err_context: perf_unpin_context(ctx); put_ctx(ctx);