mbox series

[v4,0/7] consolidate and cleanup CPU capacity

Message ID 20231027080400.56703-1-vincent.guittot@linaro.org
Headers show
Series consolidate and cleanup CPU capacity | expand

Message

Vincent Guittot Oct. 27, 2023, 8:03 a.m. UTC
This is the 1st part of consolidating how the max compute capacity is
used in the scheduler and how we calculate the frequency for a level of
utilization.

Fix some unconsistancy when computing frequency for an utilization. There
can be a mismatch between energy model and schedutil.

Next step will be to make a difference between the original
max compute capacity of a CPU and what is currently available when
there is a capping applying forever (i.e. seconds or more).


Changes since v3:
- Split patch 5 cpufreq/cppc
- Fix topology_init_cpu_capacity_cppc() 
- Fix init if AMU ratio
- Added some tags

Changes since v2:
- Remove the 1st patch which has been queued in tip
- Rework how to initialize the reference frequency for cppc_cpufreq and
  change topology_init_cpu_capacity_cppc() to also set capacity_ref_freq
- Add a RFC to convert AMU to use arch_scale_freq_ref and move the config
  of the AMU ratio to be done when intializing cpu capacity and
  capacity_ref_freq
- Added some tags

Changes since v1:
- Fix typos
- Added changes in cpufreq to use arch_scale_freq_ref() when calling
  arch_set_freq_scale (patch 3).
- arch_scale_freq_ref() is always defined and returns 0 (as proposed
  by Ionela) when not defined by the arch. This simplifies the code with
  the addition of patch 3.
- Simplify Energy Model which always uses arch_scale_freq_ref(). The
  latter returns 0 when not defined by arch instead of last item of the 
  perf domain. This is not a problem because the function is only defined
  for compilation purpose in this case and we don't care about the
  returned value. (patch 5)
- Added changes in cppc cpufreq to set capacity_ref_freq (patch 6)
- Added reviewed tag for patch 1 which got a minor change but not for
  others as I did some changes which could make previous reviewed tag
  no more relevant.

Vincent Guittot (7):
  topology: add a new arch_scale_freq_reference
  cpufreq: use the fixed and coherent frequency for scaling capacity
  cpufreq/schedutil: use a fixed reference frequency
  energy_model: use a fixed reference frequency
  cpufreq/cppc: move and rename cppc_cpufreq_{perf_to_khz|khz_to_perf}
  cpufreq/cppc: set the frequency used for computing the capacity
  arm64/amu: use capacity_ref_freq to set AMU ratio

 arch/arm/include/asm/topology.h   |   1 +
 arch/arm64/include/asm/topology.h |   1 +
 arch/arm64/kernel/topology.c      |  26 +++---
 arch/riscv/include/asm/topology.h |   1 +
 drivers/acpi/cppc_acpi.c          |  93 +++++++++++++++++++++
 drivers/base/arch_topology.c      |  55 ++++++++----
 drivers/cpufreq/cppc_cpufreq.c    | 134 ++++--------------------------
 drivers/cpufreq/cpufreq.c         |   4 +-
 include/acpi/cppc_acpi.h          |   2 +
 include/linux/arch_topology.h     |   8 ++
 include/linux/cpufreq.h           |   9 ++
 include/linux/energy_model.h      |  14 +++-
 kernel/sched/cpufreq_schedutil.c  |  26 +++++-
 13 files changed, 221 insertions(+), 153 deletions(-)

Comments

Ingo Molnar Oct. 27, 2023, 9:32 a.m. UTC | #1
* Vincent Guittot <vincent.guittot@linaro.org> wrote:

> Vincent Guittot (7):
>   topology: add a new arch_scale_freq_reference
>   cpufreq: use the fixed and coherent frequency for scaling capacity
>   cpufreq/schedutil: use a fixed reference frequency
>   energy_model: use a fixed reference frequency
>   cpufreq/cppc: move and rename cppc_cpufreq_{perf_to_khz|khz_to_perf}
>   cpufreq/cppc: set the frequency used for computing the capacity
>   arm64/amu: use capacity_ref_freq to set AMU ratio

Just a general comment wrt. titles: please capitalize the verb, as we'd do 
in a proper sentence. Saves maintainers one more thing to fix when applying 
patches.

Thanks,

	Ingo
Ingo Molnar Oct. 27, 2023, 9:38 a.m. UTC | #2
* Vincent Guittot <vincent.guittot@linaro.org> wrote:

> +/* Look up the max frequency in DMI */
> +static u64 cppc_get_dmi_max_khz(void)
> +{
> +	u16 mhz = 0;
> +
> +	dmi_walk(cppc_find_dmi_mhz, &mhz);
> +
> +	/*
> +	 * Real stupid fallback value, just in case there is no
> +	 * actual value set.
> +	 */
> +	mhz = mhz ? mhz : 1;
> +
> +	return (1000 * mhz);

Nit, and I realize this is pre-existing code, but 'return' is a keyword, 
not a function, so the parentheses are not needed.

Thanks,

	Ingo