Re: [RFCv7 PATCH 02/10] cpufreq: introduce cpufreq_driver_is_slow

[Rafael J. Wysocki]

On Thursday, February 25, 2016 04:50:29 PM Michael Turquette wrote:
> Quoting Rafael J. Wysocki (2016-02-22 17:31:09)
> > On Tue, Feb 23, 2016 at 2:22 AM, Steve Muckle <steve.muckle@xxxxxxxxxx> wrote:
> > > From: Michael Turquette <mturquette@xxxxxxxxxxxx>
> > >
> > > Some architectures and platforms perform CPU frequency transitions
> > > through a non-blocking method, while some might block or sleep. Even
> > > when frequency transitions do not block or sleep they may be very slow.
> > > This distinction is important when trying to change frequency from
> > > a non-interruptible context in a scheduler hot path.
> > >
> > > Describe this distinction with a cpufreq driver flag,
> > > CPUFREQ_DRIVER_FAST. The default is to not have this flag set,
> > > thus erring on the side of caution.
> > >
> > > cpufreq_driver_is_slow() is also introduced in this patch. Setting
> > > the above flag will allow this function to return false.
> > >
> > > [smuckle@xxxxxxxxxx: change flag/API to include drivers that are too
> > >  slow for scheduler hot paths, in addition to those that block/sleep]
> > >
> > > Cc: Rafael J. Wysocki <rafael@xxxxxxxxxx>
> > > Cc: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> > > Signed-off-by: Michael Turquette <mturquette@xxxxxxxxxxxx>
> > > Signed-off-by: Steve Muckle <smuckle@xxxxxxxxxx>
> > 
> > Something more sophisticated than this is needed, because one driver
> > may actually be able to do "fast" switching in some cases and may not
> > be able to do that in other cases.
> 
> Those drivers can set the flag dynamically when they probe based on
> their ACPI tables.

No, they can't.

Being able to to the "fast" switching is a property of the policy and
the driver together and it may change with CPU going online/offline.

> > 
> > For example, in the acpi-cpufreq case all depends on what's there in
> > the ACPI tables.
> 
> It's all a moot point until the locking in cpufreq is changed.

No, it isn't.  Look at this, for example: https://patchwork.kernel.org/patch/8426741/

> Until those changes are made it is a bad idea to call cpufreq_driver_target()
> from schedule() context, regardless of the underlying hardware, and all
> platforms should kick that work out to the kthread.

Calling cpufreq_driver_target() from the scheduler is a bad idea overall,
not just because of the locking.

But there are other ways to switch frequencies from scheduler paths.  I run
such code on my test box daily without any problems.

Thanks,
Rafael