[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240904062244.GA3614@ranerica-svr.sc.intel.com>
Date: Tue, 3 Sep 2024 23:22:44 -0700
From: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: x86 Maintainers <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
Linux PM <linux-pm@...r.kernel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Ricardo Neri <ricardo.neri@...el.com>,
Tim Chen <tim.c.chen@...el.com>
Subject: Re: [PATCH v3 1/2] x86/sched: Add basic support for CPU capacity
scaling
On Wed, Aug 28, 2024 at 01:47:25PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> In order be able to compute the sizes of tasks consistently across all
> CPUs in a hybrid system, it is necessary to provide CPU capacity scaling
> information to the scheduler via arch_scale_cpu_capacity(). Moreover,
> the value returned by arch_scale_freq_capacity() for the given CPU must
> correspond to the arch_scale_cpu_capacity() return value for it, or
> utilization computations will be inaccurate.
>
> Add support for it through per-CPU variables holding the capacity and
> maximum-to-base frequency ratio (times SCHED_CAPACITY_SCALE) that will
> be returned by arch_scale_cpu_capacity() and used by scale_freq_tick()
> to compute arch_freq_scale for the current CPU, respectively.
>
> In order to avoid adding measurable overhead for non-hybrid x86 systems,
> which are the vast majority in the field, whether or not the new hybrid
> CPU capacity scaling will be in effect is controlled by a static key.
> This static key is set by calling arch_enable_hybrid_capacity_scale()
> which also allocates memory for the per-CPU data and initializes it.
> Next, arch_set_cpu_capacity() is used to set the per-CPU variables
> mentioned above for each CPU and arch_rebuild_sched_domains() needs
> to be called for the scheduler to realize that capacity-aware
> scheduling can be used going forward.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
>
> v2 -> v3:
> * Rebase after dropping patch [1/3].
> * Rename arch_set_cpu_capacity() arguments.
> * Add empty line to arch_enable_hybrid_capacity_scale().
> * Declare local variables in scale_freq_tick() on one line.
>
> v1 -> v2:
> * Replaces WARN_ON_ONCE() with WARN_ONCE() (2 places)
> * Fix arch_enable_hybrid_capacity_scale() return value when hybrid
> capacity scaling is already enabled.
> * Allow arch_enable_hybrid_capacity_scale() to succeed when
> frequency-invariance is not enabled.
> * Fix arch_set_cpu_capacity() kerneldoc comment
> * Do not disable capacity scaling in disable_freq_invariance_workfn().
> * Relocate arch_hybrid_cap_scale_key definition.
>
> ---
Only one minor comment below...
FWIW:
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
Tested-by: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com> # scale invariance
[...]
> +
> +static struct arch_hybrid_cpu_scale __percpu *arch_cpu_scale;
> +
> +/**
> + * arch_enable_hybrid_capacity_scale - Enable hybrid CPU capacity scaling
This looks to me like a kernel-doc comment. The function name should have ().
[...]
> +/**
> + * arch_set_cpu_capacity - Set scale-invariance parameters for a CPU
Same here.
Powered by blists - more mailing lists