From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754568Ab3KKQRH (ORCPT ); Mon, 11 Nov 2013 11:17:07 -0500 Received: from merlin.infradead.org ([205.233.59.134]:37929 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754497Ab3KKQQ6 (ORCPT ); Mon, 11 Nov 2013 11:16:58 -0500 Date: Mon, 11 Nov 2013 17:16:31 +0100 From: Peter Zijlstra To: Stephane Eranian Cc: LKML , "mingo@elte.hu" , "ak@linux.intel.com" , Arnaldo Carvalho de Melo , Jiri Olsa , "Yan, Zheng" , Borislav Petkov , Maria Dimakopoulou Subject: Re: [PATCH v6 3/4] perf,x86: add Intel RAPL PMU support Message-ID: <20131111161631.GA21461@twins.programming.kicks-ass.net> References: <1383939963-7750-1-git-send-email-eranian@google.com> <1383939963-7750-4-git-send-email-eranian@google.com> <20131111155905.GM19203@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 11, 2013 at 05:07:22PM +0100, Stephane Eranian wrote: > On Mon, Nov 11, 2013 at 4:59 PM, Peter Zijlstra wrote: > > On Fri, Nov 08, 2013 at 08:46:02PM +0100, Stephane Eranian wrote: > > > > > >> +static DEFINE_SPINLOCK(rapl_hotplug_lock); > > > > What is this thing protecting? > > > > Like last time it appears to be used only from hotplug notifier > > callbacks and those are already fully serialized. > > > I thought you or somebody else had said, that hotplug in now parallel. There's a little something about hotplug here I think: http://marc.info/?l=linux-kernel&m=138118235612441 But I'm not entirely sure that's the origin of this lock. Andi seems worried about the IPI vs hotplug/migrate. I'm not entrely sure the lock as proposed actually fixes that. The 'easy' way would be to put a synchronize_sched() right after the migrate code, that forces all CPUs to have scheduled once and thus also ensures all pending/running interrupts are serviced. But I don't think that's strictly required as I seem to recall hotplug actually already does something like this -- it pretty much has to.