| Message ID | 1456190570-4475-3-git-send-email-smuckle@linaro.org (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On Tue, Feb 23, 2016 at 2:22 AM, Steve Muckle <steve.muckle@linaro.org> wrote: > From: Michael Turquette <mturquette@baylibre.com> > > 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@linaro.org: 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@kernel.org> > Cc: Viresh Kumar <viresh.kumar@linaro.org> > Signed-off-by: Michael Turquette <mturquette@baylibre.com> > Signed-off-by: Steve Muckle <smuckle@linaro.org> 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. For example, in the acpi-cpufreq case all depends on what's there in the ACPI tables. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoting Rafael J. Wysocki (2016-02-22 17:31:09) > On Tue, Feb 23, 2016 at 2:22 AM, Steve Muckle <steve.muckle@linaro.org> wrote: > > From: Michael Turquette <mturquette@baylibre.com> > > > > 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@linaro.org: 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@kernel.org> > > Cc: Viresh Kumar <viresh.kumar@linaro.org> > > Signed-off-by: Michael Turquette <mturquette@baylibre.com> > > Signed-off-by: Steve Muckle <smuckle@linaro.org> > > 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. > > 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. 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. Regards, Mike > > Thanks, > Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/25/2016 04:50 PM, Michael Turquette wrote: >> > 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. I was thinking that the reference here was to a driver that may be able to do fast switching for some transitions and not for others, say perhaps depending on the current and target frequencies, or the state of the regulators, or other system conditions. Rafael has proposed a fast_switch() addition to the cpufreq API which currently returns void. Perhaps that could be extended to return success or failure from the driver. The driver aborts if it cannot complete the request atomically and quickly. The scheduler-driven governor could attempt a fast switch if the callback is installed (and the other criteria for the fast switch are met, such as not throttled etc, no request already in flight etc). If the fast switch aborts, fall back to the slow path. I suppose the governor could also just see if policy->cur has changed as opposed to checking cpufreq_driver_fast_switch's return value. But then we can't tell the difference between the fast transition failing because it must be re-attempted in the slow path, and the fast transition failing because of some other more serious reason. In the latter case the request should probably just be dropped rather than retried in the slow path. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@linaro.org> wrote: > > > From: Michael Turquette <mturquette@baylibre.com> > > > > > > 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@linaro.org: 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@kernel.org> > > > Cc: Viresh Kumar <viresh.kumar@linaro.org> > > > Signed-off-by: Michael Turquette <mturquette@baylibre.com> > > > Signed-off-by: Steve Muckle <smuckle@linaro.org> > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 26, 2016 at 7:55 PM, Michael Turquette <mturquette@baylibre.com> wrote: > Quoting Rafael J. Wysocki (2016-02-25 17:16:17) >> 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@linaro.org> wrote: >> > > 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/ > > Thanks for the pointer. Do you mind Cc'ing me on future versions? I'll do that. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index e979ec7..88e63ca 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -154,6 +154,12 @@ bool have_governor_per_policy(void) } EXPORT_SYMBOL_GPL(have_governor_per_policy); +bool cpufreq_driver_is_slow(void) +{ + return !(cpufreq_driver->flags & CPUFREQ_DRIVER_FAST); +} +EXPORT_SYMBOL_GPL(cpufreq_driver_is_slow); + 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 88a4215..93e1c1c 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -160,6 +160,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_is_slow(void); struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy); #else static inline unsigned int cpufreq_get(unsigned int cpu) @@ -316,6 +317,14 @@ struct cpufreq_driver { */ #define CPUFREQ_NEED_INITIAL_FREQ_CHECK (1 << 5) +/* + * Indicates that it is safe to call cpufreq_driver_target from + * non-interruptable context in scheduler hot paths. Drivers must + * opt-in to this flag, as the safe default is that they might sleep + * or be too slow for hot path use. + */ +#define CPUFREQ_DRIVER_FAST (1 << 6) + int cpufreq_register_driver(struct cpufreq_driver *driver_data); int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);