@@ -935,6 +935,7 @@ static void cpufreq_init_policy(struct cpufreq_policy *policy)
struct cpufreq_policy new_policy;
int ret = 0;
+ policy->governor_state = CPUFREQ_GOV_POLICY_EXIT;
memcpy(&new_policy, policy, sizeof(*policy));
/* Update governor of new_policy to the governor used before hotplug */
@@ -1976,7 +1977,7 @@ EXPORT_SYMBOL_GPL(cpufreq_driver_target);
static int __cpufreq_governor(struct cpufreq_policy *policy,
unsigned int event)
{
- int ret;
+ int ret, state;
/* Only must be defined when default governor is known to have latency
restrictions, like e.g. conservative or ondemand.
@@ -2012,19 +2013,21 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
policy->cpu, event);
mutex_lock(&cpufreq_governor_lock);
+ state = policy->governor_state;
+
+ /* Check if operation is permitted or not */
if (policy->governor_busy
- || (policy->governor_enabled && event == CPUFREQ_GOV_START)
- || (!policy->governor_enabled
- && (event == CPUFREQ_GOV_LIMITS || event == CPUFREQ_GOV_STOP))) {
+ || (state == CPUFREQ_GOV_START && event != CPUFREQ_GOV_LIMITS && event != CPUFREQ_GOV_STOP)
+ || (state == CPUFREQ_GOV_STOP && event != CPUFREQ_GOV_START && event != CPUFREQ_GOV_POLICY_EXIT)
+ || (state == CPUFREQ_GOV_POLICY_INIT && event != CPUFREQ_GOV_START && event != CPUFREQ_GOV_POLICY_EXIT)
+ || (state == CPUFREQ_GOV_POLICY_EXIT && event != CPUFREQ_GOV_POLICY_INIT)) {
mutex_unlock(&cpufreq_governor_lock);
return -EBUSY;
}
policy->governor_busy = true;
- if (event == CPUFREQ_GOV_STOP)
- policy->governor_enabled = false;
- else if (event == CPUFREQ_GOV_START)
- policy->governor_enabled = true;
+ if (event != CPUFREQ_GOV_LIMITS)
+ policy->governor_state = event;
mutex_unlock(&cpufreq_governor_lock);
@@ -2035,13 +2038,10 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
policy->governor->initialized++;
else if (event == CPUFREQ_GOV_POLICY_EXIT)
policy->governor->initialized--;
- } else {
+ } else if (event != CPUFREQ_GOV_LIMITS) {
/* 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;
+ policy->governor_state = state;
mutex_unlock(&cpufreq_governor_lock);
}
@@ -174,7 +174,7 @@ void gov_queue_work(struct dbs_data *dbs_data, struct cpufreq_policy *policy,
int i;
mutex_lock(&cpufreq_governor_lock);
- if (!policy->governor_enabled)
+ if (policy->governor_state != CPUFREQ_GOV_START)
goto out_unlock;
if (!all_cpus) {
@@ -81,7 +81,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 */
+ int governor_state; /* Governor's current state */
bool governor_busy;
struct work_struct update; /* if update_policy() needs to be
Even after serializing calls to __cpufreq_governor() there are some races left. The races are around doing the invalid operation during some state of cpufreq governors. For example, while the governor is in CPUFREQ_GOV_POLICY_EXIT state, we can't do CPUFREQ_GOV_START without first doing CPUFREQ_GOV_POLICY_INIT. All these cases weren't handled elegantly in __cpufreq_governor() and so there were enough chances that things may go wrong when governors are changed with multiple thread in parallel. This patch renames an existing field 'policy->governor_enabled' as 'policy->governor_state' which can have values other than 0 & 1 now. Its type is also changed to 'int' from 'bool'. We maintain the current state of governors for each policy now and reject any invalid operation. Reported-and-tested-by: Robert Schöne <robert.schoene@tu-dresden.de> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/cpufreq/cpufreq.c | 26 +++++++++++++------------- drivers/cpufreq/cpufreq_governor.c | 2 +- include/linux/cpufreq.h | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-)