From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759501AbcAKKox (ORCPT ); Mon, 11 Jan 2016 05:44:53 -0500 Received: from mga02.intel.com ([134.134.136.20]:49381 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757510AbcAKKow (ORCPT ); Mon, 11 Jan 2016 05:44:52 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,552,1444719600"; d="scan'208";a="890723269" From: Alexander Shishkin To: Peter Zijlstra Cc: Ingo Molnar , linux-kernel@vger.kernel.org, vince@deater.net, eranian@google.com, johannes@sipsolutions.net, Arnaldo Carvalho de Melo Subject: Re: [PATCH 2/7] perf: Generalize task_function_call()ers In-Reply-To: <20151218090129.GQ6373@twins.programming.kicks-ass.net> References: <1449138762-15194-1-git-send-email-alexander.shishkin@linux.intel.com> <1449138762-15194-3-git-send-email-alexander.shishkin@linux.intel.com> <20151203173431.GC3816@twins.programming.kicks-ass.net> <87vb88yjs6.fsf@ashishki-desk.ger.corp.intel.com> <20151208165700.GE6357@twins.programming.kicks-ass.net> <20151217134034.GM6373@twins.programming.kicks-ass.net> <87k2odb19x.fsf@ashishki-desk.ger.corp.intel.com> <20151217150732.GG6344@twins.programming.kicks-ass.net> <20151218090129.GQ6373@twins.programming.kicks-ass.net> User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Mon, 11 Jan 2016 12:44:48 +0200 Message-ID: <877fjg9yzj.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 Peter Zijlstra writes: > On Thu, Dec 17, 2015 at 04:07:32PM +0100, Peter Zijlstra wrote: >> On Thu, Dec 17, 2015 at 04:25:14PM +0200, Alexander Shishkin wrote: >> >> > That aside, why I brought it up in the first place is because the two >> > functions are asymmetric: one is called with irqs disabled and the >> > other -- with ctx::lock held (and not because I'm into bikeshedding or >> > anything like that). Looking at the pair of them sets off my "that's not >> > right" trigger and sends me to the event_function_call() >> > implementation. So in that sense, prepending an extra underscore kind of >> > made sense. Maybe __perf_remove_from_context_{on,off}()? >> >> You are quite right, and I think I've found more problems because of >> this. Let me prod at this some more. > > So this then... > > This fixes, I think, 3 separate bugs: > > - remove_from_context used to clear ->is_active, this is against the > update rules from ctx_sched_in() which set ->is_active even though > there might be !nr_events > > - install_in_context did bad things to cpuctx->task_ctx; it would not > validate that ctx->task == current and could do random things because > of that. > > - cpuctx->task_ctx tracking was iffy > > It also unifies a lot of the weird and fragile code we had around all > those IPI calls and adds a bunch of assertions. > > It seems to survive a little pounding with 'normal' workloads. > > Please have an extra careful look.. I notice that you dropped this from your queue, do you have any plans to proceed with this, or should I pick it up? Regards, -- Alex