Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1324272
| From | Juri Lelli <juri.lelli@arm.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock |
| Date | 2016-02-02 17:50 +0100 |
| Message-ID | <qXMjo-5NR-25@gated-at.bofh.it> (permalink) |
| References | <qXGQG-1uV-11@gated-at.bofh.it> <qXGQI-1uV-47@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Hi Viresh,
On 02/02/16 16:27, Viresh Kumar wrote:
> Invalid state-transitions is verified by governor core now and there is
> no need to replicate that in cpufreq core. Also we don't drop
> policy->rwsem anymore, which makes rest of the races go away.
There are still paths where we call __cpufreq_governor() without holding
policy->rwsem, but those should be fixed with my cleanups (that I intend
to refresh and post soon). So, I'm not sure we can safely remove this
yet.
>
> Simplify code a bit now.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 24 ------------------------
> include/linux/cpufreq.h | 1 -
> 2 files changed, 25 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 5f7e24567e0e..052ad1b9372c 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -102,7 +102,6 @@ static LIST_HEAD(cpufreq_governor_list);
> static struct cpufreq_driver *cpufreq_driver;
> static DEFINE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_data);
> static DEFINE_RWLOCK(cpufreq_driver_lock);
> -DEFINE_MUTEX(cpufreq_governor_lock);
>
> /* Flag to suspend/resume CPUFreq governors */
> static bool cpufreq_suspended;
> @@ -1963,21 +1962,6 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
>
> pr_debug("%s: for CPU %u, event %u\n", __func__, policy->cpu, event);
>
> - mutex_lock(&cpufreq_governor_lock);
> - if ((policy->governor_enabled && event == CPUFREQ_GOV_START)
> - || (!policy->governor_enabled
> - && (event == CPUFREQ_GOV_LIMITS || event == CPUFREQ_GOV_STOP))) {
> - mutex_unlock(&cpufreq_governor_lock);
> - return -EBUSY;
> - }
> -
> - if (event == CPUFREQ_GOV_STOP)
> - policy->governor_enabled = false;
> - else if (event == CPUFREQ_GOV_START)
> - policy->governor_enabled = true;
> -
> - mutex_unlock(&cpufreq_governor_lock);
> -
> ret = policy->governor->governor(policy, event);
So, __cpufreq_governor() becomes effectively a wrapper around
->governor() calls and governors are left responsible for implementing
the state machine with appropriate checks.
I'm wondering if this approach is completely sane, but what we end up
with your changes should work (and we kill a lock! :)).
Maybe we add a comment somewhere stating exactly how things are meant to
work?
Thanks,
- Juri
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 0/5] cpufreq: governors: Solve the ABBA lockups Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-02 12:00 +0100
[PATCH 3/5] cpufreq: governor: Remove unused sysfs attribute macros Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-02 12:00 +0100
Re: [PATCH 3/5] cpufreq: governor: Remove unused sysfs attribute macros "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 22:40 +0100
[PATCH 4/5] cpufreq: Don't drop rwsem before calling CPUFREQ_GOV_POLICY_EXIT Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-02 12:00 +0100
Re: [PATCH 4/5] cpufreq: Don't drop rwsem before calling CPUFREQ_GOV_POLICY_EXIT "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 23:00 +0100
Re: [PATCH 4/5] cpufreq: Don't drop rwsem before calling CPUFREQ_GOV_POLICY_EXIT Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 07:00 +0100
Re: [PATCH 4/5] cpufreq: Don't drop rwsem before calling CPUFREQ_GOV_POLICY_EXIT "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 13:30 +0100
Re: [PATCH 4/5] cpufreq: Don't drop rwsem before calling CPUFREQ_GOV_POLICY_EXIT Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 14:10 +0100
[PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-02 12:00 +0100
Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock Juri Lelli <juri.lelli@arm.com> - 2016-02-02 17:50 +0100
Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 07:10 +0100
Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock Juri Lelli <juri.lelli@arm.com> - 2016-02-03 12:10 +0100
Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 12:10 +0100
Re: [PATCH 5/5] cpufreq: Get rid of ->governor_enabled and its lock "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 23:00 +0100
[PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-02 12:00 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Juri Lelli <juri.lelli@arm.com> - 2016-02-02 16:50 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 17:40 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Juri Lelli <juri.lelli@arm.com> - 2016-02-02 18:10 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 20:50 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Saravana Kannan <skannan@codeaurora.org> - 2016-02-02 23:30 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 00:50 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 02:10 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Saravana Kannan <skannan@codeaurora.org> - 2016-02-03 02:40 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 03:00 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Saravana Kannan <skannan@codeaurora.org> - 2016-02-03 05:10 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 08:00 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Saravana Kannan <skannan@codeaurora.org> - 2016-02-03 21:10 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 08:00 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Juri Lelli <juri.lelli@arm.com> - 2016-02-03 12:00 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 12:00 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Saravana Kannan <skannan@codeaurora.org> - 2016-02-03 21:20 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 08:00 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 07:40 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 22:30 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 08:00 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 13:50 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 14:30 +0100
Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-03 14:40 +0100
Re: [PATCH 0/5] cpufreq: governors: Solve the ABBA lockups Juri Lelli <juri.lelli@arm.com> - 2016-02-02 12:30 +0100
Re: [PATCH 0/5] cpufreq: governors: Solve the ABBA lockups "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-02 21:10 +0100
Re: [PATCH 0/5] cpufreq: governors: Solve the ABBA lockups Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 03:30 +0100
Re: [PATCH 0/5] cpufreq: governors: Solve the ABBA lockups Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-03 12:40 +0100
csiph-web