Commit 1112e9d8 authored by Rafael J. Wysocki's avatar Rafael J. Wysocki

cpufreq: governor: Narrow down the dbs_data_mutex coverage

Since cpufreq_governor_dbs() is now always called with policy->rwsem
held, it cannot be executed twice in parallel for the same policy.
Thus it is not necessary to hold dbs_data_mutex around the invocations
of cpufreq_governor_start/stop/limits() from it as those functions
never modify any data that can be shared between different policies.

However, cpufreq_governor_dbs() may be executed twice in parallal
for different policies using the same gov->gdbs_data object and
dbs_data_mutex is still necessary to protect that object against
concurrent updates.

For this reason, narrow down the dbs_data_mutex locking to
cpufreq_governor_init/exit() where it is needed and rename the
mutex to gov_dbs_data_mutex to reflect its purpose.
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: default avatarViresh Kumar <viresh.kumar@linaro.org>
parent e3f5ed93
...@@ -24,7 +24,7 @@ ...@@ -24,7 +24,7 @@
static DEFINE_PER_CPU(struct cpu_dbs_info, cpu_dbs); static DEFINE_PER_CPU(struct cpu_dbs_info, cpu_dbs);
static DEFINE_MUTEX(dbs_data_mutex); static DEFINE_MUTEX(gov_dbs_data_mutex);
/* Common sysfs tunables */ /* Common sysfs tunables */
/** /**
...@@ -421,10 +421,10 @@ static void free_policy_dbs_info(struct policy_dbs_info *policy_dbs, ...@@ -421,10 +421,10 @@ static void free_policy_dbs_info(struct policy_dbs_info *policy_dbs,
static int cpufreq_governor_init(struct cpufreq_policy *policy) static int cpufreq_governor_init(struct cpufreq_policy *policy)
{ {
struct dbs_governor *gov = dbs_governor_of(policy); struct dbs_governor *gov = dbs_governor_of(policy);
struct dbs_data *dbs_data = gov->gdbs_data; struct dbs_data *dbs_data;
struct policy_dbs_info *policy_dbs; struct policy_dbs_info *policy_dbs;
unsigned int latency; unsigned int latency;
int ret; int ret = 0;
/* State should be equivalent to EXIT */ /* State should be equivalent to EXIT */
if (policy->governor_data) if (policy->governor_data)
...@@ -434,6 +434,10 @@ static int cpufreq_governor_init(struct cpufreq_policy *policy) ...@@ -434,6 +434,10 @@ static int cpufreq_governor_init(struct cpufreq_policy *policy)
if (!policy_dbs) if (!policy_dbs)
return -ENOMEM; return -ENOMEM;
/* Protect gov->gdbs_data against concurrent updates. */
mutex_lock(&gov_dbs_data_mutex);
dbs_data = gov->gdbs_data;
if (dbs_data) { if (dbs_data) {
if (WARN_ON(have_governor_per_policy())) { if (WARN_ON(have_governor_per_policy())) {
ret = -EINVAL; ret = -EINVAL;
...@@ -446,8 +450,7 @@ static int cpufreq_governor_init(struct cpufreq_policy *policy) ...@@ -446,8 +450,7 @@ static int cpufreq_governor_init(struct cpufreq_policy *policy)
dbs_data->usage_count++; dbs_data->usage_count++;
list_add(&policy_dbs->list, &dbs_data->policy_dbs_list); list_add(&policy_dbs->list, &dbs_data->policy_dbs_list);
mutex_unlock(&dbs_data->mutex); mutex_unlock(&dbs_data->mutex);
goto out;
return 0;
} }
dbs_data = kzalloc(sizeof(*dbs_data), GFP_KERNEL); dbs_data = kzalloc(sizeof(*dbs_data), GFP_KERNEL);
...@@ -488,7 +491,7 @@ static int cpufreq_governor_init(struct cpufreq_policy *policy) ...@@ -488,7 +491,7 @@ static int cpufreq_governor_init(struct cpufreq_policy *policy)
get_governor_parent_kobj(policy), get_governor_parent_kobj(policy),
"%s", gov->gov.name); "%s", gov->gov.name);
if (!ret) if (!ret)
return 0; goto out;
/* Failure, so roll back. */ /* Failure, so roll back. */
pr_err("cpufreq: Governor initialization failed (dbs_data kobject init error %d)\n", ret); pr_err("cpufreq: Governor initialization failed (dbs_data kobject init error %d)\n", ret);
...@@ -502,6 +505,9 @@ static int cpufreq_governor_init(struct cpufreq_policy *policy) ...@@ -502,6 +505,9 @@ static int cpufreq_governor_init(struct cpufreq_policy *policy)
free_policy_dbs_info: free_policy_dbs_info:
free_policy_dbs_info(policy_dbs, gov); free_policy_dbs_info(policy_dbs, gov);
out:
mutex_unlock(&gov_dbs_data_mutex);
return ret; return ret;
} }
...@@ -512,6 +518,9 @@ static int cpufreq_governor_exit(struct cpufreq_policy *policy) ...@@ -512,6 +518,9 @@ static int cpufreq_governor_exit(struct cpufreq_policy *policy)
struct dbs_data *dbs_data = policy_dbs->dbs_data; struct dbs_data *dbs_data = policy_dbs->dbs_data;
int count; int count;
/* Protect gov->gdbs_data against concurrent updates. */
mutex_lock(&gov_dbs_data_mutex);
mutex_lock(&dbs_data->mutex); mutex_lock(&dbs_data->mutex);
list_del(&policy_dbs->list); list_del(&policy_dbs->list);
count = --dbs_data->usage_count; count = --dbs_data->usage_count;
...@@ -533,6 +542,8 @@ static int cpufreq_governor_exit(struct cpufreq_policy *policy) ...@@ -533,6 +542,8 @@ static int cpufreq_governor_exit(struct cpufreq_policy *policy)
} }
free_policy_dbs_info(policy_dbs, gov); free_policy_dbs_info(policy_dbs, gov);
mutex_unlock(&gov_dbs_data_mutex);
return 0; return 0;
} }
...@@ -599,31 +610,20 @@ static int cpufreq_governor_limits(struct cpufreq_policy *policy) ...@@ -599,31 +610,20 @@ static int cpufreq_governor_limits(struct cpufreq_policy *policy)
int cpufreq_governor_dbs(struct cpufreq_policy *policy, unsigned int event) int cpufreq_governor_dbs(struct cpufreq_policy *policy, unsigned int event)
{ {
int ret = -EINVAL;
/* Lock governor to block concurrent initialization of governor */
mutex_lock(&dbs_data_mutex);
if (event == CPUFREQ_GOV_POLICY_INIT) { if (event == CPUFREQ_GOV_POLICY_INIT) {
ret = cpufreq_governor_init(policy); return cpufreq_governor_init(policy);
} else if (policy->governor_data) { } else if (policy->governor_data) {
switch (event) { switch (event) {
case CPUFREQ_GOV_POLICY_EXIT: case CPUFREQ_GOV_POLICY_EXIT:
ret = cpufreq_governor_exit(policy); return cpufreq_governor_exit(policy);
break;
case CPUFREQ_GOV_START: case CPUFREQ_GOV_START:
ret = cpufreq_governor_start(policy); return cpufreq_governor_start(policy);
break;
case CPUFREQ_GOV_STOP: case CPUFREQ_GOV_STOP:
ret = cpufreq_governor_stop(policy); return cpufreq_governor_stop(policy);
break;
case CPUFREQ_GOV_LIMITS: case CPUFREQ_GOV_LIMITS:
ret = cpufreq_governor_limits(policy); return cpufreq_governor_limits(policy);
break;
} }
} }
return -EINVAL;
mutex_unlock(&dbs_data_mutex);
return ret;
} }
EXPORT_SYMBOL_GPL(cpufreq_governor_dbs); EXPORT_SYMBOL_GPL(cpufreq_governor_dbs);
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment