Re: [RFC PATCH v2 0/1] cpuidle: teo: Introduce optional util-awareness

[Rafael J. Wysocki]

On Thu, Oct 27, 2022 at 9:56 PM Lukasz Luba <lukasz.luba@xxxxxxx> wrote:
>
> Hi Doug,
>
> Thank you for your effort in testing these patches and different
> governors. We really appreciate that, since this helped us to
> better understand the platform that you are using. It is different
> to what we have and our workloads. That's why I have some comments.
>
> It would be hard to combine these two worlds and requirements.
> I have some concerns to the tests, the setup and the platform.
> I can see a reason why this patch has to prove the
> strengths on this platform and environment.
> Please see my comments below.
>
> On 10/13/22 23:12, Doug Smythies wrote:
> > Hi All,
> >
> > On Thu, Oct 13, 2022 at 4:12 AM Kajetan Puchalski
> > <kajetan.puchalski@xxxxxxx> wrote:
> >> On Wed, Oct 12, 2022 at 08:50:39PM +0200, Rafael J. Wysocki wrote:
> >>> On Mon, Oct 3, 2022 at 4:50 PM Kajetan Puchalski
> >>> <kajetan.puchalski@xxxxxxx> wrote:
> > ...
> >
> >> On the Intel & power usage angle you might have seen in the discussion,
> >> Doug sent me some interesting data privately. As far as I can tell the
> >> main issue there is that C0 on Intel doesn't actually do power saving so
> >> moving the state selection down to it is a pretty bad idea because C1
> >> could be very close in terms of latency and save much more power.
> >>
> >> A potential solution could be altering the v2 to only decrease the state
> >> selection by 1 if it's above 1, ie 2->1 but not 1->0. It's fine for us
> >> because arm systems with 2 states use the early exit path anyway. It'd
> >> just amount to changing this hunk:
> >>
> >> +       if (cpu_data->utilized && idx > 0 && !dev->states_usage[idx-1].disable)
> >> +               idx--;
> >>
> >> to:
> >>
> >> +       if (cpu_data->utilized && idx > 1 && !dev->states_usage[idx-1].disable)
> >> +               idx--;
> >>
> >> What would you think about that? Should make it much less intense for
> >> Intel systems.
> >
> > I tested the above, which you sent me as patch version v2-2.
> >
> > By default, my Intel i5-10600K has 4 idle states:
> >
> > $ grep . /sys/devices/system/cpu/cpu7/cpuidle/state*/name
> > /sys/devices/system/cpu/cpu7/cpuidle/state0/name:POLL
>
> This active polling state type worries me a bit. We don't have
> such on our platforms. Our shallowest idle state is really different.
> We don't have active polling and there is no need for such.

So as I said in a reply to Kajetan, the way to go is to avoid them
when you do this utilization-based optimization.

CPUIDLE_FLAG_POLLING is for that and it is used already in the code.

Moreover, as I said in the other message, IMO the utilization-based
optimization makes the most sense when the current candidate state is
state 1, so it may not make sense to do it on Intel systems at all.