From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
To: Ingo Molnar <mingo@kernel.org>, Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
Ingo Molnar <mingo@redhat.com>,
linux-kernel@vger.kernel.org, vince@deater.net,
eranian@google.com, Arnaldo Carvalho de Melo <acme@infradead.org>
Subject: Re: [PATCH v2 0/5] perf, bts: Fallout from the fuzzer for perf/urgent
Date: Wed, 07 Sep 2016 18:20:37 +0300 [thread overview]
Message-ID: <87mvjjka7u.fsf@ashishki-desk.ger.corp.intel.com> (raw)
In-Reply-To: <20160906171915.GA8963@gmail.com>
Ingo Molnar <mingo@kernel.org> writes:
> * Alexander Shishkin <alexander.shishkin@linux.intel.com> 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 <alexander.shishkin@linux.intel.com>
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 <alexander.shishkin@linux.intel.com>
---
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
next prev parent reply other threads:[~2016-09-07 15:20 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-06 13:23 [PATCH v2 0/5] perf, bts: Fallout from the fuzzer for perf/urgent Alexander Shishkin
2016-09-06 13:23 ` [PATCH v2 1/5] perf: Fix a race between mmap_close and set_output of AUX events Alexander Shishkin
2016-09-10 12:38 ` [tip:perf/core] perf/core: Fix a race between mmap_close() and set_output() " tip-bot for Alexander Shishkin
2016-09-06 13:23 ` [PATCH v2 2/5] perf: Fix aux_mmap_count vs aux_refcount order Alexander Shishkin
2016-09-10 12:38 ` [tip:perf/core] perf/core: " tip-bot for Alexander Shishkin
2016-09-06 13:23 ` [PATCH v2 3/5] perf/x86/intel/bts: Fix confused ordering of PMU callbacks Alexander Shishkin
2016-09-10 12:39 ` [tip:perf/core] " tip-bot for Alexander Shishkin
2016-09-06 13:23 ` [PATCH v2 4/5] perf/x86/intel/bts: Fix BTS PMI detection Alexander Shishkin
2016-09-10 12:39 ` [tip:perf/core] " tip-bot for Alexander Shishkin
2016-09-20 13:12 ` [PATCH] perf/x86/intel/bts: don't dereference ds unconditionally Sebastian Andrzej Siewior
2016-09-20 13:44 ` Alexander Shishkin
2016-09-20 13:54 ` Alexander Shishkin
2016-09-20 14:13 ` [tip:perf/urgent] perf/x86/intel/bts: Make sure debug store is valid tip-bot for Sebastian Andrzej Siewior
2016-09-06 13:23 ` [PATCH v2 5/5] perf/x86/intel/bts: Kill a silly warning Alexander Shishkin
2016-09-10 12:40 ` [tip:perf/core] " tip-bot for Alexander Shishkin
2016-09-06 17:19 ` [PATCH v2 0/5] perf, bts: Fallout from the fuzzer for perf/urgent Ingo Molnar
2016-09-07 0:13 ` Vince Weaver
2016-09-07 15:20 ` Alexander Shishkin [this message]
2016-09-07 15:36 ` Vince Weaver
2016-09-07 16:38 ` Peter Zijlstra
2016-09-07 18:33 ` Alexander Shishkin
2016-09-08 3:36 ` Vince Weaver
2016-09-08 8:51 ` Alexander Shishkin
2016-09-08 12:54 ` Vince Weaver
2016-09-08 6:21 ` Ingo Molnar
2016-09-08 6:23 ` Ingo Molnar
2016-09-08 8:43 ` Alexander Shishkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87mvjjka7u.fsf@ashishki-desk.ger.corp.intel.com \
--to=alexander.shishkin@linux.intel.com \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@infradead.org \
--cc=eranian@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=vince@deater.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).