linux-kernel - Re: [Patch 2/2] ACPI: processor: Add support to configure CPUFREQ reduction pctg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 Aug 2023 20:40:59 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Sumit Gupta <sumitg@...dia.com>
Cc:     rafael@...nel.org, rui.zhang@...el.com, lenb@...nel.org,
        treding@...dia.com, jonathanh@...dia.com,
        linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
        sanjayc@...dia.com, ksitaraman@...dia.com, srikars@...dia.com,
        jbrasen@...dia.com, bbasu@...dia.com
Subject: Re: [Patch 2/2] ACPI: processor: Add support to configure CPUFREQ
 reduction pctg

On Thu, Aug 17, 2023 at 11:31 AM Sumit Gupta <sumitg@...dia.com> wrote:
>
> From: Srikar Srimath Tirumala <srikars@...dia.com>
>
> Add support to configure the CPUFREQ reduction percentage and set the
> maximum number of throttling steps accordingly. Current implementation
> of processor_thermal performs software throttling in fixed steps of
> "20%" which can be too coarse for some platforms. Change that by adding
> new config to provide the reduction percentage.
>
> Signed-off-by: Srikar Srimath Tirumala <srikars@...dia.com>
> Signed-off-by: Sumit Gupta <sumitg@...dia.com>
> ---
>  drivers/acpi/Kconfig             | 15 +++++++++++++++
>  drivers/acpi/processor_thermal.c | 19 ++++++++++++++++---
>  2 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 00dd309b6682..287cf58defbf 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -254,6 +254,21 @@ config ACPI_DOCK
>  config ACPI_CPU_FREQ_PSS
>         bool
>
> +config ACPI_CPU_FREQ_THERM_HAS_PARAMS
> +       bool "CPU frequency throttling control"
> +       depends on ACPI_PROCESSOR
> +
> +config ACPI_CPU_FREQ_THERM_MIN_THROT_PCTG
> +       int "Minimum throttle percentage for processor_thermal cooling device"
> +       depends on ACPI_CPU_FREQ_THERM_HAS_PARAMS
> +       default 20
> +       help
> +         The processor_thermal driver uses this config to calculate the
> +         percentage amount by which cpu frequency must be reduced for each
> +         cooling state. The config is also used to calculate the maximum number
> +         of throttling steps or cooling states supported by the driver. Value
> +         must be an unsigned integer in the range [1, 50].
> +

I don't think that the new Kconfig symbols are particularly useful.
At least they don't help the distro vendors that each would need to
pick up a specific value for their kernel anyway.

I also wonder how the users building their own kernels are supposed to
determine the values suitable for the target systems.

>  config ACPI_PROCESSOR_CSTATE
>         def_bool y
>         depends on ACPI_PROCESSOR
> diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
> index b7c6287eccca..ee443cc69b73 100644
> --- a/drivers/acpi/processor_thermal.c
> +++ b/drivers/acpi/processor_thermal.c
> @@ -25,8 +25,20 @@
>   * _any_ cpufreq driver and not only the acpi-cpufreq driver.
>   */
>
> -#define CPUFREQ_THERMAL_MIN_STEP 0
> -#define CPUFREQ_THERMAL_MAX_STEP 3
> +#define CPUFREQ_THERMAL_MIN_STEP       0
> +#ifdef CONFIG_ACPI_CPU_FREQ_THERM_HAS_PARAMS
> +#define CPUFREQ_THERMAL_PCTG           CONFIG_ACPI_CPU_FREQ_THERM_MIN_THROT_PCTG
> +
> +/* Derive the MAX_STEP from minimum throttle percentage so that the reduction
> + * percentage does end up becoming negative. Also cap the MAX_STEP so that
> + * the CPU performance doesn't become 0.
> + */
> +#define CPUFREQ_THERMAL_MAX_STEP       ((100 / CPUFREQ_THERMAL_PCTG) - 1)
> +
> +#else
> +#define CPUFREQ_THERMAL_MAX_STEP       3
> +#define CPUFREQ_THERMAL_PCTG           20
> +#endif
>
>  static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
>
> @@ -113,7 +125,8 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
>                 if (!policy)
>                         return -EINVAL;
>
> -               max_freq = (policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100;
> +               max_freq = (policy->cpuinfo.max_freq *
> +                           (100 - reduction_pctg(i) * CPUFREQ_THERMAL_PCTG)) / 100;
>
>                 cpufreq_cpu_put(policy);
>
> --