[PATCH] powerpc/powernv/idle: Round up latency and residency values

[PATCH] powerpc/powernv/idle: Round up latency and residency values

[Vaidyanathan Srinivasan]

On PowerNV platforms, firmware provides exit latency and
target residency for each of the idle states in nano
seconds.  Cpuidle framework expects the values in micro
seconds.  Round up to nearest micro seconds to avoid errors
in cases where the values are defined as fractional micro
seconds.

Default idle state of 'snooze' has exit latency of zero.  If
other states have fractional micro second exit latency, they
would get rounded down to zero micro second and make cpuidle
framework choose deeper idle state when snooze loop is the
right choice.

Reported-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
---
 drivers/cpuidle/cpuidle-powernv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 42896a67aeae..5f3922392059 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -383,9 +383,9 @@ static int powernv_add_idle_states(void)
 		 * Firmware passes residency and latency values in ns.
 		 * cpuidle expects it in us.
 		 */
-		exit_latency = latency_ns[i] / 1000;
+		exit_latency = DIV_ROUND_UP(latency_ns[i], 1000);
 		if (!rc)
-			target_residency = residency_ns[i] / 1000;
+			target_residency = DIV_ROUND_UP(residency_ns[i], 1000);
 		else
 			target_residency = 0;
 
-- 
2.13.5

Re: [PATCH] powerpc/powernv/idle: Round up latency and residency values

[Gautham R Shenoy]

Hi Vaidy,
On Thu, Aug 24, 2017 at 12:28:41AM +0530, Vaidyanathan Srinivasan
wrote:

Cc'ing Rafael and linux-pm list.

> On PowerNV platforms, firmware provides exit latency and
> target residency for each of the idle states in nano
> seconds.  Cpuidle framework expects the values in micro
> seconds.  Round up to nearest micro seconds to avoid errors
> in cases where the values are defined as fractional micro
> seconds.
> 
> Default idle state of 'snooze' has exit latency of zero.  If
> other states have fractional micro second exit latency, they
> would get rounded down to zero micro second and make cpuidle
> framework choose deeper idle state when snooze loop is the
> right choice.
> 
> Reported-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>

This looks good to me.

Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
>  drivers/cpuidle/cpuidle-powernv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
> index 42896a67aeae..5f3922392059 100644
> --- a/drivers/cpuidle/cpuidle-powernv.c
> +++ b/drivers/cpuidle/cpuidle-powernv.c
> @@ -383,9 +383,9 @@ static int powernv_add_idle_states(void)
>  		 * Firmware passes residency and latency values in ns.
>  		 * cpuidle expects it in us.
>  		 */
> -		exit_latency = latency_ns[i] / 1000;
> +		exit_latency = DIV_ROUND_UP(latency_ns[i], 1000);
>  		if (!rc)
> -			target_residency = residency_ns[i] / 1000;
> +			target_residency = DIV_ROUND_UP(residency_ns[i], 1000);
>  		else
>  			target_residency = 0;
> 
> -- 
> 2.13.5
> 

Re: [PATCH] powerpc/powernv/idle: Round up latency and residency values

[Michael Ellerman]

Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> writes:

> On PowerNV platforms, firmware provides exit latency and
> target residency for each of the idle states in nano
> seconds.  Cpuidle framework expects the values in micro
> seconds.  Round up to nearest micro seconds to avoid errors
> in cases where the values are defined as fractional micro
> seconds.
>
> Default idle state of 'snooze' has exit latency of zero.  If
> other states have fractional micro second exit latency, they
> would get rounded down to zero micro second and make cpuidle
> framework choose deeper idle state when snooze loop is the
> right choice.
>
> Reported-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>

This sounds like a fairly bad bug, does it need a Fixes / Cc stable tag?

cheers

Re: [PATCH] powerpc/powernv/idle: Round up latency and residency values

[Vaidyanathan Srinivasan]

* Michael Ellerman <mpe@ellerman.id.au> [2017-08-24 20:28:19]:

> Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> writes:
> 
> > On PowerNV platforms, firmware provides exit latency and
> > target residency for each of the idle states in nano
> > seconds.  Cpuidle framework expects the values in micro
> > seconds.  Round up to nearest micro seconds to avoid errors
> > in cases where the values are defined as fractional micro
> > seconds.
> >
> > Default idle state of 'snooze' has exit latency of zero.  If
> > other states have fractional micro second exit latency, they
> > would get rounded down to zero micro second and make cpuidle
> > framework choose deeper idle state when snooze loop is the
> > right choice.
> >
> > Reported-by: Anton Blanchard <anton@samba.org>
> > Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
> 
> This sounds like a fairly bad bug, does it need a Fixes / Cc stable tag?

Yes, we will need this on stable kernel that runs on POWER9.  On older
platforms the latencies are larger and hence no impact :)

I will post to stable after this fix hits your -next tree.

--Vaidy

Re: powerpc/powernv/idle: Round up latency and residency values

[Michael Ellerman]

On Wed, 2017-08-23 at 18:58:41 UTC, Vaidyanathan Srinivasan wrote:
> On PowerNV platforms, firmware provides exit latency and
> target residency for each of the idle states in nano
> seconds.  Cpuidle framework expects the values in micro
> seconds.  Round up to nearest micro seconds to avoid errors
> in cases where the values are defined as fractional micro
> seconds.
> 
> Default idle state of 'snooze' has exit latency of zero.  If
> other states have fractional micro second exit latency, they
> would get rounded down to zero micro second and make cpuidle
> framework choose deeper idle state when snooze loop is the
> right choice.
> 
> Reported-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/8d4e10e9ed9450e18fbbf6a8872be0

cheers