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

[Rafael J. Wysocki]

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@xxxxxxxxxxxxxxx>

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?