Re: [PATCH 2/3] x86/topology: Fix AMD core count

From: Peter Zijlstra
Date: Sun Mar 20 2016 - 08:48:22 EST


On Sun, Mar 20, 2016 at 01:32:25PM +0100, Peter Zijlstra wrote:
> Yes, but IIRC the F15h driver doesn't use the NB constraints, as they
> moved all the NB events to their own set of MSRs, which has
> events/amd/uncore.c.
>
> So all the NB cruft in the core pmu is only relevant to F10h.
>
> So F15h also calling and allocating NB cruft in the core PMU driver is
> entirely pointless.

Something like so.

---
arch/x86/events/amd/core.c | 21 ++++++++++++++++++---
arch/x86/events/perf_event.h | 5 +++++
2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 049ada8d4e9c..62026a79a930 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -369,7 +369,7 @@ static int amd_pmu_cpu_prepare(int cpu)

WARN_ON_ONCE(cpuc->amd_nb);

- if (boot_cpu_data.x86_max_cores < 2)
+ if (!x86_pmu.amd_nb)
return NOTIFY_OK;

cpuc->amd_nb = amd_alloc_nb(cpu);
@@ -388,7 +388,7 @@ static void amd_pmu_cpu_starting(int cpu)

cpuc->perf_ctr_virt_mask = AMD64_EVENTSEL_HOSTONLY;

- if (boot_cpu_data.x86_max_cores < 2)
+ if (!x86_pmu.amd_nb)
return;

nb_id = amd_get_nb_id(cpu);
@@ -414,7 +414,7 @@ static void amd_pmu_cpu_dead(int cpu)
{
struct cpu_hw_events *cpuhw;

- if (boot_cpu_data.x86_max_cores < 2)
+ if (!x86_pmu.amd_nb)
return;

cpuhw = &per_cpu(cpu_hw_events, cpu);
@@ -648,6 +648,8 @@ static __initconst const struct x86_pmu amd_pmu = {
.cpu_prepare = amd_pmu_cpu_prepare,
.cpu_starting = amd_pmu_cpu_starting,
.cpu_dead = amd_pmu_cpu_dead,
+
+ .amd_nb = 1;
};

static int __init amd_core_pmu_init(void)
@@ -674,6 +676,11 @@ static int __init amd_core_pmu_init(void)
x86_pmu.eventsel = MSR_F15H_PERF_CTL;
x86_pmu.perfctr = MSR_F15H_PERF_CTR;
x86_pmu.num_counters = AMD64_NUM_COUNTERS_CORE;
+ /*
+ * AMD Core perfctr has separate MSRs for the NB events, see
+ * the amd/uncore.c driver.
+ */
+ x86_pmu.amd_nb = 0;

pr_cont("core perfctr, ");
return 0;
@@ -693,6 +700,14 @@ __init int amd_pmu_init(void)
if (ret)
return ret;

+ if (num_possible_cpus() == 1) {
+ /*
+ * No point in allocating data structures to serialize
+ * against other CPUs, when there is only the one CPU.
+ */
+ x86_pmu.amd_nb = 0;
+ }
+
/* Events are common for all AMDs */
memcpy(hw_cache_event_ids, amd_hw_cache_event_ids,
sizeof(hw_cache_event_ids));
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index ba6ef18528c9..46d2ece10a7b 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -608,6 +608,11 @@ struct x86_pmu {
atomic_t lbr_exclusive[x86_lbr_exclusive_max];

/*
+ * AMD bits
+ */
+ unsigned int amd_nb : 1;
+
+ /*
* Extra registers for events
*/
struct extra_reg *extra_regs;