linux-kernel - Re: [PATCH v2 2/4] powercap: idle_inject: Add prepare/complete callbacks

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: <CAJZ5v0gbNv-SgTEHpjiQEwk7qELg6EkByd-8en0=wZ=abFD2JA@mail.gmail.com>
Date:   Thu, 12 Jan 2023 18:25:04 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Cc:     rafael@...nel.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, daniel.lezcano@...aro.org,
        rui.zhang@...el.com, amitk@...nel.org
Subject: Re: [PATCH v2 2/4] powercap: idle_inject: Add prepare/complete callbacks

On Thu, Jan 12, 2023 at 3:53 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Wed, Nov 30, 2022 at 12:34 AM Srinivas Pandruvada
> <srinivas.pandruvada@...ux.intel.com> wrote:
> >
> > The actual idle percentage can be less than the desired because of
> > interrupts.
>
> This is somewhat unclear.
>
> > Since the objective for CPU Idle injection is for thermal
> > control, there should be some way to compensate for lost idle percentage.
>
> What does "lost idle percentage" mean here?
>
> > Some architectures provide interface to get actual idle percent observed
> > by the hardware. So, the idle percent can be adjusted using the hardware
> > feedback. For example, Intel CPUs provides package idle counters, which
> > is currently used by intel powerclamp driver to adjust idle time.
> >
> > The only way this can be done currently is by monitoring hardware idle
> > percent from a different software thread. This can be avoided by adding
> > callbacks.
> >
> > Add a capability to register a prepare and complete callback during idle
> > inject registry. Add a new register function idle_inject_register_full()
> > which also allows to register callbacks.
> >
> > If they are not NULL, then prepare callback is called before calling
> > play_idle_precise() and complete callback is called after calling
> > play_idle_precise().
> >
> > If prepare callback is present and returns non 0 value then
> > play_idle_precise() is not called to avoid over compensation.
>
> This mechanism isn't particularly straightforward, but maybe there's
> no better way.
>
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
> > ---
> > v2
> > - Replace begin/end with prepare/complete
> > - Add new interface idle_inject_register_full with callbacks
> > - Update kernel doc
> > - Update commit description
> >
> >  drivers/powercap/idle_inject.c | 62 +++++++++++++++++++++++++++++++---
> >  include/linux/idle_inject.h    |  4 +++
> >  2 files changed, 62 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/powercap/idle_inject.c b/drivers/powercap/idle_inject.c
> > index dfa989182e71..f48e71501429 100644
> > --- a/drivers/powercap/idle_inject.c
> > +++ b/drivers/powercap/idle_inject.c
> > @@ -63,13 +63,31 @@ struct idle_inject_thread {
> >   * @idle_duration_us: duration of CPU idle time to inject
> >   * @run_duration_us: duration of CPU run time to allow
> >   * @latency_us: max allowed latency
> > + * @prepare: Callback function which is called before calling
> > + *             play_idle_precise()
> > + * @complete: Callback function which is called after calling
> > + *             play_idle_precise()
>
> What about:
>
> @prepare: Optional callback deciding whether or not to skip idle
> injection in the given cycle.
> @complete: Optional callback updating the state after idle injection.

One more thing: ->complete() is not even used by powerclamp AFAICS, so
I wouldn't add it at this time, because it isn't clear if it's going
to be useful at all.