Currently the work-handler's for all the CPUs are synchronized with one another but not with cpufreq_governor_dbs(). Both work-handlers and cpufreq_governor_dbs() enqueue work. And these not being in sync is an open invitation to all kind of races to come in.
So, this patch replaces 'timer_mutex' with 'cdata->mutex' so that both work-handler and callback are in sync.
Also in update_sampling_rate() we are dropping the mutex while canceling delayed-work. There is no need to do that, keep lock active for the entire length of routine as that also enqueues works.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq_governor.c | 14 +++----------- drivers/cpufreq/cpufreq_governor.h | 6 ------ drivers/cpufreq/cpufreq_ondemand.c | 13 ++++--------- 3 files changed, 7 insertions(+), 26 deletions(-)
diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 08bb67225b85..aa24aa9a9eb3 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -228,13 +228,12 @@ static void dbs_timer(struct work_struct *work) { struct cpu_dbs_info *cdbs = container_of(work, struct cpu_dbs_info, dwork.work); - struct cpu_common_dbs_info *ccdbs = cdbs->ccdbs; - struct cpufreq_policy *policy = ccdbs->policy; + struct cpufreq_policy *policy = cdbs->ccdbs->policy; struct dbs_data *dbs_data = policy->governor_data; unsigned int sampling_rate, delay; bool modify_all = true;
- mutex_lock(&ccdbs->timer_mutex); + mutex_lock(&dbs_data->cdata->mutex);
if (dbs_data->cdata->governor == GOV_CONSERVATIVE) { struct cs_dbs_tuners *cs_tuners = dbs_data->tuners; @@ -252,7 +251,7 @@ static void dbs_timer(struct work_struct *work) delay = dbs_data->cdata->gov_dbs_timer(cdbs, dbs_data, modify_all); gov_queue_work(dbs_data, policy, delay, modify_all);
- mutex_unlock(&ccdbs->timer_mutex); + mutex_unlock(&dbs_data->cdata->mutex); }
static void set_sampling_rate(struct dbs_data *dbs_data, @@ -424,7 +423,6 @@ static int cpufreq_governor_start(struct cpufreq_policy *policy, }
ccdbs->time_stamp = ktime_get(); - mutex_init(&ccdbs->timer_mutex);
for_each_cpu(j, policy->cpus) { struct cpu_dbs_info *j_cdbs = cdata->get_cpu_cdbs(j); @@ -470,8 +468,6 @@ static void cpufreq_governor_stop(struct cpufreq_policy *policy, { struct common_dbs_data *cdata = dbs_data->cdata; unsigned int cpu = policy->cpu; - struct cpu_dbs_info *cdbs = cdata->get_cpu_cdbs(cpu); - struct cpu_common_dbs_info *ccdbs = cdbs->ccdbs;
gov_cancel_work(dbs_data, policy);
@@ -481,8 +477,6 @@ static void cpufreq_governor_stop(struct cpufreq_policy *policy,
cs_dbs_info->enable = 0; } - - mutex_destroy(&ccdbs->timer_mutex); }
static void cpufreq_governor_limits(struct cpufreq_policy *policy, @@ -495,7 +489,6 @@ static void cpufreq_governor_limits(struct cpufreq_policy *policy, if (!cdbs->ccdbs) return;
- mutex_lock(&cdbs->ccdbs->timer_mutex); if (policy->max < cdbs->ccdbs->policy->cur) __cpufreq_driver_target(cdbs->ccdbs->policy, policy->max, CPUFREQ_RELATION_H); @@ -503,7 +496,6 @@ static void cpufreq_governor_limits(struct cpufreq_policy *policy, __cpufreq_driver_target(cdbs->ccdbs->policy, policy->min, CPUFREQ_RELATION_L); dbs_check_cpu(dbs_data, cpu); - mutex_unlock(&cdbs->ccdbs->timer_mutex); }
int cpufreq_governor_dbs(struct cpufreq_policy *policy, diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h index 2125c299c602..2b2884f91fe7 100644 --- a/drivers/cpufreq/cpufreq_governor.h +++ b/drivers/cpufreq/cpufreq_governor.h @@ -131,12 +131,6 @@ static void *get_cpu_dbs_info_s(int cpu) \ /* Common to all CPUs of a policy */ struct cpu_common_dbs_info { struct cpufreq_policy *policy; - /* - * percpu mutex that serializes governor limit change with dbs_timer - * invocation. We do not want dbs_timer to run when user is changing - * the governor or limits. - */ - struct mutex timer_mutex; ktime_t time_stamp; };
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c index 11db20079fc6..87813830e169 100644 --- a/drivers/cpufreq/cpufreq_ondemand.c +++ b/drivers/cpufreq/cpufreq_ondemand.c @@ -249,6 +249,8 @@ static void update_sampling_rate(struct dbs_data *dbs_data, struct od_dbs_tuners *od_tuners = dbs_data->tuners; int cpu;
+ mutex_lock(&dbs_data->cdata->mutex); + od_tuners->sampling_rate = new_rate = max(new_rate, dbs_data->min_sampling_rate);
@@ -267,28 +269,21 @@ static void update_sampling_rate(struct dbs_data *dbs_data, dbs_info = &per_cpu(od_cpu_dbs_info, cpu); cpufreq_cpu_put(policy);
- mutex_lock(&dbs_info->cdbs.ccdbs->timer_mutex); - - if (!delayed_work_pending(&dbs_info->cdbs.dwork)) { - mutex_unlock(&dbs_info->cdbs.ccdbs->timer_mutex); + if (!delayed_work_pending(&dbs_info->cdbs.dwork)) continue; - }
next_sampling = jiffies + usecs_to_jiffies(new_rate); appointed_at = dbs_info->cdbs.dwork.timer.expires;
if (time_before(next_sampling, appointed_at)) {
- mutex_unlock(&dbs_info->cdbs.ccdbs->timer_mutex); cancel_delayed_work_sync(&dbs_info->cdbs.dwork); - mutex_lock(&dbs_info->cdbs.ccdbs->timer_mutex); - gov_queue_work(dbs_data, policy, usecs_to_jiffies(new_rate), true);
} - mutex_unlock(&dbs_info->cdbs.ccdbs->timer_mutex); } + mutex_unlock(&dbs_data->cdata->mutex); }
static ssize_t store_sampling_rate(struct dbs_data *dbs_data, const char *buf,