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

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: <CAKfTPtBZvnk11fXB6QgmE_jtNje5N3a5RyB7cFZMRPGbJJ+7KA@mail.gmail.com>
Date: Mon, 17 Jun 2024 10:52:01 +0200
From: Vincent Guittot <vincent.guittot@...aro.org>
To: Lukasz Luba <lukasz.luba@....com>
Cc: Kajetan Puchalski <kajetan.puchalski@....com>, rafael@...nel.org, daniel.lezcano@...aro.org, 
	Dietmar.Eggemann@....com, dsmythies@...us.net, yu.chen.surf@...il.com, 
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Peter Zijlstra <peterz@...radead.org>, Ulf Hansson <ulf.hansson@...aro.org>, 
	Qais Yousef <qyousef@...alina.io>
Subject: Re: [PATCH v6 2/2] cpuidle: teo: Introduce util-awareness

On Wed, 12 Jun 2024 at 11:16, Lukasz Luba <lukasz.luba@....com> wrote:
>
>
>
> On 6/12/24 10:04, Vincent Guittot wrote:
> > On Wed, 12 Jun 2024 at 09:25, Lukasz Luba <lukasz.luba@....com> 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@....com> 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.

I'm not expecting to change PSCI but the cpuidle governor must keep in
mind that it doesn't have the full picture of the final selected idle
state of the system. Typically this means that doing a minus 1 on the
index of the selected cpuidle state is not correct and it must take
into account the final idle states selected outside cpuidle.