From f0da7256374da9ab055d1d85cdf0adcb47f9190e Mon Sep 17 00:00:00 2001 From: Junjie Wu Date: Fri, 20 Mar 2015 14:41:50 -0700 Subject: PM / devfreq: governor_cpufreq: Rewrite locking to avoid deadlocks A devfreq governor store in parallel with a cpu freq update can cause deadlock as shown below. Assume current devfreq governor is cpufreq, and user tries to change to some other governor. Write to sysfs store_governor | cpufreq driver updating cpu freq ------------------------------- | ----------------------------------- echo bw_hwmon > governor | | takes rcu_read_lock and calls all | cpufreq transition callbacks for | PRECHANGE or POSTCHANGE | GOV_STOP on governor_cpufreq. | unregister_cpufreq() accquires | state_lock mutex. | | try to accquire same state_lock in | cpufreq_trans_notifier(). Blocked. unregister from cpufreq | transition notifier and wait for| all rcu_readers to finish. | Deadlock A similar deadlock can happen with governor change and policy notifier callbacks. The state_lock currently protects multiple unrelated critical sections: registering/unregistering of cpufreq notifiers, read/writing the device list, and tracking the cpu states and updating device frequencies. There is no need for register/unregister of the cpufreq notifiers to be mutually excluded against the other critical sections using the same lock. Split state_lock into two locks to protect the register/unregister of cpufreq notifiers from the rest of the critical sections. Change-Id: Id06d326748a5cb0c84c4787da5d0910f44eb5c3c Signed-off-by: Pan Fang Signed-off-by: Arun KS Signed-off-by: Junjie Wu Suggested-by: Saravana Kannan --- drivers/devfreq/governor_cpufreq.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/devfreq/governor_cpufreq.c b/drivers/devfreq/governor_cpufreq.c index 01a6003cc391..fc4da03201f2 100644 --- a/drivers/devfreq/governor_cpufreq.c +++ b/drivers/devfreq/governor_cpufreq.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, The Linux Foundation. All rights reserved. + * Copyright (c) 2014-2015, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -53,6 +53,7 @@ struct devfreq_node { }; static LIST_HEAD(devfreq_list); static DEFINE_MUTEX(state_lock); +static DEFINE_MUTEX(cpufreq_reg_lock); #define show_attr(name) \ static ssize_t show_##name(struct device *dev, \ @@ -240,7 +241,7 @@ static int register_cpufreq(void) unsigned int cpu; struct cpufreq_policy *policy; - mutex_lock(&state_lock); + mutex_lock(&cpufreq_reg_lock); if (cpufreq_cnt) goto cnt_not_zero; @@ -271,7 +272,7 @@ out: cnt_not_zero: if (!ret) cpufreq_cnt++; - mutex_unlock(&state_lock); + mutex_unlock(&cpufreq_reg_lock); return ret; } @@ -280,7 +281,7 @@ static int unregister_cpufreq(void) int ret = 0; int cpu; - mutex_lock(&state_lock); + mutex_lock(&cpufreq_reg_lock); if (cpufreq_cnt > 1) goto out; @@ -300,7 +301,7 @@ static int unregister_cpufreq(void) out: cpufreq_cnt--; - mutex_unlock(&state_lock); + mutex_unlock(&cpufreq_reg_lock); return ret; } -- cgit v1.2.3