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

["Rafael J. Wysocki" <rjw@rjwysocki.net>]

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();