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

[Rik van Riel]

On Thu, 2018-03-22 at 18:09 +0100, Rafael J. Wysocki wrote:

> Does it improve if you do something like the below on top of it?

First off, apologies for doing my testing wrong.
There was an error, which was cleverly hidden by
scripts, and things were not running the way they
should.

After running the tests right, I have seen some
results. My own test patch of disabling polling,
and always dropping into at least C1, resulted in
a CPU use increase of 0.4%.

Your patch below, with a single cpu_relax(), and
the local variable to not read the TSC too often,
seems to give the best performance here, with a
0.2% reduction in CPU use.

Again, apologies for messing up the testing.

I will kick off proper tests for that other patch
series right now.

> > Let me go test the nohz idle series by itself,
> > without this patch.
> 
> OK
> 
> ---
>  drivers/cpuidle/poll_state.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> 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		1000
>  
>  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
>  
>  	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)
> +				continue;
>  
> +			loop_count = 0;
>  			if (local_clock() - time_start >
> POLL_IDLE_TIME_LIMIT)
>  				break;
>  		}
> 
> 
-- 
All Rights Reversed.

Attachment: signature.asc
Description: This is a digitally signed message part