Messages in this thread | | | Subject | Re: [PATCH v3] cpuidle: poll_state: Add time limit to poll_idle() | From | Rik van Riel <> | Date | Sun, 25 Mar 2018 17:45:13 -0400 |
| |
On Sun, 2018-03-25 at 23:34 +0200, Rafael J. Wysocki wrote: > 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:
That sounds like a plausible thing to try. Let me kick off a test with that variant, too.
> 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(); > > -- All Rights Reversed.[unhandled content-type:application/pgp-signature] | |