Re: [PATCH v3 2/4] cpufreq: introduce cpufreq_driver_might_sleep

[Michael Turquette]

On Mon, Jun 29, 2015 at 10:03 AM, Felipe Balbi <balbi@xxxxxx> wrote:
> Hi,
>
> On Mon, Jun 29, 2015 at 09:56:55AM -0700, Michael Turquette wrote:
>> > > > > @@ -112,6 +112,12 @@ bool have_governor_per_policy(void)
>> > > > >  }
>> > > > >  EXPORT_SYMBOL_GPL(have_governor_per_policy);
>> > > > >
>> > > > > +bool cpufreq_driver_might_sleep(void)
>> > > > > +{
>> > > > > +     return !(cpufreq_driver->flags & CPUFREQ_DRIVER_WILL_NOT_SLEEP);
>> > > > > +}
>> > > > > +EXPORT_SYMBOL_GPL(cpufreq_driver_might_sleep);
>> > > > > +
>> > > > >  struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy)
>> > > > >  {
>> > > > >       if (have_governor_per_policy())
>> > > > > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
>> > > > > index 2ee4888..1f2c9a1 100644
>> > > > > --- a/include/linux/cpufreq.h
>> > > > > +++ b/include/linux/cpufreq.h
>> > > > > @@ -157,6 +157,7 @@ u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy);
>> > > > >  int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu);
>> > > > >  int cpufreq_update_policy(unsigned int cpu);
>> > > > >  bool have_governor_per_policy(void);
>> > > > > +bool cpufreq_driver_might_sleep(void);
>> > > > >  struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy);
>> > > > >  #else
>> > > > >  static inline unsigned int cpufreq_get(unsigned int cpu)
>> > > > > @@ -314,6 +315,14 @@ struct cpufreq_driver {
>> > > > >   */
>> > > > >  #define CPUFREQ_NEED_INITIAL_FREQ_CHECK      (1 << 5)
>> > > > >
>> > > > > +/*
>> > > > > + * Set by drivers that will never block or sleep during their frequency
>> > > > > + * transition. Used to indicate when it is safe to call cpufreq_driver_target
>> > > > > + * from non-interruptable context. Drivers must opt-in to this flag, as the
>> > > > > + * safe default is that they might sleep.
>> > > > > + */
>> > > > > +#define CPUFREQ_DRIVER_WILL_NOT_SLEEP        (1 << 6)
>> > > >
>> > > > don't you need to update current drivers and pass this flag where
>> > > > necessary ?
>> > >
>> > > Felipe,
>> > >
>> > > Thanks for the review.
>> > >
>> > > Setting the flag can be done, but it is an opt-in feature. First, none
>> > > of the legacy cpufreq governors would actually make use of this flag.
>> > > Everything they do is in process context. The first potential user of it
>> > > is in patch #3.
>> > >
>> > > Secondly, the governor in patch #3 will work without this flag set for a
>> > > cpufreq driver. It will just defer the dvfs transition to a kthread
>> > > instead of performing it in the hot path of the scheduler.
>> > >
>> > > Finally, the only hardware I am aware of that can make use of this flag
>> > > is Intel hardware. I know nothing about it and am happy for someone more
>> > > knowledgeable than myself submit a patch enabling this flag for that
>> > > architecture.
>> >
>> > the follow-up question would be: then why introduce the flag at all ?
>> > :-p
>>
>> I included it at Rafael's request:
>>
>> http://lkml.kernel.org/r/<49407954.UBSF2FlX46@xxxxxxxxxxxxxx>
>
> Fair enough, just think it might be an unused code path for a while,
> since the flag isn't enabled anywhere :-s

The point of the series isn't to provide the final word on
scheduler-driven dvfs. The policy needs to be experimented with (see
patch #4) and part of that experimentation is to test on lots of
hardware. Having the flag ready to go will help those that are
experimenting on Intel hardware, or any other platform that has an
async/fast interface to kick off cpu dvfs transitions.

Regards,
Mike

>
> --
> balbi



-- 
Michael Turquette
CEO
BayLibre - At the Heart of Embedded Linux
http://baylibre.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/