From d0142cbf68bb4f8b844ca9a09f97e16d0b222f80 Mon Sep 17 00:00:00 2001 From: Amir Vajid Date: Tue, 9 Jun 2020 16:41:42 -0700 Subject: [PATCH] devfreq: memlat: fix potential deadlock during CPU hotplug The cpu_grp->mons_lock is acquired in the memlat hotplug notifier callbacks to prevent races within the driver. The cpus_read_lock is inherently acquired as well since it is in the middle of a hotplug event. In parallel, stop_hwmon() could unregister from the hotplug notifier (i.e. when last memlat mon in system is removed). However, unregistering the hotplug notifier also requires the cpus_read_lock and is done with the cpu_grp->mons_lock held as well which would lead to a deadlock if a hotplug is currently in progress with the memlat notifier callback waiting for the cpu_grp->mons_lock. To fix this, release the cpu_grp->mons_lock in stop_hwmon() before unregistering with the hotplug notifier. Also, make sure the memlat hotplug notifier callbacks check cpu_grp->num_active_mons before restarting events since that work won't be necessary if there are no active memlat mons in the system. Change-Id: I401639a6e28a472a64a96456d2c06f57b35dbc0d Signed-off-by: Amir Vajid --- drivers/devfreq/arm-memlat-mon.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/devfreq/arm-memlat-mon.c b/drivers/devfreq/arm-memlat-mon.c index c5ba138e9490..5fb9782addab 100644 --- a/drivers/devfreq/arm-memlat-mon.c +++ b/drivers/devfreq/arm-memlat-mon.c @@ -461,8 +461,10 @@ static int memlat_event_hotplug_coming_up(unsigned int cpu) if (!cpu_grp) return -ENODEV; mutex_lock(&cpu_grp->mons_lock); - ret = memlat_hp_restart_events(cpu, true); - per_cpu(cpu_is_hp, cpu) = false; + if (cpu_grp->num_active_mons) { + ret = memlat_hp_restart_events(cpu, true); + per_cpu(cpu_is_hp, cpu) = false; + } mutex_unlock(&cpu_grp->mons_lock); return ret; @@ -477,8 +479,10 @@ static int memlat_event_hotplug_going_down(unsigned int cpu) return -ENODEV; /* avoid race between cpu hotplug and update_counts */ mutex_lock(&cpu_grp->mons_lock); - per_cpu(cpu_is_hp, cpu) = true; - ret = memlat_hp_restart_events(cpu, false); + if (cpu_grp->num_active_mons) { + per_cpu(cpu_is_hp, cpu) = true; + ret = memlat_hp_restart_events(cpu, false); + } mutex_unlock(&cpu_grp->mons_lock); return ret; @@ -629,6 +633,7 @@ static void stop_hwmon(struct memlat_hwmon *hw) hp_idle_register_cnt--; mutex_unlock(¬ify_lock); } + mutex_unlock(&cpu_grp->mons_lock); mutex_lock(¬ify_lock); if (!hp_idle_register_cnt) { cpuhp_remove_state_nocalls(CPUHP_AP_ONLINE_DYN); @@ -639,7 +644,6 @@ static void stop_hwmon(struct memlat_hwmon *hw) } } mutex_unlock(¬ify_lock); - mutex_unlock(&cpu_grp->mons_lock); } /**