Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Tue, 22 Jun 2021 16:44:11 +0200 | Subject | Re: [RFC PATCH 2/4] cpuidle: Add Active Stats calls tracking idle entry/exit |
| |
On Tue, Jun 22, 2021 at 3:59 PM Lukasz Luba <lukasz.luba@arm.com> wrote: > > > > On 6/22/21 1:33 PM, Rafael J. Wysocki wrote: > > On Tue, Jun 22, 2021 at 9:59 AM Lukasz Luba <lukasz.luba@arm.com> wrote: > >> > >> 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?
> > > >> 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?
> > > > 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.
| |