Re: [PATCH v6 2/2] cpuidle: teo: Introduce util-awareness

[Lukasz Luba]

On 6/12/24 10:04, Vincent Guittot wrote:
> On Wed, 12 Jun 2024 at 09:25, Lukasz Luba <lukasz.luba@xxxxxxx> wrote:
> > Hi Vincent,
> >
> > My apologies for delay, I was on sick leave.
> >
> > On 5/28/24 15:07, Vincent Guittot wrote:
> > > On Tue, 28 May 2024 at 11:59, Lukasz Luba <lukasz.luba@xxxxxxx> wrote:
> > > > Hi Vincent,
> > > >
> > > > On 5/28/24 10:29, Vincent Guittot wrote:
> > > > > Hi All,
> > > > >
> > > > > I'm quite late on this thread but this patchset creates a major
> > > > > regression for psci cpuidle driver when using the OSI mode (OS
> > > > > initiated mode).  In such a case, cpuidle driver takes care only of
> > > > > CPUs power state and the deeper C-states ,which includes cluster and
> > > > > other power domains, are handled with power domain framework. In such
> > > > > configuration ,cpuidle has only 2 c-states : WFI and cpu off states
> > > > > and others states that include the clusters, are managed by genpd and
> > > > > its governor.
> > > > >
> > > > > This patch selects cpuidle c-state N-1 as soon as the utilization is
> > > > > above CPU capacity / 64 which means at most a level of 16 on the big
> > > > > core but can be as low as 4 on little cores. These levels are very low
> > > > > and the main result is that as soon as there is very little activity
> > > > > on a CPU, cpuidle always selects WFI states whatever the estimated
> > > > > sleep duration and which prevents any deeper states. Another effect is
> > > > > that it also keeps the tick firing every 1ms in my case.
> > > >
> > > > Thanks for reporting this.
> > > > Could you add what regression it's causing, please?
> > > > Performance or higher power?
> > >
> > > It's not a perf but rather a power regression. I don't have a power
> > > counter so it's difficult to give figures but I found it while running
> > > a unitary test below on my rb5:
> > > run 500us every 19457ms on medium core (uclamp_min: 600).
> >
> > Mid cores are built differently, they have low static power (leakage).
> > Therefore, for them the residency in deeper idle state should be
> > longer than for Big CPU. When you power off the CPU you loose your
> > cache data/code. The data needs to be stored in the L3 or
> > further memory. When the cpu is powered on again, it needs code & data.
> > Thus, it will transfer that data/code from L3 or from DDR. That
> > information transfer has energy cost (it's not for free). The cost
> > of data from DDR is very high.
> > Then we have to justify if the energy lost while sleeping in shallower
> > idle state can be higher than loading data/code from outside.
> > For different CPU it would be different.
>
> I'm aware of these points and the residency time of an idle state is
> set to reflect this cost. In my case, the idle time is far above the
> residency time which means that we should get some energy saving.
> cpu off 4.488ms
> cluster off 9.987ms
> vs
> sleep duration 18.000ms
>
> Also, the policy of selecting a shallower idle state than the final
> selected one doesn't work with PSCI OSI because cpuidle is only aware
> of per CPU idle states but it is not aware of the cluster or
> deeper/wider idle states so cpuidle doesn't know what will be the
> final selected idle state. This is a major problem, in addition to
> keep the tick firing

I think we are aligned with this.
Something has to change in this implementation of idle gov.

I'm a bit more skeptical about your second point with PSCI.
That standard might be to strong to change.