Subject: Re: [PATCH 10/14] powerpc/64s: cpuidle set polling before enabling irqs

From: Gautham R Shenoy <[email-masked]>
Date: Mon, 12 Jun 2017 20:40:25 +0530

On Mon, Jun 12, 2017 at 09:58:31AM +1000, Nicholas Piggin wrote:
> local_irq_enable can cause interrupts to be taken which could
> take significant amount of processing time. The idle process
> should set its polling flag before this, so another process that
> wakes it during this time will not have to send an IPI.
> 
> Expand the TIF_POLLING_NRFLAG coverage to as large as possible.
> 
> Signed-off-by: Nicholas Piggin <[email-masked]>

Looks good. Were you able to see this make a difference in any of the
tests ?

Reviewed-by: Gautham R. Shenoy <[email-masked]>

> ---
>  drivers/cpuidle/cpuidle-powernv.c | 4 +++-
>  drivers/cpuidle/cpuidle-pseries.c | 3 ++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
> index 79152676f62b..50b3c2e0306f 100644
> --- a/drivers/cpuidle/cpuidle-powernv.c
> +++ b/drivers/cpuidle/cpuidle-powernv.c
> @@ -51,9 +51,10 @@ static int snooze_loop(struct cpuidle_device *dev,
>  {
>  	u64 snooze_exit_time;
> 
> -	local_irq_enable();
>  	set_thread_flag(TIF_POLLING_NRFLAG);
> 
> +	local_irq_enable();
> +
>  	snooze_exit_time = get_tb() + snooze_timeout;
>  	ppc64_runlatch_off();
>  	HMT_very_low();
> @@ -66,6 +67,7 @@ static int snooze_loop(struct cpuidle_device *dev,
>  	ppc64_runlatch_on();
>  	clear_thread_flag(TIF_POLLING_NRFLAG);
>  	smp_mb();
> +
>  	return index;
>  }
> 
> diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
> index 166ccd711ec9..7b12bb2ea70f 100644
> --- a/drivers/cpuidle/cpuidle-pseries.c
> +++ b/drivers/cpuidle/cpuidle-pseries.c
> @@ -62,9 +62,10 @@ static int snooze_loop(struct cpuidle_device *dev,
>  	unsigned long in_purr;
>  	u64 snooze_exit_time;
> 
> +	set_thread_flag(TIF_POLLING_NRFLAG);
> +
>  	idle_loop_prolog(&in_purr);
>  	local_irq_enable();
> -	set_thread_flag(TIF_POLLING_NRFLAG);
>  	snooze_exit_time = get_tb() + snooze_timeout;
> 
>  	while (!need_resched()) {
> -- 
> 2.11.0
>