[PATCH 2/2] drm/i915/pmu: Handle CPU hotplug more efficiently

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Dynamic CPU hotplug notification slots are a scarce resource and given
how we already register the multi instance type state, we can and should
add multiple instance of the i915 PMU to this same state and not allocate
a new one every time.

At the same time this also fixes the event migration when multiple GPUs
are present by making sure events are migrated for not just the first
callback against i915 cpu hotplug state.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Suggested-by: Daniel Vetter <daniel.vetter at intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 103 +++++++++++++++++++++-----------
 drivers/gpu/drm/i915/i915_pmu.h |   7 +--
 2 files changed, 70 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 5d1b38e66afb..c4442b0e7a4b 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -30,6 +30,7 @@
 #define ENGINE_SAMPLE_BITS (1 << I915_PMU_SAMPLE_BITS)
 
 static cpumask_t i915_pmu_cpumask;
+static unsigned int i915_pmu_target_cpu;
 
 static u8 engine_config_sample(u64 config)
 {
@@ -1036,7 +1037,7 @@ static void free_event_attributes(struct i915_pmu *pmu)
 
 static int i915_pmu_cpu_online(unsigned int cpu, struct hlist_node *node)
 {
-	struct i915_pmu *pmu = hlist_entry_safe(node, typeof(*pmu), cpuhp.node);
+	struct i915_pmu *pmu = hlist_entry_safe(node, typeof(*pmu), cpuhp_node);
 
 	GEM_BUG_ON(!pmu->base.event_init);
 
@@ -1049,54 +1050,87 @@ static int i915_pmu_cpu_online(unsigned int cpu, struct hlist_node *node)
 
 static int i915_pmu_cpu_offline(unsigned int cpu, struct hlist_node *node)
 {
-	struct i915_pmu *pmu = hlist_entry_safe(node, typeof(*pmu), cpuhp.node);
-	unsigned int target;
+	if (cpumask_test_and_clear_cpu(cpu, &i915_pmu_cpumask)) {
+		unsigned int target =
+			cpumask_any_but(topology_sibling_cpumask(cpu), cpu);
+
+			/* Migrate events if there is a valid target */
+			if (target < nr_cpu_ids) {
+				cpumask_set_cpu(target, &i915_pmu_cpumask);
+				i915_pmu_target_cpu = target;
+			}
+	}
 
-	GEM_BUG_ON(!pmu->base.event_init);
+	if (i915_pmu_target_cpu < nr_cpu_ids) {
+		struct i915_pmu *pmu =
+			hlist_entry_safe(node, typeof(*pmu), cpuhp_node);
 
-	if (cpumask_test_and_clear_cpu(cpu, &i915_pmu_cpumask)) {
-		target = cpumask_any_but(topology_sibling_cpumask(cpu), cpu);
-		/* Migrate events if there is a valid target */
-		if (target < nr_cpu_ids) {
-			cpumask_set_cpu(target, &i915_pmu_cpumask);
-			perf_pmu_migrate_context(&pmu->base, cpu, target);
-		}
+		GEM_BUG_ON(!pmu->base.event_init);
+
+		if (cpu != i915_pmu_target_cpu)
+			perf_pmu_migrate_context(&pmu->base, cpu,
+						 i915_pmu_target_cpu);
+		else
+			WARN_ON_ONCE(1);
 	}
 
 	return 0;
 }
 
-static int i915_pmu_register_cpuhp_state(struct i915_pmu *pmu)
-{
-	enum cpuhp_state slot;
-	int ret;
-
-	ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
-				      "perf/x86/intel/i915:online",
-				      i915_pmu_cpu_online,
-				      i915_pmu_cpu_offline);
-	if (ret < 0)
-		return ret;
+static DEFINE_MUTEX(cpuhp_mutex);
+static unsigned int cpuhp_ref;
+static enum cpuhp_state cpuhp_slot = CPUHP_INVALID;
 
-	slot = ret;
-	ret = cpuhp_state_add_instance(slot, &pmu->cpuhp.node);
-	if (ret) {
-		cpuhp_remove_multi_state(slot);
-		return ret;
+static void __unregister_state(void)
+{
+	mutex_lock(&cpuhp_mutex);
+	if (--cpuhp_ref == 0) {
+		cpuhp_remove_multi_state(cpuhp_slot);
+		cpuhp_slot = CPUHP_INVALID;
 	}
-
-	pmu->cpuhp.slot = slot;
-	return 0;
+	mutex_unlock(&cpuhp_mutex);
 }
 
 static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
 {
 	struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
 
-	drm_WARN_ON(&i915->drm, pmu->cpuhp.slot == CPUHP_INVALID);
-	drm_WARN_ON(&i915->drm, cpuhp_state_remove_instance(pmu->cpuhp.slot, &pmu->cpuhp.node));
-	cpuhp_remove_multi_state(pmu->cpuhp.slot);
-	pmu->cpuhp.slot = CPUHP_INVALID;
+	drm_WARN_ON(&i915->drm, cpuhp_slot == CPUHP_INVALID);
+
+	if (drm_WARN_ON(&i915->drm,
+			cpuhp_state_remove_instance(cpuhp_slot,
+						    &pmu->cpuhp_node)))
+		return;
+
+	__unregister_state();
+}
+
+static int i915_pmu_register_cpuhp_state(struct i915_pmu *pmu)
+{
+	int ret;
+
+	mutex_lock(&cpuhp_mutex);
+	if (cpuhp_ref == 0) {
+		ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
+					      "perf/x86/intel/i915:online",
+					      i915_pmu_cpu_online,
+					      i915_pmu_cpu_offline);
+		if (ret < 0) {
+			mutex_unlock(&cpuhp_mutex);
+			return ret;
+		}
+
+		GEM_BUG_ON(cpuhp_slot != CPUHP_INVALID);
+		cpuhp_slot = ret;
+		cpuhp_ref++;
+	}
+	mutex_unlock(&cpuhp_mutex);
+
+	ret = cpuhp_state_add_instance(cpuhp_slot, &pmu->cpuhp_node);
+	if (ret)
+		__unregister_state();
+
+	return ret;
 }
 
 static bool is_igp(struct drm_i915_private *i915)
@@ -1130,7 +1164,6 @@ void i915_pmu_register(struct drm_i915_private *i915)
 	spin_lock_init(&pmu->lock);
 	hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	pmu->timer.function = i915_sample;
-	pmu->cpuhp.slot = CPUHP_INVALID;
 
 	if (!is_igp(i915)) {
 		pmu->name = kasprintf(GFP_KERNEL,
diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
index 59a0d19afb67..02c220614aff 100644
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -39,12 +39,9 @@ struct i915_pmu_sample {
 
 struct i915_pmu {
 	/**
-	 * @cpuhp: Struct used for CPU hotplug handling.
+	 * @cpuhp_node: PMU node for CPU hotplug handling.
 	 */
-	struct {
-		struct hlist_node node;
-		enum cpuhp_state slot;
-	} cpuhp;
+	struct hlist_node cpuhp_node;
 	/**
 	 * @base: PMU base.
 	 */
-- 
2.25.1