diff options
authorXiaoguang Chen <>2013-06-19 15:00:07 +0800
committerMarek Szyprowski <>2014-04-01 13:07:33 +0200
commit427ea63dd1d92f937aab1d2fad4f4c6d0d02630b (patch)
parent4bd38a0bea6359db34876a7c2db0454717b4f0bb (diff)
cpufreq: Fix governor start/stop race condition
Cpufreq governors' stop and start operations should be carried out in sequence. Otherwise, there will be unexpected behavior, like in the example below. Suppose there are 4 CPUs and policy->cpu=CPU0, CPU1/2/3 are linked to CPU0. The normal sequence is: 1) Current governor is userspace. An application tries to set the governor to ondemand. It will call __cpufreq_set_policy() in which it will stop the userspace governor and then start the ondemand governor. 2) Current governor is userspace. The online of CPU3 runs on CPU0. It will call cpufreq_add_policy_cpu() in which it will first stop the userspace governor, and then start it again. If the sequence of the above two cases interleaves, it becomes: 1) Application stops userspace governor 2) Hotplug stops userspace governor which is a problem, because the governor shouldn't be stopped twice in a row. What happens next is: 3) Application starts ondemand governor 4) Hotplug starts a governor In step 4, the hotplug is supposed to start the userspace governor, but now the governor has been changed by the application to ondemand, so the ondemand governor is started once again, which is incorrect. The solution is to prevent policy governors from being stopped multiple times in a row. A governor should only be stopped once for one policy. After it has been stopped, no more governor stop operations should be executed. Also add a mutex to serialize governor operations. Change-Id: Ie380dc7c551f2721b81ceb8e4849efa09345ce4b [rjw: Changelog. And you owe me a beverage of my choice.] Signed-off-by: Xiaoguang Chen <> Acked-by: Viresh Kumar <> Signed-off-by: Rafael J. Wysocki <>
2 files changed, 25 insertions, 0 deletions
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 5dcb6ea316f..ca486c9a99b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -47,6 +47,7 @@ static DEFINE_PER_CPU(char[CPUFREQ_NAME_LEN], cpufreq_cpu_governor);
static DEFINE_RWLOCK(cpufreq_driver_lock);
static LIST_HEAD(cpufreq_policy_list);
+static DEFINE_MUTEX(cpufreq_governor_lock);
* cpu_policy_rwsem is a per CPU reader-writer semaphore designed to cure
@@ -1632,6 +1633,21 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
pr_debug("__cpufreq_governor for CPU %u, event %u\n",
policy->cpu, event);
+ mutex_lock(&cpufreq_governor_lock);
+ if ((!policy->governor_enabled && (event == CPUFREQ_GOV_STOP)) ||
+ (policy->governor_enabled && (event == CPUFREQ_GOV_START))) {
+ mutex_unlock(&cpufreq_governor_lock);
+ return -EBUSY;
+ }
+ if (event == CPUFREQ_GOV_STOP)
+ policy->governor_enabled = false;
+ else if (event == CPUFREQ_GOV_START)
+ policy->governor_enabled = true;
+ mutex_unlock(&cpufreq_governor_lock);
ret = policy->governor->governor(policy, event);
if (!ret) {
@@ -1639,6 +1655,14 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
else if (event == CPUFREQ_GOV_POLICY_EXIT)
+ } else {
+ /* Restore original values */
+ mutex_lock(&cpufreq_governor_lock);
+ if (event == CPUFREQ_GOV_STOP)
+ policy->governor_enabled = true;
+ else if (event == CPUFREQ_GOV_START)
+ policy->governor_enabled = false;
+ mutex_unlock(&cpufreq_governor_lock);
/* we keep one module reference alive for
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 6b076e37a99..9791c329c3e 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -111,6 +111,7 @@ struct cpufreq_policy {
unsigned int policy; /* see above */
struct cpufreq_governor *governor; /* see below */
void *governor_data;
+ bool governor_enabled; /* governor start/stop flag */
struct work_struct update; /* if update_policy() needs to be
* called, but you're in IRQ context */