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]

Message-ID: <CAJZ5v0hBmOh3gOa71sAV1kbzCzoJO-gphr4CEgyA6+-+FquvOQ@mail.gmail.com>
Date:   Mon, 21 Aug 2023 19:09:24 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Sumit Gupta <sumitg@...dia.com>
Cc:     "Rafael J. Wysocki" <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 Mon, Aug 21, 2023 at 3:24 PM Sumit Gupta <sumitg@...dia.com> wrote:
>
>
>
> On 19/08/23 00:10, Rafael J. Wysocki wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > 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.
> >
>
> We observed some perf gain after reducing the throttle percentage.
> Currently, kept the default to '20%' as before.

So you should add this information to the patch changelog, ideally
along with the description of the hardware configuration in which the
improvement has been observed.

> Based on need, a vendor can overwrite the default value with macro
> 'CONFIG_ACPI_CPU_FREQ_THERM_MIN_THROT_PCTG'. Otherwise, the behavior
> will remain same.

Yes, that's how it works.

What I'm saying is that the way it works does not appear to be
particularly useful.

For example, how exactly is a distribution supposed to guess the
"right" value for their general-purpose kernel?