Re: [PATCH 20/21] cpuidle: don't calculate time-diff if entered_state == 0

2013-09-26 Thread Daniel Lezcano 

On 09/26/2013 10:28 AM, Viresh Kumar wrote:

On 26 September 2013 13:55, Daniel Lezcano  wrote:

Yes I agree, but why are you saying "If entered_state == 0, we don't need to
..." ?


Ahh.. that's a mistake.. s/==/< :)


Ah ok.



--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 20/21] cpuidle: don't calculate time-diff if entered_state == 0

2013-09-26 Thread Viresh Kumar 
On 26 September 2013 13:55, Daniel Lezcano  wrote:
> Yes I agree, but why are you saying "If entered_state == 0, we don't need to
> ..." ?

Ahh.. that's a mistake.. s/==/< :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 20/21] cpuidle: don't calculate time-diff if entered_state == 0

2013-09-26 Thread Daniel Lezcano 

On 09/26/2013 08:24 AM, Viresh Kumar wrote:

On 26 September 2013 04:08, Daniel Lezcano  wrote:

On 09/22/2013 03:21 AM, Viresh Kumar wrote:

If entered_state == 0, we don't need to set dev->last_residency to 'diff' as we
will be setting it to zero without using its new value.


I don't get it, can you elaborate.


Sure.. We have following code in cpuidle_enter_state():

-
 diff = ktime_to_us(ktime_sub(time_end, time_start));
 if (diff > INT_MAX)
 diff = INT_MAX;

 dev->last_residency = (int) diff;

 if (entered_state >= 0) {
 /* Update cpuidle counters */
 /* This can be moved to within driver enter routine
  * but that results in multiple copies of same code.
  */
 dev->states_usage[entered_state].time += dev->last_residency;
 dev->states_usage[entered_state].usage++;
 } else {
 dev->last_residency = 0;
 }
---

We are setting last_residency to 0 when (entered_state < 0) and aren't using
the value of "diff". So, we can actually skip calculations of time_end, diff and
last_residency when (entered_state < 0).. Makes sense?


Yes I agree, but why are you saying "If entered_state == 0, we don't 
need to ..." ?



We can be a long time in this state
(eg. if the prediction is false).


Okay.. I didn't get it :)




--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 20/21] cpuidle: don't calculate time-diff if entered_state == 0

2013-09-25 Thread Viresh Kumar 
On 26 September 2013 04:08, Daniel Lezcano  wrote:
> On 09/22/2013 03:21 AM, Viresh Kumar wrote:
>> If entered_state == 0, we don't need to set dev->last_residency to 'diff' as 
>> we
>> will be setting it to zero without using its new value.
>
> I don't get it, can you elaborate.

Sure.. We have following code in cpuidle_enter_state():

-
diff = ktime_to_us(ktime_sub(time_end, time_start));
if (diff > INT_MAX)
diff = INT_MAX;

dev->last_residency = (int) diff;

if (entered_state >= 0) {
/* Update cpuidle counters */
/* This can be moved to within driver enter routine
 * but that results in multiple copies of same code.
 */
dev->states_usage[entered_state].time += dev->last_residency;
dev->states_usage[entered_state].usage++;
} else {
dev->last_residency = 0;
}
---

We are setting last_residency to 0 when (entered_state < 0) and aren't using
the value of "diff". So, we can actually skip calculations of time_end, diff and
last_residency when (entered_state < 0).. Makes sense?


> We can be a long time in this state
> (eg. if the prediction is false).

Okay.. I didn't get it :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 20/21] cpuidle: don't calculate time-diff if entered_state == 0

2013-09-25 Thread Daniel Lezcano 
On 09/22/2013 03:21 AM, Viresh Kumar wrote:
> If entered_state == 0, we don't need to set dev->last_residency to 'diff' as 
> we
> will be setting it to zero without using its new value.

I don't get it, can you elaborate. We can be a long time in this state
(eg. if the prediction is false).

> And so move calculation of diff also inside the "if" statement.
> Signed-off-by: Viresh Kumar 
> ---
>  drivers/cpuidle/cpuidle.c | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index bf80236..cb81689 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -77,23 +77,22 @@ int cpuidle_enter_state(struct cpuidle_device *dev, 
> struct cpuidle_driver *drv,
>  
>   struct cpuidle_state *target_state = &drv->states[index];
>   ktime_t time_start, time_end;
> - s64 diff;
>  
>   time_start = ktime_get();
>  
>   entered_state = target_state->enter(dev, drv, index);
>  
> - time_end = ktime_get();
> + if (entered_state >= 0) {
> + s64 diff;
>  
> - local_irq_enable();
> + time_end = ktime_get();
> + diff = ktime_to_us(ktime_sub(time_end, time_start));
>  
> - diff = ktime_to_us(ktime_sub(time_end, time_start));
> - if (diff > INT_MAX)
> - diff = INT_MAX;
> + if (diff > INT_MAX)
> + diff = INT_MAX;
>  
> - dev->last_residency = (int) diff;
> + dev->last_residency = (int) diff;
>  
> - if (entered_state >= 0) {
>   /* Update cpuidle counters */
>   /* This can be moved to within driver enter routine
>* but that results in multiple copies of same code.
> @@ -104,6 +103,8 @@ int cpuidle_enter_state(struct cpuidle_device *dev, 
> struct cpuidle_driver *drv,
>   dev->last_residency = 0;
>   }
>  
> + local_irq_enable();
> +
>   return entered_state;
>  }
>  
> 


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 20/21] cpuidle: don't calculate time-diff if entered_state == 0

2013-09-21 Thread Viresh Kumar 
If entered_state == 0, we don't need to set dev->last_residency to 'diff' as we
will be setting it to zero without using its new value.

And so move calculation of diff also inside the "if" statement.

Signed-off-by: Viresh Kumar 
---
 drivers/cpuidle/cpuidle.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index bf80236..cb81689 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -77,23 +77,22 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct 
cpuidle_driver *drv,
 
struct cpuidle_state *target_state = &drv->states[index];
ktime_t time_start, time_end;
-   s64 diff;
 
time_start = ktime_get();
 
entered_state = target_state->enter(dev, drv, index);
 
-   time_end = ktime_get();
+   if (entered_state >= 0) {
+   s64 diff;
 
-   local_irq_enable();
+   time_end = ktime_get();
+   diff = ktime_to_us(ktime_sub(time_end, time_start));
 
-   diff = ktime_to_us(ktime_sub(time_end, time_start));
-   if (diff > INT_MAX)
-   diff = INT_MAX;
+   if (diff > INT_MAX)
+   diff = INT_MAX;
 
-   dev->last_residency = (int) diff;
+   dev->last_residency = (int) diff;
 
-   if (entered_state >= 0) {
/* Update cpuidle counters */
/* This can be moved to within driver enter routine
 * but that results in multiple copies of same code.
@@ -104,6 +103,8 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct 
cpuidle_driver *drv,
dev->last_residency = 0;
}
 
+   local_irq_enable();
+
return entered_state;
 }
 
-- 
1.7.12.rc2.18.g61b472e

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/