linux-kernel - Re: [PATCH v3] cpuidle: poll_state: Add time limit to poll

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1522014313.6308.48.camel@surriel.com>
Date:   Sun, 25 Mar 2018 17:45:13 -0400
From:   Rik van Riel <riel@...riel.com>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:     Linux PM <linux-pm@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Frederic Weisbecker <fweisbec@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Paul McKenney <paulmck@...ux.vnet.ibm.com>,
        Thomas Ilsche <thomas.ilsche@...dresden.de>,
        Doug Smythies <dsmythies@...us.net>,
        Aubrey Li <aubrey.li@...ux.intel.com>,
        Mike Galbraith <mgalbraith@...e.de>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] cpuidle: poll_state: Add time limit to poll_idle()

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.
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)