Re: cpufreq ondemand governor debugobjects warning

[Jane Li <jiel@marvell.com>]

On 12/27/2013 04:01 PM, Viresh Kumar wrote:
> On 27 December 2013 08:36, Jane Li <jiel@marvell.com> 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".