path: root/arch/x86/events
diff options
authorKan Liang <kan.liang@linux.intel.com>2020-01-21 10:13:38 -0800
committerIngo Molnar <mingo@kernel.org>2020-02-11 13:23:48 +0100
commit6c1c07b33eb093e5a2a313ece89baa596ba6135e (patch)
treeb7f85e52634693a16c4709ab37cf2de8483011c3 /arch/x86/events
parentf861854e1b435b27197417f6f90d87188003cb24 (diff)
perf/x86/intel: Avoid unnecessary PEBS_ENABLE MSR access in PMI
The perf PMI handler, intel_pmu_handle_irq(), currently does unnecessary MSR accesses for PEBS_ENABLE MSR in __intel_pmu_enable/disable_all() when PEBS is enabled. When entering the handler, global ctrl is explicitly disabled. All counters do not count anymore. It doesn't matter if PEBS is enabled or not in a PMI handler. Furthermore, for most cases, the cpuc->pebs_enabled is not changed in PMI. The PEBS status doesn't change. The PEBS_ENABLE MSR doesn't need to be changed either when exiting the handler. PMI throttle may change the PEBS status during PMI handler. The x86_pmu_stop() ends up in intel_pmu_pebs_disable() which can update cpuc->pebs_enabled. But the MSR_IA32_PEBS_ENABLE is not updated at the same time. Because the cpuc->enabled has been forced to 0. The patch explicitly update the MSR_IA32_PEBS_ENABLE for this case. Use ftrace to measure the duration of intel_pmu_handle_irq() on BDX. #perf record -e cycles:P -- ./tchain_edit The average duration of intel_pmu_handle_irq(): Without the patch 1.144 us With the patch 1.025 us Signed-off-by: Kan Liang <kan.liang@linux.intel.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Signed-off-by: Ingo Molnar <mingo@kernel.org> Link: https://lkml.kernel.org/r/20200121181338.3234-1-kan.liang@linux.intel.com
Diffstat (limited to 'arch/x86/events')
1 files changed, 22 insertions, 3 deletions
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index dff6623804c2..332954cccece 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -1945,6 +1945,14 @@ static __initconst const u64 knl_hw_cache_extra_regs
* 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.
+ *
+ * Avoid PEBS_ENABLE MSR access in PMIs.
+ * The GLOBAL_CTRL has been disabled. All the counters do not count anymore.
+ * It doesn't matter if the PEBS is enabled or not.
+ * Usually, the PEBS status are not changed in PMIs. It's unnecessary to
+ * access PEBS_ENABLE MSR in disable_all()/enable_all().
+ * However, there are some cases which may change PEBS status, e.g. PMI
+ * throttle. The PEBS_ENABLE should be updated where the status changes.
static void __intel_pmu_disable_all(void)
@@ -1954,13 +1962,12 @@ static void __intel_pmu_disable_all(void)
if (test_bit(INTEL_PMC_IDX_FIXED_BTS, cpuc->active_mask))
- intel_pmu_pebs_disable_all();
static void intel_pmu_disable_all(void)
+ intel_pmu_pebs_disable_all();
@@ -1968,7 +1975,6 @@ static void __intel_pmu_enable_all(int added, bool pmi)
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
- intel_pmu_pebs_enable_all();
x86_pmu.intel_ctrl & ~cpuc->intel_ctrl_guest_mask);
@@ -1986,6 +1992,7 @@ static void __intel_pmu_enable_all(int added, bool pmi)
static void intel_pmu_enable_all(int added)
+ intel_pmu_pebs_enable_all();
__intel_pmu_enable_all(added, false);
@@ -2374,9 +2381,21 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
* PEBS overflow sets bit 62 in the global status register
if (__test_and_clear_bit(62, (unsigned long *)&status)) {
+ u64 pebs_enabled = cpuc->pebs_enabled;
status &= x86_pmu.intel_ctrl | GLOBAL_STATUS_TRACE_TOPAPMI;
+ /*
+ * PMI throttle may be triggered, which stops the PEBS event.
+ * Although cpuc->pebs_enabled is updated accordingly, the
+ * MSR_IA32_PEBS_ENABLE is not updated. Because the
+ * cpuc->enabled has been forced to 0 in PMI.
+ * Update the MSR if pebs_enabled is changed.
+ */
+ if (pebs_enabled != cpuc->pebs_enabled)
+ wrmsrl(MSR_IA32_PEBS_ENABLE, cpuc->pebs_enabled);