[Intel-gfx] [PATCH 1/2] drm/i915/pmu: Avoid using globals for CPU hotplug state

Michał Winiarski michal.winiarski at intel.com
Wed Feb 19 16:18:21 UTC 2020


Attempting to bind / unbind module from devices where we have both
integrated and discreete GPU handled by i915 can lead to leaks and
warnings from cpuhp:
Error: Removing state XXX which has instances left.

Let's move the state to i915_pmu.

Fixes: 05488673a4d4 ("drm/i915/pmu: Support multiple GPUs")
Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_pmu.c | 18 +++++++++---------
 drivers/gpu/drm/i915/i915_pmu.h |  7 +++++--
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index a3b61fb96226..12f4818c78b8 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -1042,7 +1042,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), node);
+	struct i915_pmu *pmu = hlist_entry_safe(node, typeof(*pmu), cpuhp.node);
 
 	GEM_BUG_ON(!pmu->base.event_init);
 
@@ -1055,7 +1055,7 @@ 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), node);
+	struct i915_pmu *pmu = hlist_entry_safe(node, typeof(*pmu), cpuhp.node);
 	unsigned int target;
 
 	GEM_BUG_ON(!pmu->base.event_init);
@@ -1072,8 +1072,6 @@ static int i915_pmu_cpu_offline(unsigned int cpu, struct hlist_node *node)
 	return 0;
 }
 
-static enum cpuhp_state cpuhp_slot = CPUHP_INVALID;
-
 static int i915_pmu_register_cpuhp_state(struct i915_pmu *pmu)
 {
 	enum cpuhp_state slot;
@@ -1087,21 +1085,22 @@ static int i915_pmu_register_cpuhp_state(struct i915_pmu *pmu)
 		return ret;
 
 	slot = ret;
-	ret = cpuhp_state_add_instance(slot, &pmu->node);
+	ret = cpuhp_state_add_instance(slot, &pmu->cpuhp.node);
 	if (ret) {
 		cpuhp_remove_multi_state(slot);
 		return ret;
 	}
 
-	cpuhp_slot = slot;
+	pmu->cpuhp.slot = slot;
 	return 0;
 }
 
 static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
 {
-	WARN_ON(cpuhp_slot == CPUHP_INVALID);
-	WARN_ON(cpuhp_state_remove_instance(cpuhp_slot, &pmu->node));
-	cpuhp_remove_multi_state(cpuhp_slot);
+	WARN_ON(pmu->cpuhp.slot == CPUHP_INVALID);
+	WARN_ON(cpuhp_state_remove_instance(pmu->cpuhp.slot, &pmu->cpuhp.node));
+	cpuhp_remove_multi_state(pmu->cpuhp.slot);
+	pmu->cpuhp.slot = CPUHP_INVALID;
 }
 
 static bool is_igp(struct drm_i915_private *i915)
@@ -1128,6 +1127,7 @@ 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 6c1647c5daf2..207058391cec 100644
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -39,9 +39,12 @@ struct i915_pmu_sample {
 
 struct i915_pmu {
 	/**
-	 * @node: List node for CPU hotplug handling.
+	 * @cpuhp: Struct used for CPU hotplug handling.
 	 */
-	struct hlist_node node;
+	struct {
+		struct hlist_node node;
+		enum cpuhp_state slot;
+	} cpuhp;
 	/**
 	 * @base: PMU base.
 	 */
-- 
2.21.1



More information about the Intel-gfx mailing list