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 <fangpan@codeaurora.org>
Signed-off-by: Arun KS <arunks@codeaurora.org>
Signed-off-by: Junjie Wu <junjiew@codeaurora.org>
Suggested-by: Saravana Kannan <skannan@codeaurora.org>
diff --git a/drivers/devfreq/governor_cpufreq.c b/drivers/devfreq/governor_cpufreq.c
index bae1d39..03ec792 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 @@
};
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 @@
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 @@
cnt_not_zero:
if (!ret)
cpufreq_cnt++;
- mutex_unlock(&state_lock);
+ mutex_unlock(&cpufreq_reg_lock);
return ret;
}
@@ -280,7 +281,7 @@
int ret = 0;
int cpu;
- mutex_lock(&state_lock);
+ mutex_lock(&cpufreq_reg_lock);
if (cpufreq_cnt > 1)
goto out;
@@ -300,7 +301,7 @@
out:
cpufreq_cnt--;
- mutex_unlock(&state_lock);
+ mutex_unlock(&cpufreq_reg_lock);
return ret;
}