linux-kernel - Re: [PATCH v3 1/2] x86/sched: Add basic support for CPU capacity scaling

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: <CAJZ5v0hh=anBCtCFEfxQ0WJ5Zm8XuXA4UHFWZNhm9m7NSYNCQA@mail.gmail.com>
Date: Wed, 4 Sep 2024 13:24:09 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>, 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, Sep 4, 2024 at 8:17 AM Ricardo Neri
<ricardo.neri-calderon@...ux.intel.com> wrote:
>
> 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

Thanks!

> [...]
>
> > +
> > +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 ().

Well, there are quite a few function kerneldoc comments without the
parens at the end of the name, but sure.

> [...]
> > +/**
> > + * arch_set_cpu_capacity - Set scale-invariance parameters for a CPU
>
> Same here.