Re: [PATCH] cpuidle: use high confidence factors only when considering polling

[Rik van Riel <riel@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4969 bytes --]

On Fri, 2016-03-18 at 23:56 +0100, Rafael J. Wysocki wrote:
> On Fri, Mar 18, 2016 at 10:35 PM, Rik van Riel <riel@redhat.com>
> wrote:
> > 
> > On Fri, 18 Mar 2016 11:32:28 -0700
> > "Doug Smythies" <dsmythies@telus.net> wrote:
> > 
> > > 
> > > Energy:
> > > Kernel 4.5-rc7-rjw10-rvr6: 55864 Joules
> > > 
> > > Trace data (very crude summary):
> > > Kernel 4.5-rc7-rjw10-rvr5: ~3049 long durations at high CPU load
> > > (idle state 0)
> > > Kernel 4.5-rc7-rjw10-rvr5: ~183 long durations at high, but less,
> > > CPU load (not all idle state 0)
> > This is a bit of a big hammer, but since the polling loop already
> > uses
> > full CPU power anyway, this could be harmless, and solve your
> > problem.
> > 
> > ---8<---
> > 
> > Subject: cpuidle: break out of idle polling loop after HLT
> > threshold
> > 
> > Staying in the poll loop can be beneficial for workloads that
> > wake back up very, very quickly, but staying in the poll loop
> > for too long can waste a lot of power.
> > 
> > Break out of the idle polling loop if the system has spent more
> > time in the loop than the threshold for entering the next power
> > saving mode.
> > 
> > The poll loop uses full CPU power already, so this will not
> > increase the power use of the poll loop.
> > 
> > Signed-off-by: Rik van Riel <riel@redhat.com>
> > ---
> >  drivers/cpuidle/driver.c | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
> > index 389ade4572be..a5dfeafd243f 100644
> > --- a/drivers/cpuidle/driver.c
> > +++ b/drivers/cpuidle/driver.c
> > @@ -181,10 +181,29 @@ static void __cpuidle_driver_init(struct
> > cpuidle_driver *drv)
> >  static int poll_idle(struct cpuidle_device *dev,
> >                 struct cpuidle_driver *drv, int index)
> >  {
> > +       /*
> > +        * Polling is state 0; break out of the polling loop after
> > the
> > +        * threshold for the next power state has passed. Doing
> > this several
> > +        * times in a row should cause the menu governor to
> > immediately
> > +        * select a deeper power state.
> > +        */
> > +       u64 limit = drv->states[1].target_residency *
> > NSEC_PER_USEC;
> > +       ktime_t start = ktime_get();
> > +       int i = 0;
> > +
> >         local_irq_enable();
> >         if (!current_set_polling_and_test()) {
> > -               while (!need_resched())
> > +               while (!need_resched()) {
> > +                       ktime_t now;
> >                         cpu_relax();
> > +
> > +                       /* Occasionally check for a timeout. */
> > +                       if (!(i % 1000)) {
> > +                               now = ktime_get();
> > +                               if (ktime_to_ns(ktime_sub(now,
> > start)) > limit)
> > +                                       break;
> > +                       }
> > +               }
> >         }
> >         current_clr_polling();
> I'm not sure how this is going to help, but maybe I'm missing
> something.
> 
> Say we hit the timeout and break out of the loop.  We get back to
> cpuidle_enter_state() and from there (via a couple of irrelevant
> steps) to cpuidle_idle_call() and to cpu_idle_loop().  Then, since we
> are still idle, the while (1) loop continues and we land in
> menu_select() again.  That will choose polling again (because now
> there's less time to the next timer than it was last time, so if it
> didn't choose C1 then, it won't do that now either) and we land in
> idle_poll() again.
> 
> So is there anything I'm overlooking?

After about three microseconds of the above, get_typical_interval()
will return an interval larger than the C1 target residency, or
fail to find a typical interval, because 3 of the 8 intervals will
exceed the C1 target residency at that point.

Only repeated very fast wakeups will lead to polling, and just
a few periods of polling timing out will lead to the system
reverting to HLT.

In other words, the system will only use polling while it appears
to be beneficial, and should snap out of that mode very quickly.

The kernel test robot also showed over 6% improvement in (IIRC)
the wikibench test, with polling being done a little more often.

Other parts of the kernel do pretty much anything for 2% extra
performance, so if we can find a way to preserve that 6%, without
hurting power usage significantly, I suspect it may be worthwhile.

-- 
All Rights Reversed.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]