From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756961AbcIGPUn (ORCPT ); Wed, 7 Sep 2016 11:20:43 -0400 Received: from mga09.intel.com ([134.134.136.24]:35466 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753990AbcIGPUl (ORCPT ); Wed, 7 Sep 2016 11:20:41 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,296,1470726000"; d="scan'208";a="165456665" From: Alexander Shishkin To: Ingo Molnar , Peter Zijlstra Cc: Peter Zijlstra , Ingo Molnar , linux-kernel@vger.kernel.org, vince@deater.net, eranian@google.com, Arnaldo Carvalho de Melo Subject: Re: [PATCH v2 0/5] perf, bts: Fallout from the fuzzer for perf/urgent In-Reply-To: <20160906171915.GA8963@gmail.com> References: <20160906132353.19887-1-alexander.shishkin@linux.intel.com> <20160906171915.GA8963@gmail.com> User-Agent: Notmuch/0.22.1 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Wed, 07 Sep 2016 18:20:37 +0300 Message-ID: <87mvjjka7u.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 Ingo Molnar writes: > * Alexander Shishkin wrote: > >> Hi, >> >> There were more bugs since the previous version, plus the BTS barriers got >> fixed. With these patches, my testcase keeps running and no spurious NMI >> warnings pop up any more. > > Could you please also run the fuzzer that Vince uses, does it now pass on hardware > you have access to? Sure. And yes, I did catch a warning, which calls for one more patch (below). Also one unrelated thing in PEBS that Peter fixed. > I'd like to make "passes the fuzzer" a standard requirement before new changes are > accepted to perf core. Let's make it so. For the sake of consistency, this one needs to go before 3/5. I'll re-send the whole series, though, if need be. I've got 2 perf_fuzzers running on this meanwhile. >>From c170c607bfdc3804578033faa43f342a5d95eb6c Mon Sep 17 00:00:00 2001 From: Alexander Shishkin Date: Wed, 7 Sep 2016 18:05:08 +0300 Subject: [PATCH] perf/x86/intel: Don't disable "intel_bts" around "intel" event batching At the moment, intel_bts events get disabled from intel PMU's disable callback, which includes event scheduling transactions of said PMU, which have nothing to do with intel_bts events. We do want to keep intel_bts events off inside the PMI handler to avoid filling up their buffer too soon. This patch moves intel_bts enabling/disabling directly to the PMI handler. Signed-off-by: Alexander Shishkin --- arch/x86/events/intel/core.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c index 88792f846d..e2d71513c9 100644 --- a/arch/x86/events/intel/core.c +++ b/arch/x86/events/intel/core.c @@ -1730,9 +1730,11 @@ static __initconst const u64 knl_hw_cache_extra_regs * disabled state if called consecutively. * * During consecutive calls, the same disable value will be written to related - * registers, so the PMU state remains unchanged. hw.state in - * intel_bts_disable_local will remain PERF_HES_STOPPED too in consecutive - * calls. + * registers, so the PMU state remains unchanged. + * + * intel_bts events don't coexist with intel pmu's BTS events because of + * x86_add_exclusive(x86_lbr_exclusive_lbr); there's no need to keep them + * disabled around intel pmu's event batching etc, only inside the PMI handler. */ static void __intel_pmu_disable_all(void) { @@ -1742,8 +1744,6 @@ static void __intel_pmu_disable_all(void) if (test_bit(INTEL_PMC_IDX_FIXED_BTS, cpuc->active_mask)) intel_pmu_disable_bts(); - else - intel_bts_disable_local(); intel_pmu_pebs_disable_all(); } @@ -1771,8 +1771,7 @@ static void __intel_pmu_enable_all(int added, bool pmi) return; intel_pmu_enable_bts(event->hw.config); - } else - intel_bts_enable_local(); + } } static void intel_pmu_enable_all(int added) @@ -2076,6 +2075,7 @@ static int intel_pmu_handle_irq(struct pt_regs *regs) */ if (!x86_pmu.late_ack) apic_write(APIC_LVTPC, APIC_DM_NMI); + intel_bts_disable_local(); __intel_pmu_disable_all(); handled = intel_pmu_drain_bts_buffer(); handled += intel_bts_interrupt(); @@ -2175,6 +2175,7 @@ done: /* Only restore PMU state when it's active. See x86_pmu_disable(). */ if (cpuc->enabled) __intel_pmu_enable_all(0, true); + intel_bts_enable_local(); /* * Only unmask the NMI after the overflow counters -- 2.9.3