diff options
author | Viresh Kumar <viresh.kumar@linaro.org> | 2013-09-17 00:52:11 -0400 |
---|---|---|
committer | Rafael J. Wysocki <rafael.j.wysocki@intel.com> | 2013-09-17 18:01:52 -0400 |
commit | 8efd57657d8ef666810b55e609da72de92314dc4 (patch) | |
tree | 160796a62e6c96212665afcd028daa8cb7aa899a /drivers/cpufreq/cpufreq.c | |
parent | 9c8f1ee40b6368e6b2775c9c9f816e2a5dca3c07 (diff) |
cpufreq: unlock correct rwsem while updating policy->cpu
Current code looks like this:
WARN_ON(lock_policy_rwsem_write(cpu));
update_policy_cpu(policy, new_cpu);
unlock_policy_rwsem_write(cpu);
{lock|unlock}_policy_rwsem_write(cpu) takes/releases policy->cpu's rwsem.
Because cpu is changing with the call to update_policy_cpu(), the
unlock_policy_rwsem_write() will release the incorrect lock.
The right solution would be to release the same lock as was taken earlier. Also
update_policy_cpu() was also called from cpufreq_add_dev() without any locks and
so its better if we move this locking to inside update_policy_cpu().
This patch fixes a regression introduced in 3.12 by commit f9ba680d23
(cpufreq: Extract the handover of policy cpu to a helper function).
Reported-and-tested-by: Jon Medhurst<tixy@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Diffstat (limited to 'drivers/cpufreq/cpufreq.c')
-rw-r--r-- | drivers/cpufreq/cpufreq.c | 13 |
1 files changed, 11 insertions, 2 deletions
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index dbfe219667d3..82ecbe39dfb0 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c | |||
@@ -952,9 +952,20 @@ static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu) | |||
952 | if (cpu == policy->cpu) | 952 | if (cpu == policy->cpu) |
953 | return; | 953 | return; |
954 | 954 | ||
955 | /* | ||
956 | * Take direct locks as lock_policy_rwsem_write wouldn't work here. | ||
957 | * Also lock for last cpu is enough here as contention will happen only | ||
958 | * after policy->cpu is changed and after it is changed, other threads | ||
959 | * will try to acquire lock for new cpu. And policy is already updated | ||
960 | * by then. | ||
961 | */ | ||
962 | down_write(&per_cpu(cpu_policy_rwsem, policy->cpu)); | ||
963 | |||
955 | policy->last_cpu = policy->cpu; | 964 | policy->last_cpu = policy->cpu; |
956 | policy->cpu = cpu; | 965 | policy->cpu = cpu; |
957 | 966 | ||
967 | up_write(&per_cpu(cpu_policy_rwsem, policy->last_cpu)); | ||
968 | |||
958 | #ifdef CONFIG_CPU_FREQ_TABLE | 969 | #ifdef CONFIG_CPU_FREQ_TABLE |
959 | cpufreq_frequency_table_update_policy_cpu(policy); | 970 | cpufreq_frequency_table_update_policy_cpu(policy); |
960 | #endif | 971 | #endif |
@@ -1200,9 +1211,7 @@ static int __cpufreq_remove_dev_prepare(struct device *dev, | |||
1200 | 1211 | ||
1201 | new_cpu = cpufreq_nominate_new_policy_cpu(policy, cpu, frozen); | 1212 | new_cpu = cpufreq_nominate_new_policy_cpu(policy, cpu, frozen); |
1202 | if (new_cpu >= 0) { | 1213 | if (new_cpu >= 0) { |
1203 | WARN_ON(lock_policy_rwsem_write(cpu)); | ||
1204 | update_policy_cpu(policy, new_cpu); | 1214 | update_policy_cpu(policy, new_cpu); |
1205 | unlock_policy_rwsem_write(cpu); | ||
1206 | 1215 | ||
1207 | if (!frozen) { | 1216 | if (!frozen) { |
1208 | pr_debug("%s: policy Kobject moved to cpu: %d " | 1217 | pr_debug("%s: policy Kobject moved to cpu: %d " |