On Tue, Jun 22, 2021 at 3:59 PM Lukasz Luba [off-list ref] wrote:

On 6/22/21 1:33 PM, Rafael J. Wysocki wrote:

quoted

On Tue, Jun 22, 2021 at 9:59 AM Lukasz Luba [off-list ref] wrote:

quoted

The Active Stats framework tracks and accounts the activity of the CPU
for each performance level. It accounts the real residency,

No, it doesn't.  It just measures the time between the entry and exit
and that's not the real residency (because it doesn't take the exit
latency into account, for example).

It's 'just' a 'model' and as other models has limitations, but it's
better than existing one, which IPA has to use:
cpu_util + currect_freq_at_sampling_time

But the idle duration is already measured by cpuidle as
last_residency_ns.  Why does it need to be measured once more in
addition to that?

quoted

quoted

when the CPU was not idle, at a given performance level. This patch adds needed calls
which provide the CPU idle entry/exit events to the Active Stats
framework.

And it adds overhead to overhead-sensitive code.

AFAICS, some users of that code will not really get the benefit, so
adding the overhead to it is questionable.

First, why is the existing instrumentation in the idle loop insufficient?

The instrumentation (tracing) cannot be used at run time AFAIK. I need
this idle + freq information combined in a running platform, not for
post-processing (like we have in LISA). The thermal governor IPA would
use them for used power estimation.

What about snapshotting last_residency_ns in the CPU wakeup path?

quoted

Second, why do you need to add locking to this code?

The idle entry/exit updates the CPU's accounting data structure.
There is a reader of those data structures: thermal governor,
run from different CPU, which is the reason why I put locking for them.

So please consider doing it in a lockless manner and avoid running
this code when it is not needed in the first place.