This series is partially in response to a discussion around DT bindings
for CPUfreq drivers [0], but it is also needed for on-going work to
integrate CPUfreq with the scheduler. In particular a scheduler-driven
cpu frequency scaling policy would be well served to know if the
underlying CPUfreq driver .target callback might sleep or block for a
long time.
[0] http://lkml.kernel.org/r/<fcb88cd21f31a467d2d49911c2505082837f72ea.1410323179.git.viresh.kumar@linaro.org>
Mike Turquette (2):
cpufreq: add driver flag for sleepable transitions
cpufreq: new function to query driver for flags
drivers/cpufreq/cpufreq.c | 9 +++++++++
include/linux/cpufreq.h | 13 +++++++++++++
2 files changed, 22 insertions(+)
--
1.8.3.2
The CPUfreq core does not differentiate between .target & .target_index
callbacks that may sleep or block and callbacks that are fast and return
immediately. To date this has not mattered much since the typical
CPUfreq governor calls the .target callback from process context via a
workqueue.
When calling the .target callback from a different context, such as a
scheduler load balance operation (see Morten's "energy aware scheduler"
RFC[0]), this detail matters.
This patch introduces a new CPUfreq driver flag for fast .target
callbacks that are guaranteed to neither sleep nor block. Setting this
flag allows for .target to be called from schedule() and other call
sites that have interrupts disabled or other constraints. Drivers may
set CPUFREQ_NO_SLEEP at driver registration-time.
The default is to not have this flag set, resulting in the need to defer
calls to .target and .target_index.
[0] http://lkml.kernel.org/r/<[email protected]>
Signed-off-by: Mike Turquette <[email protected]>
---
include/linux/cpufreq.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 138336b..9034573 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -308,6 +308,19 @@ struct cpufreq_driver {
*/
#define CPUFREQ_NEED_INITIAL_FREQ_CHECK (1 << 5)
+/*
+ * Set by drivers whose .target or .target_index callback will never sleep or
+ * block. Setting this flag allows for more optimal code by calling .target
+ * from a context that otherwise would require deferring the work.
+ *
+ * An example use of this flag is when scaling cpu frequency from within a call
+ * to schedule() with interrupts disabled and runqueue locks held.
+ *
+ * This flag is not set by default, causing calls to .target to be done in
+ * process context.
+ */
+#define CPUFREQ_NO_SLEEP (1 << 6)
+
int cpufreq_register_driver(struct cpufreq_driver *driver_data);
int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
--
1.8.3.2
There are cases for CPUfreq driver flags to be exposed outside of the
CPUfreq core code. In particular the CPUFREQ_NO_SLEEP flag can be used
by CPUfreq governors to optimize when and how they call that drivers
.target callback. In fact this knowledge is a requirement for the
on-going work to initiate cpu frequency transitions from the scheduler.
This patch implements a simple function to return the CPUfreq driver
flags. While currently the flags are a u8, that could grow at a future
date and cpufreq_driver_get_flags returns an int. Additionally the
function needs to return an error in case no driver is registered.
Signed-off-by: Mike Turquette <[email protected]>
---
drivers/cpufreq/cpufreq.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 9b471b2..f3b9042 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1980,6 +1980,15 @@ int cpufreq_driver_target(struct cpufreq_policy *policy,
}
EXPORT_SYMBOL_GPL(cpufreq_driver_target);
+int cpufreq_driver_get_flags(void)
+{
+ if (!cpufreq_driver)
+ return -ENODEV;
+
+ return cpufreq_driver->flags;
+}
+EXPORT_SYMBOL_GPL(cpufreq_driver_get_flags);
+
/*
* when "event" is CPUFREQ_GOV_LIMITS
*/
--
1.8.3.2
On 8 October 2014 13:18, Mike Turquette <[email protected]> wrote:
> This series is partially in response to a discussion around DT bindings
> for CPUfreq drivers [0], but it is also needed for on-going work to
> integrate CPUfreq with the scheduler. In particular a scheduler-driven
> cpu frequency scaling policy would be well served to know if the
> underlying CPUfreq driver .target callback might sleep or block for a
> long time.
>
> [0] http://lkml.kernel.org/r/<fcb88cd21f31a467d2d49911c2505082837f72ea.1410323179.git.viresh.kumar@linaro.org>
Firstly this link is broken and then the last comment I gave in Thomas's
thread was that he doesn't need this routine anymore. And so your scheduler
work is the only user of it.. And so probably we should get this included with
the scheduler patches only, isn't it? Just including them without any user
wouldn't benefit..
Or am I missing something ?
Dear Viresh Kumar,
On Wed, 8 Oct 2014 13:24:30 +0530, Viresh Kumar wrote:
> On 8 October 2014 13:18, Mike Turquette <[email protected]> wrote:
> > This series is partially in response to a discussion around DT bindings
> > for CPUfreq drivers [0], but it is also needed for on-going work to
> > integrate CPUfreq with the scheduler. In particular a scheduler-driven
> > cpu frequency scaling policy would be well served to know if the
> > underlying CPUfreq driver .target callback might sleep or block for a
> > long time.
> >
> > [0] http://lkml.kernel.org/r/<fcb88cd21f31a467d2d49911c2505082837f72ea.1410323179.git.viresh.kumar@linaro.org>
>
> Firstly this link is broken and then the last comment I gave in Thomas's
> thread was that he doesn't need this routine anymore. And so your scheduler
> work is the only user of it.. And so probably we should get this included with
> the scheduler patches only, isn't it? Just including them without any user
> wouldn't benefit..
>
> Or am I missing something ?
Well, when one has to merge a large number of changes, we often
recommend to merge them piece by piece, which is what Mike is trying to
do here. So we cannot at the same time ask developers to merge things
in small pieces that are easy to review and to merge everything
together because the users of a given API are not there yet.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On 8 October 2014 13:41, Thomas Petazzoni
<[email protected]> wrote:
> On Wed, 8 Oct 2014 13:24:30 +0530, Viresh Kumar wrote:
>> On 8 October 2014 13:18, Mike Turquette <[email protected]> wrote:
>> > This series is partially in response to a discussion around DT bindings
>> > for CPUfreq drivers [0], but it is also needed for on-going work to
>> > integrate CPUfreq with the scheduler. In particular a scheduler-driven
> Well, when one has to merge a large number of changes, we often
> recommend to merge them piece by piece, which is what Mike is trying to
> do here. So we cannot at the same time ask developers to merge things
> in small pieces that are easy to review and to merge everything
> together because the users of a given API are not there yet.
>From the wording of Mike it looks like:
- This is required by cpufreq drivers - today
- And this will also be useful for scheduler
The first point doesn't stand true anymore. Lets wait for Mike's reply and
see his opinion.
And then the patches are so small and there are no objections against
them. I don't think getting them with the scheduler changes is that bad
of an idea. In worst case, what if he has to redesign his idea? The new
routines will stay without a caller then :)
--
viresh
Quoting Viresh Kumar (2014-10-08 01:19:40)
> On 8 October 2014 13:41, Thomas Petazzoni
> <[email protected]> wrote:
> > On Wed, 8 Oct 2014 13:24:30 +0530, Viresh Kumar wrote:
> >> On 8 October 2014 13:18, Mike Turquette <[email protected]> wrote:
>
> >> > This series is partially in response to a discussion around DT bindings
> >> > for CPUfreq drivers [0], but it is also needed for on-going work to
> >> > integrate CPUfreq with the scheduler. In particular a scheduler-driven
>
> > Well, when one has to merge a large number of changes, we often
> > recommend to merge them piece by piece, which is what Mike is trying to
> > do here. So we cannot at the same time ask developers to merge things
> > in small pieces that are easy to review and to merge everything
> > together because the users of a given API are not there yet.
>
> From the wording of Mike it looks like:
> - This is required by cpufreq drivers - today
> - And this will also be useful for scheduler
Hi Viresh and Thomas,
Apologies if my wording was confusing. Without getting into a grammar
war, I did say that these patches were "in response" to this thread
(entirely accurate) and only necessary for the "on-going work" I'm doing
with the scheduler. Sorry if any of that came across as me stating that
these patches were necessary to solve Thomas' problem.
>
> The first point doesn't stand true anymore. Lets wait for Mike's reply and
> see his opinion.
>
> And then the patches are so small and there are no objections against
> them. I don't think getting them with the scheduler changes is that bad
> of an idea. In worst case, what if he has to redesign his idea? The new
> routines will stay without a caller then :)
Whether you merge the patches now or later is fine by me. I prefer to
get my patches out early and often so I can avoid any surprises later
on. If you have a fundamental objection to these patches then please let
me know. Otherwise it would be wonderful to have an Ack.
Thanks!
Mike
>
> --
> viresh
On 9 October 2014 05:31, Mike Turquette <[email protected]> wrote:
> Whether you merge the patches now or later is fine by me. I prefer to
> get my patches out early and often so I can avoid any surprises later
> on. If you have a fundamental objection to these patches then please let
> me know. Otherwise it would be wonderful to have an Ack.
Acked-by: Viresh Kumar <[email protected]>
But I would still ask you to get these merged with scheduler changes. :)