perf: perf_fuzzer still triggers bts warning

perf: perf_fuzzer still triggers bts warning

[Vince Weaver]

Hello

I'm running 4.8-rc6 git from this morning (with the various perf fixes).

Fuzzing on Skylake I still managed to trigger the following warning.

It maps to

void intel_bts_enable_local(void)
{
...
        /*
         * Here we transition from INACTIVE to ACTIVE;
         * if we instead are STOPPED from the interrupt handler,
         * stay that way. Can't be ACTIVE here though.
         */
        if (WARN_ON_ONCE(state == BTS_STATE_ACTIVE))
                return;

[ 4070.994175] ------------[ cut here ]------------
[ 4070.994469] WARNING: CPU: 0 PID: 25484 at arch/x86/events/intel/bts.c:344 intel_bts_enable_local+0x58/0x60
[ 4070.995428] CPU: 0 PID: 25484 Comm: perf_fuzzer Not tainted 4.8.0-rc6+ #5
[ 4070.995430] Hardware name: LENOVO 10FY0017US/SKYBAY, BIOS FWKT53A   06/06/2016
[ 4070.995433]  0000000000000086 00000000dfbb88aa ffff9045fdc03dc8 ffffffff93b3b863
[ 4070.995438]  0000000000000000 0000000000000000 ffff9045fdc03e08 ffffffff9387f7d1
[ 4070.995443]  00000158fdc03e08 ffff9045fdc0a480 0000000000000000 ffff9045fdc0a480
[ 4070.995448] Call Trace:
[ 4070.995450]  <IRQ>  [<ffffffff93b3b863>] dump_stack+0x63/0x90
[ 4070.995465]  [<ffffffff9387f7d1>] __warn+0xd1/0xf0
[ 4070.995471]  [<ffffffff9387f8fd>] warn_slowpath_null+0x1d/0x20
[ 4070.995474]  [<ffffffff9380d8b8>] intel_bts_enable_local+0x58/0x60
[ 4070.995478]  [<ffffffff9380b2a0>] __intel_pmu_enable_all+0x80/0xb0
[ 4070.995481]  [<ffffffff9380b2e0>] intel_pmu_enable_all+0x10/0x20
[ 4070.995486]  [<ffffffff93807741>] x86_pmu_enable+0x261/0x2f0
[ 4070.995491]  [<ffffffff93976950>] ? __perf_install_in_context+0x110/0x110
[ 4070.995494]  [<ffffffff93974ff2>] perf_pmu_enable+0x22/0x30
[ 4070.995498]  [<ffffffff93976a61>] perf_mux_hrtimer_handler+0x111/0x1c0
[ 4070.995504]  [<ffffffff938ef633>] __hrtimer_run_queues+0xf3/0x280
[ 4070.995509]  [<ffffffff938efb08>] hrtimer_interrupt+0xa8/0x1a0
[ 4070.995515]  [<ffffffff93852d88>] local_apic_timer_interrupt+0x38/0x60
[ 4070.995521]  [<ffffffff93e1de8d>] smp_apic_timer_interrupt+0x3d/0x50
[ 4070.995525]  [<ffffffff93e1d1a2>] apic_timer_interrupt+0x82/0x90
[ 4070.995526]  <EOI> 
[ 4070.995529] ---[ end trace 6247e022342bacf7 ]---

Re: perf: perf_fuzzer still triggers bts warning

[Vince Weaver]

Probably not related but it also triggered this just now:

static void x86_pmu_start(struct perf_event *event, int flags)
{
        struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
        int idx = event->hw.idx;

        if (WARN_ON_ONCE(!(event->hw.state & PERF_HES_STOPPED)))
                return;


[ 7501.295830] ------------[ cut here ]------------
[ 7501.295844] WARNING: CPU: 2 PID: 7166 at arch/x86/events/core.c:1236 x86_pmu_start+0xae/0x100
[ 7501.295948] CPU: 2 PID: 7166 Comm: perf_fuzzer Tainted: G        W       4.8.0-rc6+ #5
[ 7501.295950] Hardware name: LENOVO 10FY0017US/SKYBAY, BIOS FWKT53A   06/06/2016
[ 7501.295952]  0000000000000086 000000006d6df8e9 ffff9045fdc83d70 ffffffff93b3b863
[ 7501.295958]  0000000000000000 0000000000000000 ffff9045fdc83db0 ffffffff9387f7d1
[ 7501.295963]  000004d4ee352100 ffff9045fdc8a480 ffff9045f0d18800 0000000000000001
[ 7501.295968] Call Trace:
[ 7501.295970]  <IRQ>  [<ffffffff93b3b863>] dump_stack+0x63/0x90
[ 7501.295985]  [<ffffffff9387f7d1>] __warn+0xd1/0xf0
[ 7501.295989]  [<ffffffff9387f8fd>] warn_slowpath_null+0x1d/0x20
[ 7501.295994]  [<ffffffff93806f9e>] x86_pmu_start+0xae/0x100
[ 7501.295999]  [<ffffffff93977050>] perf_event_task_tick+0x1f0/0x2c0
[ 7501.296004]  [<ffffffff938abbc8>] scheduler_tick+0x78/0xd0
[ 7501.296008]  [<ffffffff938fe8b0>] ? tick_sched_do_timer+0x30/0x30
[ 7501.296014]  [<ffffffff938ee8a7>] update_process_times+0x47/0x60
[ 7501.296017]  [<ffffffff938fe285>] tick_sched_handle.isra.13+0x25/0x60
[ 7501.296020]  [<ffffffff938fe8ed>] tick_sched_timer+0x3d/0x70
[ 7501.296025]  [<ffffffff938ef633>] __hrtimer_run_queues+0xf3/0x280
[ 7501.296030]  [<ffffffff938efb08>] hrtimer_interrupt+0xa8/0x1a0
[ 7501.296035]  [<ffffffff93852d88>] local_apic_timer_interrupt+0x38/0x60
[ 7501.296040]  [<ffffffff93e1de8d>] smp_apic_timer_interrupt+0x3d/0x50
[ 7501.296045]  [<ffffffff93e1d1a2>] apic_timer_interrupt+0x82/0x90
[ 7501.296046]  <EOI>  [<ffffffff939bc799>] ? vm_normal_page+0x9/0xa0
[ 7501.296057]  [<ffffffff939bd637>] ? unmap_page_range+0x557/0x930
[ 7501.296062]  [<ffffffff939bda8d>] unmap_single_vma+0x7d/0xe0
[ 7501.296066]  [<ffffffff939bddda>] unmap_vmas+0x4a/0xa0
[ 7501.296070]  [<ffffffff939c6687>] exit_mmap+0xa7/0x170
[ 7501.296075]  [<ffffffff9387c962>] mmput+0x62/0xf0
[ 7501.296078]  [<ffffffff93883739>] do_exit+0x339/0xb60
[ 7501.296082]  [<ffffffff93883fe3>] do_group_exit+0x43/0xc0
[ 7501.296085]  [<ffffffff9388f4f8>] get_signal+0x2b8/0x6b0
[ 7501.296090]  [<ffffffff9382d5d7>] do_signal+0x37/0x7c0
[ 7501.296094]  [<ffffffff939c0b2c>] ? handle_mm_fault+0xb0c/0x15d0
[ 7501.296099]  [<ffffffff938373d9>] ? sched_clock+0x9/0x10
[ 7501.296102]  [<ffffffff938373d9>] ? sched_clock+0x9/0x10
[ 7501.296107]  [<ffffffff93831761>] ? nmi_handle+0x71/0x120
[ 7501.296111]  [<ffffffff9380358c>] exit_to_usermode_loop+0x8c/0xd0
[ 7501.296115]  [<ffffffff93803a36>] prepare_exit_to_usermode+0x26/0x30
[ 7501.296118]  [<ffffffff93e1bee5>] retint_user+0x8/0x13
[ 7501.296121] ---[ end trace 6247e022342bacf8 ]---

[PATCH] perf/x86/intel: Don't disable "intel_bts" around "intel" event batching

[Alexander Shishkin]

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

Re: perf: perf_fuzzer still triggers bts warning

[Alexander Shishkin]

Vince Weaver <vincent.weaver@maine.edu> writes:

> Hello
>
> I'm running 4.8-rc6 git from this morning (with the various perf fixes).
>
> Fuzzing on Skylake I still managed to trigger the following warning.

I was sure that I'd sent the fix for this one in the earlier series, but
somehow I missed it. Sending now.

Thanks,
--
Alex

[tip:perf/urgent] perf/x86/intel: Don't disable "intel_bts" around "intel" event batching

[tip-bot for Alexander Shishkin]

Commit-ID:  cecf62352aee2b4fe114aafd1b8c5f265a4243ce
Gitweb:     http://git.kernel.org/tip/cecf62352aee2b4fe114aafd1b8c5f265a4243ce
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Thu, 15 Sep 2016 11:22:33 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 15 Sep 2016 11:25:26 +0200

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.

Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: vince@deater.net
Link: http://lkml.kernel.org/r/20160915082233.11065-1-alexander.shishkin@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 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 2cbde2f..4c9a79b 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)
@@ -2073,6 +2072,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();
@@ -2172,6 +2172,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