From: "Rafael J. Wysocki" <firstname.lastname@example.org>
To: Lukasz Luba <email@example.com>
Cc: Linux Kernel Mailing List <firstname.lastname@example.org>,
Daniel Lezcano <email@example.com>,
Linux PM <firstname.lastname@example.org>,
Amit Kucheria <email@example.com>,
"Zhang, Rui" <firstname.lastname@example.org>,
Dietmar Eggemann <email@example.com>,
Chris Redpath <Chris.Redpath@arm.com>,
Beata.Michalska@arm.com, Viresh Kumar <firstname.lastname@example.org>,
"Rafael J. Wysocki" <email@example.com>,
Amit Kachhap <firstname.lastname@example.org>
Subject: Re: [RFC PATCH v2 0/6] Introduce Active Stats framework with CPU performance statistics
Date: Tue, 6 Jul 2021 17:28:34 +0200 [thread overview]
Message-ID: <CAJZ5v0ga1O9Y9Lam=BoXofE7sjTNpYVSTjAWvSGZ+j__aCeXJw@mail.gmail.com> (raw)
On Tue, Jul 6, 2021 at 3:18 PM Lukasz Luba <email@example.com> wrote:
> Hi all,
> This patch set introduces a new mechanism: Active Stats framework (ASF), which
> gathers and maintains statistics of CPU performance - time residency at each
> performance state.
> The ASF tracks all the frequency transitions as well as CPU
> idle entry/exit events for all CPUs. Based on that it accounts the active
> (non-idle) residency time for each CPU at each frequency. This information can
> be used by some other subsystems (like thermal governor) to enhance their
> estimations about CPU usage at a given period.
This seems to mean that what is needed is something like the cpufreq
stats but only collected during the time when CPUs are not in idle
> Does it fix something in mainline?
> Yes, there is thermal governor Intelligent Power Allocation (IPA), which
> estimates the CPUs power used in the past. IPA is sampling the CPU utilization
> and frequency and relies on the info available at the time of sampling
> and this imposes the estimation errors.
> The use of ASF solve the issue and enables IPA to make better estimates.
Obviously the IPA is not used on all platforms where cpufreq and
cpuidle are used. What platforms are going to benefit from this
> Why it couldn't be done using existing frameworks?
> The CPUFreq and CPUIdle statistics are not combined, so it is not possible
> to derive the information on how long exactly the CPU was running with a given
But it doesn't mean that the statistics could not be combined.
For instance, the frequency of the CPU cannot change via cpufreq when
active_stats_cpu_idle_enter() is running, so instead of using an
entirely new framework for collecting statistics it might update the
existing cpufreq stats to register that event.
And analogously for the wakeup.
> This new framework combines that information and provides
> it in a handy way.
I'm not convinced about the last piece.
> IMHO it has to be implemented as a new framework, next to
> CPUFreq and CPUIdle, due to a clean design and not just hooks from thermal
> governor into the frequency change and idle code paths.
As far as the design is concerned, I'm not sure if I agree with it.
From my perspective it's all a 1000-line patch that I have to read and
understand to figure out what the design is.
> Tha patch 4/6 introduces a new API for cooling devices, which allows to
> stop tracking the freq and idle statistics.
> The patch set contains also a patches 5/6 6/6 which adds the new power model
> based on ASF into the cpufreq cooling (used by thermal governor IPA).
> It is added as ifdef option, since Active Stats might be not compiled in.
> The ASF is a compile time option, but that might be changed and IPA could
> select it, which would allow to remove some redundant code from
> Comments and suggestions are very welcome.
I'm totally not convinced that it is necessary to put the extra 1000
lines of code into the kernel to address the problem at hand.
next prev parent reply other threads:[~2021-07-06 15:28 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-06 13:18 [RFC PATCH v2 0/6] Introduce Active Stats framework with CPU performance statistics Lukasz Luba
2021-07-06 13:18 ` [RFC PATCH v2 1/6] PM: Introduce Active Stats framework Lukasz Luba
2021-07-06 13:18 ` [RFC PATCH v2 2/6] cpuidle: Add Active Stats calls tracking idle entry/exit Lukasz Luba
2021-07-06 13:18 ` [RFC PATCH v2 3/6] cpufreq: Add Active Stats calls tracking frequency changes Lukasz Luba
2021-07-06 13:18 ` [RFC PATCH v2 4/6] thermal: Add interface to cooling devices to handle governor change Lukasz Luba
2021-07-06 13:18 ` [RFC PATCH v2 5/6] thermal/core/power allocator: Prepare power actors and calm down when not used Lukasz Luba
2021-07-06 13:18 ` [RFC PATCH v2 6/6] thermal: cpufreq_cooling: Improve power estimation based on Active Stats framework Lukasz Luba
2021-07-06 15:28 ` Rafael J. Wysocki [this message]
2021-07-06 15:56 ` [RFC PATCH v2 0/6] Introduce Active Stats framework with CPU performance statistics Lukasz Luba
2021-07-06 16:34 ` Rafael J. Wysocki
2021-07-06 16:51 ` Lukasz Luba
2021-07-14 18:30 ` Rafael J. Wysocki
2021-07-19 8:52 ` Lukasz Luba
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).