Skip to content

Commit 0d2e2a8

Browse files
rmurphy-armwildea01
authored andcommitted
perf/arm-cci: Remove broken race mitigation
Uncore PMU drivers face an awkward cyclic dependency wherein: - They have to pick a valid online CPU to associate with before registering the PMU device, since it will get exposed to userspace immediately. - The PMU registration has to be be at least partly complete before hotplug events can be handled, since trying to migrate an uninitialised context would be bad. - The hotplug handler has to be ready as soon as a CPU is chosen, lest it go offline without the user-visible cpumask value getting updated. The arm-cci driver has tried to solve this by using get_cpu() to pick the current CPU and prevent it from disappearing while both registrations are performed, but that results in taking mutexes with preemption disabled, which makes certain configurations very unhappy: [ 1.983337] BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:2004 [ 1.983340] in_atomic(): 1, irqs_disabled(): 0, pid: 1, name: swapper/0 [ 1.983342] Preemption disabled at: [ 1.983353] [<ffffff80089801f4>] cci_pmu_probe+0x1dc/0x488 [ 1.983360] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.18.20-rt8-yocto-preempt-rt #1 [ 1.983362] Hardware name: ZynqMP ZCU102 Rev1.0 (DT) [ 1.983364] Call trace: [ 1.983369] dump_backtrace+0x0/0x158 [ 1.983372] show_stack+0x24/0x30 [ 1.983378] dump_stack+0x80/0xa4 [ 1.983383] ___might_sleep+0x138/0x160 [ 1.983386] __might_sleep+0x58/0x90 [ 1.983391] __rt_mutex_lock_state+0x30/0xc0 [ 1.983395] _mutex_lock+0x24/0x30 [ 1.983400] perf_pmu_register+0x2c/0x388 [ 1.983404] cci_pmu_probe+0x2bc/0x488 [ 1.983409] platform_drv_probe+0x58/0xa8 It is not feasible to resolve all the possible races outside of the perf core itself, so address the immediate bug by following the example of nearly every other PMU driver and not even trying to do so. Registering the hotplug notifier first should minimise the window in which things can go wrong, so that's about as much as we can reasonably do here. This also revealed an additional race in assigning the global pointer too late relative to the hotplug notifier, which gets fixed in the process. Reported-by: Li, Meng <[email protected]> Tested-by: Corentin Labbe <[email protected]> Reviewed-by: Suzuki K Poulose <[email protected]> Signed-off-by: Robin Murphy <[email protected]> Signed-off-by: Will Deacon <[email protected]>
1 parent 3d659e7 commit 0d2e2a8

File tree

1 file changed

+12
-9
lines changed

1 file changed

+12
-9
lines changed

drivers/perf/arm-cci.c

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1684,21 +1684,24 @@ static int cci_pmu_probe(struct platform_device *pdev)
16841684
raw_spin_lock_init(&cci_pmu->hw_events.pmu_lock);
16851685
mutex_init(&cci_pmu->reserve_mutex);
16861686
atomic_set(&cci_pmu->active_events, 0);
1687-
cci_pmu->cpu = get_cpu();
1688-
1689-
ret = cci_pmu_init(cci_pmu, pdev);
1690-
if (ret) {
1691-
put_cpu();
1692-
return ret;
1693-
}
16941687

1688+
cci_pmu->cpu = raw_smp_processor_id();
1689+
g_cci_pmu = cci_pmu;
16951690
cpuhp_setup_state_nocalls(CPUHP_AP_PERF_ARM_CCI_ONLINE,
16961691
"perf/arm/cci:online", NULL,
16971692
cci_pmu_offline_cpu);
1698-
put_cpu();
1699-
g_cci_pmu = cci_pmu;
1693+
1694+
ret = cci_pmu_init(cci_pmu, pdev);
1695+
if (ret)
1696+
goto error_pmu_init;
1697+
17001698
pr_info("ARM %s PMU driver probed", cci_pmu->model->name);
17011699
return 0;
1700+
1701+
error_pmu_init:
1702+
cpuhp_remove_state(CPUHP_AP_PERF_ARM_CCI_ONLINE);
1703+
g_cci_pmu = NULL;
1704+
return ret;
17021705
}
17031706

17041707
static int cci_pmu_remove(struct platform_device *pdev)

0 commit comments

Comments
 (0)