On 12/27/2013 04:01 PM, Viresh Kumar wrote:
On 27 December 2013 08:36, Jane Li <jiel@xxxxxxxxxxx> wrote:
When gov_queue_work(), governor_enabled may be modified. Following patch
can fix it by adding cpufreq_governor_lock in gov_queue_work. But in this
way, cpufreq_governor_lock also protects __gov_queue_work(). Do you think
this is a good idea?
I don't see a alternative than this solution after the deadlock issue with timer
lock.
diff --git a/drivers/cpufreq/cpufreq_governor.c
b/drivers/cpufreq/cpufreq_governor.c
index e6be635..a27246c 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -118,9 +118,11 @@ void gov_queue_work(struct dbs_data *dbs_data,
struct cpufreq_policy *policy,
unsigned int delay, bool all_cpus)
{
int i;
-
don't remove this.
- if (!policy->governor_enabled)
+ mutex_lock(&cpufreq_governor_lock);
+ if (!policy->governor_enabled) {
+ mutex_unlock(&cpufreq_governor_lock);
return;
+ }
if (!all_cpus) {
/*
@@ -135,6 +137,7 @@ void gov_queue_work(struct dbs_data *dbs_data,
struct cpufreq_policy *policy,
for_each_cpu(i, policy->cpus)
__gov_queue_work(i, dbs_data, delay);
}
+ mutex_unlock(&cpufreq_governor_lock);
}
EXPORT_SYMBOL_GPL(gov_queue_work);
You also need to remove 'static' from lock's declaration.
--
viresh
Thanks. I have pushed one patch named "[PATCH] cpufreq: Fix timer/workqueue corruption by protecting reading governor_enabled".
--
To unsubscribe from this list: send the line "unsubscribe cpufreq" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
