On Fri, 26 Jul 2013 14:36:08 +0200 Rafael J. Wysocki rjw@sisk.pl wrote,
On Friday, July 26, 2013 10:33:21 AM Lukasz Majewski wrote:
On Fri, 26 Jul 2013 12:47:15 +0530 Viresh Kumar wrote,
On 25 July 2013 22:03, Lukasz Majewski l.majewski@samsung.com wrote:
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
/*********************************************************************
BOOST *
*********************************************************************/ +static int cpufreq_boost_set_sw(int state) +{
struct cpufreq_frequency_table *freq_table;struct cpufreq_policy *policy;int ret = -EINVAL;list_for_each_entry(policy, &cpufreq_policy_list,policy_list) {
freq_table =cpufreq_frequency_get_table(policy->cpu);
if (freq_table) {ret =cpufreq_frequency_table_cpuinfo(policy,
freq_table);
if (!ret) {policy->user_policy.max =policy->max;
__cpufreq_governor(policy,CPUFREQ_GOV_LIMITS);
}}}return ret;+}
+int cpufreq_boost_trigger_state(int state) +{
unsigned long flags;int ret = 0;if (cpufreq_driver->boost_enabled == state)return 0;write_lock_irqsave(&cpufreq_driver_lock, flags);cpufreq_driver->boost_enabled = state;write_unlock_irqrestore(&cpufreq_driver_lock, flags);^^^^^^^^^^^^^^^^^^^^ [*]Not sure if we should leave the lock at this point of time, as we haven't enabled boost until now.
The problem here is with the cpufreq_driver->set_boost() call.
I tried to avoid acquiring lock at one function and release it at another (in this case cpufreq_boost_set_sw), especially since the __cpufreq_governor() acquires its own lock - good place for deadlock.
Is it OK for you to grab lock at one function (cpufreq_boost_trigger_state()) and then at other function (cpufreq_boost_set_sw) release it before calling __cpufreq_governor() and grab it again after its completion?
It generally is better to avoid doing that, although it is not unheard of.
In this particular case, one also needs to pass the "flags" parameter to the set_boost() function. This looks a bit unnatural to mix lock layer with the boost.
Thanks, Rafael