diff options
| author | Junjie Wu <junjiew@codeaurora.org> | 2015-06-10 17:57:07 -0700 |
|---|---|---|
| committer | Hanumath Prasad <hpprasad@codeaurora.org> | 2015-06-23 14:02:51 +0530 |
| commit | 3e04aa7b32dae8abdeb27b605c046ca91b51ca27 (patch) | |
| tree | 1252b004140cf81b45e83c5d6c660a1b24000ada | |
| parent | 5129f649348db303db6ed0ea890cf397f2f271e7 (diff) | |
| download | qcom-3e04aa7b32dae8abdeb27b605c046ca91b51ca27.tar.gz | |
cpufreq: Check current frequency in device driver
__cpufreq_driver_target() checks if policy->cur is same as target_freq
without holding any lock. This function is used by governor to
directly set CPU frequency. Governor calling this function can't hold
any CPUfreq framework locks due to deadlock possibility.
However, this results in a race condition where one thread could see
a stale policy->cur while another thread is changing CPU frequency.
Thread A: Governor calls __cpufreq_driver_target(), starts increasing
frequency but hasn't sent out CPUFREQ_POSTCHANGE notification yet.
Thread B: Some other driver (could be thermal mitigation) starts
limiting frequency using cpufreq_update_policy(). Every limits are
applied to policy->min/max and final policy->max happens to be same as
policy->cur. __cpufreq_driver_target() simply returns 0.
Thread A: Governor finish scaling and now policy->cur violates
policy->max and could last forever until next CPU frequency scaling
happens.
Shifting the responsibility of checking policy->cur and target_freq
to CPUfreq device driver would resolve the race as long as the device
driver holds a common mutex.
Change-Id: I6f943228e793a4a4300c58b3ae0143e09ed01d7d
Signed-off-by: Junjie Wu <junjiew@codeaurora.org>
Signed-off-by: Hanumath Prasad <hpprasad@codeaurora.org>
| -rw-r--r-- | drivers/cpufreq/cpufreq.c | 9 | ||||
| -rw-r--r-- | drivers/cpufreq/qcom-cpufreq.c | 5 |
2 files changed, 4 insertions, 10 deletions
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 6592c89af45..aed64adc356 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1734,15 +1734,6 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy, pr_debug("target for CPU %u: %u kHz, relation %u, requested %u kHz\n", policy->cpu, target_freq, relation, old_target_freq); - /* - * This might look like a redundant call as we are checking it again - * after finding index. But it is left intentionally for cases where - * exactly same freq is called again and so we can save on few function - * calls. - */ - if (target_freq == policy->cur) - return 0; - if (cpufreq_driver->target) retval = cpufreq_driver->target(policy, target_freq, relation); else if (cpufreq_driver->target_index) { diff --git a/drivers/cpufreq/qcom-cpufreq.c b/drivers/cpufreq/qcom-cpufreq.c index 6f12bd3e1d2..e30b0cb7483 100644 --- a/drivers/cpufreq/qcom-cpufreq.c +++ b/drivers/cpufreq/qcom-cpufreq.c @@ -73,12 +73,15 @@ static int msm_cpufreq_target(struct cpufreq_policy *policy, unsigned int target_freq, unsigned int relation) { - int ret = -EFAULT; + int ret = 0; int index; struct cpufreq_frequency_table *table; mutex_lock(&per_cpu(cpufreq_suspend, policy->cpu).suspend_mutex); + if (target_freq == policy->cur) + goto done; + if (per_cpu(cpufreq_suspend, policy->cpu).device_suspended) { pr_debug("cpufreq: cpu%d scheduling frequency change " "in suspend.\n", policy->cpu); |