Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1328956
| From | Viresh Kumar <viresh.kumar@linaro.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | [PATCH V3 07/13] cpufreq: Call __cpufreq_governor() with policy->rwsem held |
| Date | 2016-02-08 12:50 +0100 |
| Message-ID | <qZSum-4Ei-19@gated-at.bofh.it> (permalink) |
| References | <qZSul-4Ei-3@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
This isn't followed properly by all parts of the core code, some follow
it, whereas others don't.
Enforcing it will also enable us to remove cpufreq_governor_lock, that
is used today because we can't guarantee that __cpufreq_governor() isn't
executed in parallel.
We should also ensure that the lock is held across state changes to the
governors.
For example, while adding a CPU to the policy on cpu-online path, we
need to stop the governor, change policy->cpus, start the governor and
then refresh its limits. The complete sequence must be guaranteed to
execute without any concurrent races. And that can be achieved using
policy->rwsem around these use cases.
Also note that cpufreq_driver->stop_cpu() and ->exit() can get called
while policy->rwsem is held. That shouldn't have any side effects
though.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 49 +++++++++++++++++++++++++++++++----------------
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 863ac26c4ecf..51fb47cd38a0 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1048,30 +1048,29 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cp
if (cpumask_test_cpu(cpu, policy->cpus))
return 0;
+ down_write(&policy->rwsem);
if (has_target()) {
ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
if (ret) {
pr_err("%s: Failed to stop governor\n", __func__);
- return ret;
+ goto unlock;
}
}
- down_write(&policy->rwsem);
cpumask_set_cpu(cpu, policy->cpus);
- up_write(&policy->rwsem);
if (has_target()) {
ret = __cpufreq_governor(policy, CPUFREQ_GOV_START);
if (!ret)
ret = __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
- if (ret) {
+ if (ret)
pr_err("%s: Failed to start governor\n", __func__);
- return ret;
- }
}
- return 0;
+unlock:
+ up_write(&policy->rwsem);
+ return ret;
}
static struct cpufreq_policy *cpufreq_policy_alloc(unsigned int cpu)
@@ -1374,13 +1373,13 @@ static void cpufreq_offline(unsigned int cpu)
return;
}
+ down_write(&policy->rwsem);
if (has_target()) {
ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
if (ret)
pr_err("%s: Failed to stop governor\n", __func__);
}
- down_write(&policy->rwsem);
cpumask_clear_cpu(cpu, policy->cpus);
if (policy_is_inactive(policy)) {
@@ -1393,7 +1392,6 @@ static void cpufreq_offline(unsigned int cpu)
/* Nominate new CPU */
policy->cpu = cpumask_any(policy->cpus);
}
- up_write(&policy->rwsem);
/* Start governor again for active policy */
if (!policy_is_inactive(policy)) {
@@ -1406,7 +1404,7 @@ static void cpufreq_offline(unsigned int cpu)
pr_err("%s: Failed to start governor\n", __func__);
}
- return;
+ goto unlock;
}
if (cpufreq_driver->stop_cpu)
@@ -1428,6 +1426,9 @@ static void cpufreq_offline(unsigned int cpu)
cpufreq_driver->exit(policy);
policy->freq_table = NULL;
}
+
+unlock:
+ up_write(&policy->rwsem);
}
/**
@@ -1624,6 +1625,7 @@ EXPORT_SYMBOL(cpufreq_generic_suspend);
void cpufreq_suspend(void)
{
struct cpufreq_policy *policy;
+ int ret;
if (!cpufreq_driver)
return;
@@ -1634,7 +1636,11 @@ void cpufreq_suspend(void)
pr_debug("%s: Suspending Governors\n", __func__);
for_each_active_policy(policy) {
- if (__cpufreq_governor(policy, CPUFREQ_GOV_STOP))
+ down_write(&policy->rwsem);
+ ret = __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
+ up_write(&policy->rwsem);
+
+ if (ret)
pr_err("%s: Failed to stop governor for policy: %p\n",
__func__, policy);
else if (cpufreq_driver->suspend
@@ -1656,6 +1662,7 @@ void cpufreq_suspend(void)
void cpufreq_resume(void)
{
struct cpufreq_policy *policy;
+ int ret;
if (!cpufreq_driver)
return;
@@ -1668,13 +1675,20 @@ void cpufreq_resume(void)
pr_debug("%s: Resuming Governors\n", __func__);
for_each_active_policy(policy) {
- if (cpufreq_driver->resume && cpufreq_driver->resume(policy))
+ if (cpufreq_driver->resume && cpufreq_driver->resume(policy)) {
pr_err("%s: Failed to resume driver: %p\n", __func__,
policy);
- else if (__cpufreq_governor(policy, CPUFREQ_GOV_START)
- || __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS))
- pr_err("%s: Failed to start governor for policy: %p\n",
- __func__, policy);
+ } else {
+ down_write(&policy->rwsem);
+ ret = __cpufreq_governor(policy, CPUFREQ_GOV_START);
+ if (!ret)
+ __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
+ up_write(&policy->rwsem);
+
+ if (ret)
+ pr_err("%s: Failed to start governor for policy: %p\n",
+ __func__, policy);
+ }
}
/*
@@ -2325,8 +2339,11 @@ static int cpufreq_boost_set_sw(int state)
__func__);
break;
}
+
+ down_write(&policy->rwsem);
policy->user_policy.max = policy->max;
__cpufreq_governor(policy, CPUFREQ_GOV_LIMITS);
+ up_write(&policy->rwsem);
}
}
--
2.7.1.370.gb2aa7f8
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH V3 00/13] cpufreq: governors: Fix ABBA lockups Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 12:50 +0100
[PATCH V3 07/13] cpufreq: Call __cpufreq_governor() with policy->rwsem held Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 12:50 +0100
[PATCH V3 02/13] cpufreq: governor: Move common tunables to 'struct dbs_data' Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 12:50 +0100
[PATCH V3 05/13] Revert "cpufreq: Drop rwsem lock around CPUFREQ_GOV_POLICY_EXIT" Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 12:50 +0100
[PATCH V3 13/13] cpufreq: conservative: Update sample_delay_ns immediately Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 12:50 +0100
[PATCH V3 10/13] cpufreq: governor: No need to manage state machine now Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 12:50 +0100
[PATCH V3 04/13] cpufreq: governor: Drop unused macros for creating governor tunable attributes Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 12:50 +0100
[PATCH V3 09/13] cpufreq: governor: Move common sysfs tunables to cpufreq_governor.c Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 12:50 +0100
Re: [PATCH V3 09/13] cpufreq: governor: Move common sysfs tunables to cpufreq_governor.c "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-08 14:00 +0100
Re: [PATCH V3 09/13] cpufreq: governor: Move common sysfs tunables to cpufreq_governor.c Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 14:10 +0100
Re: [PATCH V3 09/13] cpufreq: governor: Move common sysfs tunables to cpufreq_governor.c "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-08 14:30 +0100
[PATCH V3 01/13] cpufreq: governor: Create generic macro for global tuners Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 12:50 +0100
Re: [PATCH V3 01/13] cpufreq: governor: Create generic macro for global tuners "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-08 17:40 +0100
Re: [PATCH V3 00/13] cpufreq: governors: Fix ABBA lockups Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> - 2016-02-08 14:00 +0100
Re: [PATCH V3 00/13] cpufreq: governors: Fix ABBA lockups Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 14:00 +0100
Re: [PATCH V3 00/13] cpufreq: governors: Fix ABBA lockups Juri Lelli <juri.lelli@arm.com> - 2016-02-08 17:40 +0100
Re: [PATCH V3 00/13] cpufreq: governors: Fix ABBA lockups Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-08 18:00 +0100
Re: [PATCH V3 00/13] cpufreq: governors: Fix ABBA lockups "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-08 22:50 +0100
csiph-web