Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932222AbbD1GXq (ORCPT ); Tue, 28 Apr 2015 02:23:46 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:33312 "EHLO mail-ob0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751503AbbD1GXo (ORCPT ); Tue, 28 Apr 2015 02:23:44 -0400 MIME-Version: 1.0 In-Reply-To: <37D7C6CF3E00A74B8858931C1DB2F077017EDC4C@SHSMSX103.ccr.corp.intel.com> References: <20150423054738.GA3722@thinkpad> <20150425043817.GM13605@tassilo.jf.intel.com> <37D7C6CF3E00A74B8858931C1DB2F077017EDC4C@SHSMSX103.ccr.corp.intel.com> Date: Mon, 27 Apr 2015 23:23:43 -0700 Message-ID: Subject: Re: [PATCH] perf/x86/intel/uncore: fix IMC missing box initialization From: Stephane Eranian To: "Liang, Kan" Cc: Andi Kleen , Bjorn Helgaas , Vince Weaver , LKML , Peter Zijlstra , "mingo@elte.hu" , Sonny Rao Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4215 Lines: 114 On Sun, Apr 26, 2015 at 8:43 PM, Liang, Kan wrote: > >> >> > This leads me to believe that this patch: >> > >> > commit c05199e5a57a579fea1e8fa65e2b511ceb524ffc >> > Author: Kan Liang >> > Date: Tue Jan 20 04:54:25 2015 +0000 >> > >> > perf/x86/intel/uncore: Move uncore_box_init() out of driver >> initialization >> > >> > If I revert it, I bet things will work again. >> >> Yes the initialization needs to be moved out of the IPI context. >> > > Maybe we can move them to event init, which is not in IPI context. > > What do you think of this patch? > > --- > > From 8a61c48144921e9d1c841656829c3bae9bfb4408 Mon Sep 17 00:00:00 2001 > From: Kan Liang > Date: Sun, 26 Apr 2015 16:24:59 -0400 > Subject: [PATCH 1/1] perf/x86/intel/uncore: move uncore_box_init to uncore > event init > > commit c05199e5a57a("perf/x86/intel/uncore: Move uncore_box_init() out > of driver initialization") moves uncore_box_init into uncore_enable_box > to prevent potential boot failures. However, uncore_enable_box is not > called on some client platforms (SNB/IVB/HSW) for counting IMC event. > When it is not called, the box is not initialized, which hard locks the > system. > > Additionally, uncore_enable_box along with the initialization code in it > is always called in uncore event start functions, which are in IPI > context. But the initizlization code should not be in IPI context. This > is because, for example, the IMC box initialization codes for client > platforms include ioremap, which is not allowed to be called in IPI > context. > > This patch moves uncore_box_init out of IPI context, to uncore event > init. The box is initialized only when it has not yet been initialized. > > Signed-off-by: Kan Liang Ok this works for me now. Thanks. Tested-by: Stephane Eranian > --- > arch/x86/kernel/cpu/perf_event_intel_uncore.c | 4 ++++ > arch/x86/kernel/cpu/perf_event_intel_uncore.h | 2 -- > arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c | 3 +++ > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c > index c635b8b..cbc1a93 100644 > --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c > +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c > @@ -623,6 +623,10 @@ static int uncore_pmu_event_init(struct perf_event *event) > box = uncore_pmu_to_box(pmu, event->cpu); > if (!box || box->cpu < 0) > return -EINVAL; > + > + /* Init box if it's not initialized yet */ > + uncore_box_init(box); > + > event->cpu = box->cpu; > > event->hw.idx = -1; > diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.h b/arch/x86/kernel/cpu/perf_event_intel_uncore.h > index 6c8c1e7..1fb2905 100644 > --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.h > +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h > @@ -273,8 +273,6 @@ static inline void uncore_disable_box(struct intel_uncore_box *box) > > static inline void uncore_enable_box(struct intel_uncore_box *box) > { > - uncore_box_init(box); > - > if (box->pmu->type->ops->enable_box) > box->pmu->type->ops->enable_box(box); > } > diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c b/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c > index 4562e9e..ead70a6 100644 > --- a/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c > +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c > @@ -279,6 +279,9 @@ static int snb_uncore_imc_event_init(struct perf_event *event) > if (!box || box->cpu < 0) > return -EINVAL; > > + /* Init box if it's not initialized yet */ > + uncore_box_init(box); > + > event->cpu = box->cpu; > > event->hw.idx = -1; > > Thanks, > Kan > >> -Andi >> >> >> -- >> ak@linux.intel.com -- Speaking for myself only -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/