Re: [PATCH v3] cpuidle: poll_state: Add time limit to poll_idle()

[Rafael J. Wysocki]

On Sunday, March 25, 2018 10:15:52 PM CEST Rik van Riel wrote:
> 
> --=-e8yLbs0aoH4SrxOskwwl
> Content-Type: text/plain; charset="UTF-8"
> Content-Transfer-Encoding: quoted-printable
> 
> On Thu, 2018-03-22 at 18:09 +0100, Rafael J. Wysocki wrote:
> >=20
> > +++ linux-pm/drivers/cpuidle/poll_state.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/sched/idle.h>
> > =20
> >  #define POLL_IDLE_TIME_LIMIT       (TICK_NSEC / 16)
> > +#define POLL_IDLE_COUNT            1000
> > =20
> >  static int __cpuidle poll_idle(struct cpuidle_device *dev,
> >                            struct cpuidle_driver *drv, int
> > index)
> > @@ -18,9 +19,14 @@ static int __cpuidle poll_idle(struct cp
> > =20
> >     local_irq_enable();
> >     if (!current_set_polling_and_test()) {
> > +           unsigned int loop_count =3D 0;
> > +
> >             while (!need_resched()) {
> >                     cpu_relax();
> > +                   if (loop_count++ < POLL_IDLE_COUNT)
> > +                           continue;
> > =20
> > +                   loop_count =3D 0;
> >                     if (local_clock() - time_start >
> > POLL_IDLE_TIME_LIMIT)
> >                             break;
> >             }
> 
> OK, I am still seeing a performance
> degradation with the above, though
> not throughout the entire workload.
> 
> It appears that making the idle loop
> do anything besides cpu_relax() for
> a significant amount of time slows
> things down.

I see.

> I plan to try two more things:
> 
> 1) Disable polling on SMT systems, with
>    the idea that putting one thread to
>    sleep with monitor/mwait in C1 will
>    allow the other thread to run faster.

Sounds plausible.

> 2) Insert more cpu_relax() calls into the
>    main loop, so the CPU core spends more
>    of its time in cpu_relax() and less
>    time doing other things:

Well, maybe it's a matter of doing cpu_relax() between any other bits of
significant computation in there:


---
 drivers/cpuidle/poll_state.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/cpuidle/poll_state.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/poll_state.c
+++ linux-pm/drivers/cpuidle/poll_state.c
@@ -10,6 +10,7 @@
 #include <linux/sched/idle.h>
 
 #define POLL_IDLE_TIME_LIMIT   (TICK_NSEC / 16)
+#define POLL_IDLE_COUNT                200
 
 static int __cpuidle poll_idle(struct cpuidle_device *dev,
                               struct cpuidle_driver *drv, int index)
@@ -18,11 +19,21 @@ static int __cpuidle poll_idle(struct cp
 
        local_irq_enable();
        if (!current_set_polling_and_test()) {
+               unsigned int loop_count = 0;
+
                while (!need_resched()) {
                        cpu_relax();
-
+                       if (loop_count++ < POLL_IDLE_COUNT) {
+                               cpu_relax();
+                               continue;
+                       }
+                       cpu_relax();
+                       loop_count = 0;
+                       cpu_relax();
                        if (local_clock() - time_start > POLL_IDLE_TIME_LIMIT)
                                break;
+
+                       cpu_relax();
                }
        }
        current_clr_polling();