From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756758AbcAMPA4 (ORCPT ); Wed, 13 Jan 2016 10:00:56 -0500 Received: from mga14.intel.com ([192.55.52.115]:63650 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756528AbcAMPAz (ORCPT ); Wed, 13 Jan 2016 10:00:55 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,289,1449561600"; d="scan'208";a="889710019" From: Alexander Shishkin To: Peter Zijlstra , mingo@kernel.org, eranian@google.com Cc: linux-kernel@vger.kernel.org, vince@deater.net, dvyukov@google.com, andi@firstfloor.org, jolsa@redhat.com, peterz@infradead.org Subject: Re: [RFC][PATCH 12/12] perf: Collapse and fix event_function_call() users In-Reply-To: <20160111163229.411314288@infradead.org> References: <20160111162458.427203780@infradead.org> <20160111163229.411314288@infradead.org> User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Wed, 13 Jan 2016 17:00:50 +0200 Message-ID: <87r3hl8qxp.fsf@ashishki-desk.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org I think I caught one, below. Peter Zijlstra writes: > +static int event_function(void *info) > +{ > + struct event_function_struct *efs = info; > + struct perf_event *event = efs->event; > + struct perf_event_context *ctx = event->ctx; > + struct perf_cpu_context *cpuctx = __get_cpu_context(ctx); > + struct perf_event_context *task_ctx = cpuctx->task_ctx; > + > + WARN_ON_ONCE(!irqs_disabled()); > + > + /* > + * Since we do the IPI call without holding ctx->lock things can have > + * changed, double check we hit the task we set out to hit. > + * > + * If ctx->task == current, we know things must remain valid because > + * we have IRQs disabled so we cannot schedule. > + */ > + if (ctx->task) { > + if (ctx->task != current) > + return -EAGAIN; > + > + WARN_ON_ONCE(task_ctx != ctx); Looks like between dropping ctx::lock in event_function_call() and here, cpuctx::task_ctx may still become NULL. > + } else { > + WARN_ON_ONCE(&cpuctx->ctx != ctx); > + } > + > + perf_ctx_lock(cpuctx, task_ctx); > + /* > + * Now that we hold locks, double check state. Paranoia pays. > + */ > + if (task_ctx) { > + WARN_ON_ONCE(task_ctx->task != current); > + /* > + * We only use event_function_call() on established contexts, > + * and event_function() is only ever called when active (or > + * rather, we'll have bailed in task_function_call() or the > + * above ctx->task != current test), therefore we must have > + * ctx->is_active here. > + */ > + WARN_ON_ONCE(!ctx->is_active); > + /* > + * And since we have ctx->is_active, cpuctx->task_ctx must > + * match. > + */ > + WARN_ON_ONCE(cpuctx->task_ctx != task_ctx); > + } > + efs->func(event, cpuctx, ctx, efs->data); In which case we probably don't want to call the callback. Not sure if this is what Dmitry ran into, his logs contain warnings from this function, but hard to tell exactly which ones. Regards, -- Alex