Re: [RFC PATCH v2 6/8] cpuidle: make fast idle threshold tunable

Rafael J. Wysocki
2017-10-14T01:08:38Z

On Saturday, September 30, 2017 9:20:32 AM CEST Aubrey Li wrote:
> Add a knob to make fast idle threshold tunable
>
> Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>

I first of all am not sure about the need to add a tunable for this at all
in the first place.

> ---
> drivers/cpuidle/cpuidle.c | 3 ++-
> include/linux/cpuidle.h | 1 +
> kernel/sysctl.c | 12 ++++++++++++
> 3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 6cb7e17..5d4f0b6 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -35,6 +35,7 @@ LIST_HEAD(cpuidle_detected_devices);
> static int enabled_devices;
> static int off __read_mostly;
> static int initialized __read_mostly;
> +int sysctl_fast_idle_ratio = 10;
>
> int cpuidle_disabled(void)
> {
> @@ -346,7 +347,7 @@ void cpuidle_predict(void)
> if (!dev)
> return;
>
> - overhead_threshold = dev->idle_stat.overhead;
> + overhead_threshold = dev->idle_stat.overhead * sysctl_fast_idle_ratio;
>
> if (cpuidle_curr_governor->predict) {
> dev->idle_stat.predicted_us = cpuidle_curr_governor->predict();
> diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
> index 791db15..45b8264 100644
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -24,6 +24,7 @@ struct module;
> struct cpuidle_device;
> struct cpuidle_driver;
>
> +extern int sysctl_fast_idle_ratio;
>
> /****************************
> * CPUIDLE DEVICE INTERFACE *
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 6648fbb..97f7e8af 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -67,6 +67,7 @@
> #include <linux/kexec.h>
> #include <linux/bpf.h>
> #include <linux/mount.h>
> +#include <linux/cpuidle.h>
>
> #include <linux/uaccess.h>
> #include <asm/processor.h>
> @@ -1229,6 +1230,17 @@ static struct ctl_table kern_table[] = {
> .extra2 = &one,
> },
> #endif
> +#ifdef CONFIG_CPU_IDLE
> + {
> + .procname = "fast_idle_ratio",
> + .data = &sysctl_fast_idle_ratio,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &one,
> + .extra2 = &one_hundred,
> + },
> +#endif
> { }
> };
>

And if there is a good enough reason to add it, shouldn't the tunable be
there in the cpuidle framework?