Skip to content

Commit 87b593d

Browse files
committed
drm/i915/pmu: Drop custom hotplug code
Since commit 4ba4f1a ("perf: Generic hotplug support for a PMU with a scope"), there's generic support for system-wide counters and integration with cpu hotplug. The i915 counters are system-wide, even though the migration code is using the wrong topology mask: target = cpumask_any_but(topology_sibling_cpumask(cpu), cpu); So one could think the counter has core scope rather than system scope, which is not the case. That was never caught in tests since they would disable just one cpu at a time. After the removal of hotpluggable CPU0 in commit 5475abb ("x86/smpboot: Remove the CPU0 hotplug kludge") and commit e59e74d ("x86/topology: Remove CPU0 hotplug option") this became essentially non-testable for our systems. Using the generic hotplug code, the same cpu0 is still reported in cpumask - only if it was possible to unplug it, it would migrate to another cpu, so there isn't much a change in behavior. The one thing that changes is the return code for perf_event_open() if an invalid cpu is used: previously i915 would return -EINVAL, but generic perf code returns -ENODEV. That should be ok for all the users and not cause breakages. Cc: Kan Liang <kan.liang@linux.intel.com> Cc: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com> Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> Reviewed-by: Kan Liang <kan.liang@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20250131231304.4151998-2-lucas.demarchi@intel.com Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
1 parent c771600 commit 87b593d

File tree

3 files changed

+1
-126
lines changed

3 files changed

+1
-126
lines changed

drivers/gpu/drm/i915/i915_module.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,6 @@ static const struct {
7171
{ .init = i915_vma_resource_module_init,
7272
.exit = i915_vma_resource_module_exit },
7373
{ .init = i915_mock_selftests },
74-
{ .init = i915_pmu_init,
75-
.exit = i915_pmu_exit },
7674
{ .init = i915_pci_register_driver,
7775
.exit = i915_pci_unregister_driver },
7876
{ .init = i915_perf_sysctl_register,

drivers/gpu/drm/i915/i915_pmu.c

Lines changed: 1 addition & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@
2828
BIT(I915_SAMPLE_WAIT) | \
2929
BIT(I915_SAMPLE_SEMA))
3030

31-
static cpumask_t i915_pmu_cpumask;
32-
static unsigned int i915_pmu_target_cpu = -1;
33-
3431
static struct i915_pmu *event_to_pmu(struct perf_event *event)
3532
{
3633
return container_of(event->pmu, struct i915_pmu, base);
@@ -642,10 +639,6 @@ static int i915_pmu_event_init(struct perf_event *event)
642639
if (event->cpu < 0)
643640
return -EINVAL;
644641

645-
/* only allow running on one cpu at a time */
646-
if (!cpumask_test_cpu(event->cpu, &i915_pmu_cpumask))
647-
return -EINVAL;
648-
649642
if (is_engine_event(event))
650643
ret = engine_event_init(event);
651644
else
@@ -935,23 +928,6 @@ static ssize_t i915_pmu_event_show(struct device *dev,
935928
return sprintf(buf, "config=0x%lx\n", eattr->val);
936929
}
937930

938-
static ssize_t cpumask_show(struct device *dev,
939-
struct device_attribute *attr, char *buf)
940-
{
941-
return cpumap_print_to_pagebuf(true, buf, &i915_pmu_cpumask);
942-
}
943-
944-
static DEVICE_ATTR_RO(cpumask);
945-
946-
static struct attribute *i915_cpumask_attrs[] = {
947-
&dev_attr_cpumask.attr,
948-
NULL,
949-
};
950-
951-
static const struct attribute_group i915_pmu_cpumask_attr_group = {
952-
.attrs = i915_cpumask_attrs,
953-
};
954-
955931
#define __event(__counter, __name, __unit) \
956932
{ \
957933
.counter = (__counter), \
@@ -1168,100 +1144,19 @@ static void free_event_attributes(struct i915_pmu *pmu)
11681144
pmu->pmu_attr = NULL;
11691145
}
11701146

1171-
static int i915_pmu_cpu_online(unsigned int cpu, struct hlist_node *node)
1172-
{
1173-
struct i915_pmu *pmu = hlist_entry_safe(node, typeof(*pmu), cpuhp.node);
1174-
1175-
/* Select the first online CPU as a designated reader. */
1176-
if (cpumask_empty(&i915_pmu_cpumask))
1177-
cpumask_set_cpu(cpu, &i915_pmu_cpumask);
1178-
1179-
return 0;
1180-
}
1181-
1182-
static int i915_pmu_cpu_offline(unsigned int cpu, struct hlist_node *node)
1183-
{
1184-
struct i915_pmu *pmu = hlist_entry_safe(node, typeof(*pmu), cpuhp.node);
1185-
unsigned int target = i915_pmu_target_cpu;
1186-
1187-
/*
1188-
* Unregistering an instance generates a CPU offline event which we must
1189-
* ignore to avoid incorrectly modifying the shared i915_pmu_cpumask.
1190-
*/
1191-
if (!pmu->registered)
1192-
return 0;
1193-
1194-
if (cpumask_test_and_clear_cpu(cpu, &i915_pmu_cpumask)) {
1195-
target = cpumask_any_but(topology_sibling_cpumask(cpu), cpu);
1196-
1197-
/* Migrate events if there is a valid target */
1198-
if (target < nr_cpu_ids) {
1199-
cpumask_set_cpu(target, &i915_pmu_cpumask);
1200-
i915_pmu_target_cpu = target;
1201-
}
1202-
}
1203-
1204-
if (target < nr_cpu_ids && target != pmu->cpuhp.cpu) {
1205-
perf_pmu_migrate_context(&pmu->base, cpu, target);
1206-
pmu->cpuhp.cpu = target;
1207-
}
1208-
1209-
return 0;
1210-
}
1211-
1212-
static enum cpuhp_state cpuhp_state = CPUHP_INVALID;
1213-
1214-
int i915_pmu_init(void)
1215-
{
1216-
int ret;
1217-
1218-
ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
1219-
"perf/x86/intel/i915:online",
1220-
i915_pmu_cpu_online,
1221-
i915_pmu_cpu_offline);
1222-
if (ret < 0)
1223-
pr_notice("Failed to setup cpuhp state for i915 PMU! (%d)\n",
1224-
ret);
1225-
else
1226-
cpuhp_state = ret;
1227-
1228-
return 0;
1229-
}
1230-
1231-
void i915_pmu_exit(void)
1232-
{
1233-
if (cpuhp_state != CPUHP_INVALID)
1234-
cpuhp_remove_multi_state(cpuhp_state);
1235-
}
1236-
1237-
static int i915_pmu_register_cpuhp_state(struct i915_pmu *pmu)
1238-
{
1239-
if (cpuhp_state == CPUHP_INVALID)
1240-
return -EINVAL;
1241-
1242-
return cpuhp_state_add_instance(cpuhp_state, &pmu->cpuhp.node);
1243-
}
1244-
1245-
static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
1246-
{
1247-
cpuhp_state_remove_instance(cpuhp_state, &pmu->cpuhp.node);
1248-
}
1249-
12501147
void i915_pmu_register(struct drm_i915_private *i915)
12511148
{
12521149
struct i915_pmu *pmu = &i915->pmu;
12531150
const struct attribute_group *attr_groups[] = {
12541151
&i915_pmu_format_attr_group,
12551152
&pmu->events_attr_group,
1256-
&i915_pmu_cpumask_attr_group,
12571153
NULL
12581154
};
12591155
int ret = -ENOMEM;
12601156

12611157
spin_lock_init(&pmu->lock);
12621158
hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
12631159
pmu->timer.function = i915_sample;
1264-
pmu->cpuhp.cpu = -1;
12651160
init_rc6(pmu);
12661161

12671162
if (IS_DGFX(i915)) {
@@ -1290,6 +1185,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
12901185

12911186
pmu->base.module = THIS_MODULE;
12921187
pmu->base.task_ctx_nr = perf_invalid_context;
1188+
pmu->base.scope = PERF_PMU_SCOPE_SYS_WIDE;
12931189
pmu->base.event_init = i915_pmu_event_init;
12941190
pmu->base.add = i915_pmu_event_add;
12951191
pmu->base.del = i915_pmu_event_del;
@@ -1301,16 +1197,10 @@ void i915_pmu_register(struct drm_i915_private *i915)
13011197
if (ret)
13021198
goto err_groups;
13031199

1304-
ret = i915_pmu_register_cpuhp_state(pmu);
1305-
if (ret)
1306-
goto err_unreg;
1307-
13081200
pmu->registered = true;
13091201

13101202
return;
13111203

1312-
err_unreg:
1313-
perf_pmu_unregister(&pmu->base);
13141204
err_groups:
13151205
kfree(pmu->base.attr_groups);
13161206
err_attr:
@@ -1334,8 +1224,6 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
13341224

13351225
hrtimer_cancel(&pmu->timer);
13361226

1337-
i915_pmu_unregister_cpuhp_state(pmu);
1338-
13391227
perf_pmu_unregister(&pmu->base);
13401228
kfree(pmu->base.attr_groups);
13411229
if (IS_DGFX(i915))

drivers/gpu/drm/i915/i915_pmu.h

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,6 @@ struct i915_pmu_sample {
5656
};
5757

5858
struct i915_pmu {
59-
/**
60-
* @cpuhp: Struct used for CPU hotplug handling.
61-
*/
62-
struct {
63-
struct hlist_node node;
64-
unsigned int cpu;
65-
} cpuhp;
6659
/**
6760
* @base: PMU base.
6861
*/
@@ -155,15 +148,11 @@ struct i915_pmu {
155148
};
156149

157150
#ifdef CONFIG_PERF_EVENTS
158-
int i915_pmu_init(void);
159-
void i915_pmu_exit(void);
160151
void i915_pmu_register(struct drm_i915_private *i915);
161152
void i915_pmu_unregister(struct drm_i915_private *i915);
162153
void i915_pmu_gt_parked(struct intel_gt *gt);
163154
void i915_pmu_gt_unparked(struct intel_gt *gt);
164155
#else
165-
static inline int i915_pmu_init(void) { return 0; }
166-
static inline void i915_pmu_exit(void) {}
167156
static inline void i915_pmu_register(struct drm_i915_private *i915) {}
168157
static inline void i915_pmu_unregister(struct drm_i915_private *i915) {}
169158
static inline void i915_pmu_gt_parked(struct intel_gt *gt) {}

0 commit comments

Comments
 (0)