[PATCH] perf/arm-dmc620: Reverse locking order in dmc620_pmu_get_irq()

From: Waiman Long
Date: Wed Apr 05 2023 - 13:30:18 EST


The following circular locking dependency was reported when running
cpus online/offline test on an arm64 system.

[ 84.195923] Chain exists of:
dmc620_pmu_irqs_lock --> cpu_hotplug_lock --> cpuhp_state-down

[ 84.207305] Possible unsafe locking scenario:

[ 84.213212] CPU0 CPU1
[ 84.217729] ---- ----
[ 84.222247] lock(cpuhp_state-down);
[ 84.225899] lock(cpu_hotplug_lock);
[ 84.232068] lock(cpuhp_state-down);
[ 84.238237] lock(dmc620_pmu_irqs_lock);
[ 84.242236]
*** DEADLOCK ***

The problematic locking order seems to be

lock(dmc620_pmu_irqs_lock) --> lock(cpu_hotplug_lock)

This locking order happens when dmc620_pmu_get_irq() is called from
dmc620_pmu_device_probe(). Fix this possible deadlock scenario by
reversing the locking order.

Also export __cpuhp_state_add_instance_cpuslocked() so that it can be
accessed by kernel modules.

Signed-off-by: Waiman Long <longman@xxxxxxxxxx>
---
drivers/perf/arm_dmc620_pmu.c | 4 +++-
kernel/cpu.c | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/arm_dmc620_pmu.c b/drivers/perf/arm_dmc620_pmu.c
index 54aa4658fb36..78d3bfbe96a6 100644
--- a/drivers/perf/arm_dmc620_pmu.c
+++ b/drivers/perf/arm_dmc620_pmu.c
@@ -425,7 +425,7 @@ static struct dmc620_pmu_irq *__dmc620_pmu_get_irq(int irq_num)
if (ret)
goto out_free_irq;

- ret = cpuhp_state_add_instance_nocalls(cpuhp_state_num, &irq->node);
+ ret = cpuhp_state_add_instance_nocalls_cpuslocked(cpuhp_state_num, &irq->node);
if (ret)
goto out_free_irq;

@@ -445,9 +445,11 @@ static int dmc620_pmu_get_irq(struct dmc620_pmu *dmc620_pmu, int irq_num)
{
struct dmc620_pmu_irq *irq;

+ cpus_read_lock();
mutex_lock(&dmc620_pmu_irqs_lock);
irq = __dmc620_pmu_get_irq(irq_num);
mutex_unlock(&dmc620_pmu_irqs_lock);
+ cpus_read_unlock();

if (IS_ERR(irq))
return PTR_ERR(irq);
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 6c0a92ca6bb5..05daaef362e6 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -2036,6 +2036,7 @@ int __cpuhp_state_add_instance_cpuslocked(enum cpuhp_state state,
mutex_unlock(&cpuhp_state_mutex);
return ret;
}
+EXPORT_SYMBOL_GPL(__cpuhp_state_add_instance_cpuslocked);

int __cpuhp_state_add_instance(enum cpuhp_state state, struct hlist_node *node,
bool invoke)
--
2.31.1