cpufreq: serialize calls to __cpufreq_governor()
We can't take a big lock around __cpufreq_governor() as this causes recursive locking for some cases. But calls to this routine must be serialized for every policy. Otherwise we can see some unpredictable events. For example, consider following scenario: __cpufreq_remove_dev() __cpufreq_governor(policy, CPUFREQ_GOV_STOP); policy->governor->governor(policy, CPUFREQ_GOV_STOP); cpufreq_governor_dbs() case CPUFREQ_GOV_STOP: mutex_destroy(&cpu_cdbs->timer_mutex) cpu_cdbs->cur_policy = NULL; <PREEMPT> store() __cpufreq_set_policy() __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS); policy->governor->governor(policy, CPUFREQ_GOV_LIMITS); case CPUFREQ_GOV_LIMITS: mutex_lock(&cpu_cdbs->timer_mutex); <-- Warning (destroyed mutex) if (policy->max < cpu_cdbs->cur_policy->cur) <- cur_policy == NULL And so store() will eventually result in a crash if cur_policy is NULL at this point. Introduce an additional variable which would guarantee serialization here. Reported-by: Stephen Boyd <sboyd@codeaurora.org> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
This commit is contained in:
parent
f73d393384
commit
19c763031a
2 changed files with 7 additions and 1 deletions
|
@ -1692,13 +1692,15 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
|
||||||
policy->cpu, event);
|
policy->cpu, event);
|
||||||
|
|
||||||
mutex_lock(&cpufreq_governor_lock);
|
mutex_lock(&cpufreq_governor_lock);
|
||||||
if ((policy->governor_enabled && event == CPUFREQ_GOV_START)
|
if (policy->governor_busy
|
||||||
|
|| (policy->governor_enabled && event == CPUFREQ_GOV_START)
|
||||||
|| (!policy->governor_enabled
|
|| (!policy->governor_enabled
|
||||||
&& (event == CPUFREQ_GOV_LIMITS || event == CPUFREQ_GOV_STOP))) {
|
&& (event == CPUFREQ_GOV_LIMITS || event == CPUFREQ_GOV_STOP))) {
|
||||||
mutex_unlock(&cpufreq_governor_lock);
|
mutex_unlock(&cpufreq_governor_lock);
|
||||||
return -EBUSY;
|
return -EBUSY;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
policy->governor_busy = true;
|
||||||
if (event == CPUFREQ_GOV_STOP)
|
if (event == CPUFREQ_GOV_STOP)
|
||||||
policy->governor_enabled = false;
|
policy->governor_enabled = false;
|
||||||
else if (event == CPUFREQ_GOV_START)
|
else if (event == CPUFREQ_GOV_START)
|
||||||
|
@ -1727,6 +1729,9 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
|
||||||
((event == CPUFREQ_GOV_POLICY_EXIT) && !ret))
|
((event == CPUFREQ_GOV_POLICY_EXIT) && !ret))
|
||||||
module_put(policy->governor->owner);
|
module_put(policy->governor->owner);
|
||||||
|
|
||||||
|
mutex_lock(&cpufreq_governor_lock);
|
||||||
|
policy->governor_busy = false;
|
||||||
|
mutex_unlock(&cpufreq_governor_lock);
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -76,6 +76,7 @@ struct cpufreq_policy {
|
||||||
struct cpufreq_governor *governor; /* see below */
|
struct cpufreq_governor *governor; /* see below */
|
||||||
void *governor_data;
|
void *governor_data;
|
||||||
bool governor_enabled; /* governor start/stop flag */
|
bool governor_enabled; /* governor start/stop flag */
|
||||||
|
bool governor_busy;
|
||||||
|
|
||||||
struct work_struct update; /* if update_policy() needs to be
|
struct work_struct update; /* if update_policy() needs to be
|
||||||
* called, but you're in IRQ context */
|
* called, but you're in IRQ context */
|
||||||
|
|
Loading…
Add table
Reference in a new issue