Venkatesh Pallipadi | 30 Sep 00:21
Favicon

[patch 0/2] cpuidle: Time accounting fix

As noted in PM minisummit notes here
http://lwn.net/Articles/292447/

Richard Woodruff <r-woodruff2 <at> ti.com> reported a bug
"the CPUIDLE bug that if target is avoided due to BM activity the original
target state is still accounted the time."

Following provides a mechanism to fix the problem for all drivers and fixes
the problem for acpi cpuidle driver.

--

-- 

Venkatesh Pallipadi | 30 Sep 00:24
Favicon

[patch 1/2] cpuidle: use last_state which can reflect the actual state entered

cpuidle accounts the idle time for the C-state it was trying to enter and
not to the actual state that the driver eventually entered. The driver may
select a different state than the one chosen by cpuidle due to
constraints like bus-mastering, etc.

Change the time acounting code to look at the dev->last_state after
returning from target_state->enter(). Driver can modify dev->last_state
internally, inside the enter routine to reflect the actual C-state
entered.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi <at> intel.com>

---
 drivers/cpuidle/cpuidle.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: tip/drivers/cpuidle/cpuidle.c
===================================================================
--- tip.orig/drivers/cpuidle/cpuidle.c	2008-09-23 13:52:59.000000000 -0700
+++ tip/drivers/cpuidle/cpuidle.c	2008-09-23 14:22:43.000000000 -0700
@@ -71,8 +71,11 @@ static void cpuidle_idle_call(void)
 	target_state = &dev->states[next_state];

 	/* enter the state and update stats */
-	dev->last_residency = target_state->enter(dev, target_state);
 	dev->last_state = target_state;
+	dev->last_residency = target_state->enter(dev, target_state);
+	if (dev->last_state)
+		target_state = dev->last_state;
+
(Continue reading)

Kevin Hilman | 1 Oct 13:09
Favicon

Re: [patch 1/2] cpuidle: use last_state which can reflect the actual state entered

Venkatesh Pallipadi <venkatesh.pallipadi <at> intel.com> writes:

> cpuidle accounts the idle time for the C-state it was trying to enter and
> not to the actual state that the driver eventually entered. The driver may
> select a different state than the one chosen by cpuidle due to
> constraints like bus-mastering, etc.
>
> Change the time acounting code to look at the dev->last_state after
> returning from target_state->enter(). Driver can modify dev->last_state
> internally, inside the enter routine to reflect the actual C-state
> entered.
>
> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi <at> intel.com>
>
> ---
>  drivers/cpuidle/cpuidle.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> Index: tip/drivers/cpuidle/cpuidle.c
> ===================================================================
> --- tip.orig/drivers/cpuidle/cpuidle.c	2008-09-23 13:52:59.000000000 -0700
> +++ tip/drivers/cpuidle/cpuidle.c	2008-09-23 14:22:43.000000000 -0700
> @@ -71,8 +71,11 @@ static void cpuidle_idle_call(void)
>  	target_state = &dev->states[next_state];
>  
>  	/* enter the state and update stats */
> -	dev->last_residency = target_state->enter(dev, target_state);
>  	dev->last_state = target_state;
> +	dev->last_residency = target_state->enter(dev, target_state);
> +	if (dev->last_state)
(Continue reading)

Favicon

Re: [patch 1/2] cpuidle: use last_state which can reflect the actual state entered


>-----Original Message-----
>From: Kevin Hilman [mailto:khilman <at> deeprootsystems.com]
>Sent: Wednesday, October 01, 2008 4:09 AM
>To: Pallipadi, Venkatesh
>Cc: lenb <at> kernel.org; linux-pm <at> lists.linux-foundation.org
>Subject: Re: [linux-pm] [patch 1/2] cpuidle: use last_state
>which can reflect the actual state entered
>
>Venkatesh Pallipadi <venkatesh.pallipadi <at> intel.com> writes:
>
>> cpuidle accounts the idle time for the C-state it was trying
>to enter and
>> not to the actual state that the driver eventually entered.
>The driver may
>> select a different state than the one chosen by cpuidle due to
>> constraints like bus-mastering, etc.
>>
>> Change the time acounting code to look at the dev->last_state after
>> returning from target_state->enter(). Driver can modify
>dev->last_state
>> internally, inside the enter routine to reflect the actual C-state
>> entered.
>>
>> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi <at> intel.com>
>>
>> ---
>>  drivers/cpuidle/cpuidle.c |    5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
(Continue reading)

Kevin Hilman | 9 Oct 13:47
Favicon

Re: [patch 1/2] cpuidle: use last_state which can reflect the actual state entered

"Pallipadi, Venkatesh" <venkatesh.pallipadi <at> intel.com> writes:

>>Venkatesh Pallipadi <venkatesh.pallipadi <at> intel.com> writes:
>>
>>> cpuidle accounts the idle time for the C-state it was trying
>>to enter and
>>> not to the actual state that the driver eventually entered.
>>The driver may
>>> select a different state than the one chosen by cpuidle due to
>>> constraints like bus-mastering, etc.
>>>
>>> Change the time acounting code to look at the dev->last_state after
>>> returning from target_state->enter(). Driver can modify
>>dev->last_state
>>> internally, inside the enter routine to reflect the actual C-state
>>> entered.
>>>
>>> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi <at> intel.com>
>>>
>>> ---
>>>  drivers/cpuidle/cpuidle.c |    5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> Index: tip/drivers/cpuidle/cpuidle.c
>>> ===================================================================
>>> --- tip.orig/drivers/cpuidle/cpuidle.c        2008-09-23
>>13:52:59.000000000 -0700
>>> +++ tip/drivers/cpuidle/cpuidle.c     2008-09-23
>>14:22:43.000000000 -0700
>>> @@ -71,8 +71,11 @@ static void cpuidle_idle_call(void)
(Continue reading)

Len Brown | 16 Oct 23:55
Favicon

Re: [patch 1/2] cpuidle: use last_state which can reflect the actual state entered

1,2 applied.

thanks,
-len

On Mon, 29 Sep 2008, Venkatesh Pallipadi wrote:

> cpuidle accounts the idle time for the C-state it was trying to enter and
> not to the actual state that the driver eventually entered. The driver may
> select a different state than the one chosen by cpuidle due to
> constraints like bus-mastering, etc.
> 
> Change the time acounting code to look at the dev->last_state after
> returning from target_state->enter(). Driver can modify dev->last_state
> internally, inside the enter routine to reflect the actual C-state
> entered.
> 
> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi <at> intel.com>
> 
> ---
>  drivers/cpuidle/cpuidle.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> Index: tip/drivers/cpuidle/cpuidle.c
> ===================================================================
> --- tip.orig/drivers/cpuidle/cpuidle.c	2008-09-23 13:52:59.000000000 -0700
> +++ tip/drivers/cpuidle/cpuidle.c	2008-09-23 14:22:43.000000000 -0700
> @@ -71,8 +71,11 @@ static void cpuidle_idle_call(void)
>  	target_state = &dev->states[next_state];
>  
(Continue reading)

Venkatesh Pallipadi | 30 Sep 00:24
Favicon

[patch 2/2] cpuidle: update the last_state acpi cpuidle reflecting actual state entered

reflect the actual state entered in dev->last_state, when actaul state entered
is different from intended one.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi <at> intel.com>

---
 drivers/acpi/processor_idle.c |    1 +
 1 file changed, 1 insertion(+)

Index: tip/drivers/acpi/processor_idle.c
===================================================================
--- tip.orig/drivers/acpi/processor_idle.c	2008-09-23 11:40:50.000000000 -0700
+++ tip/drivers/acpi/processor_idle.c	2008-09-23 14:22:48.000000000 -0700
@@ -1587,6 +1587,7 @@ static int acpi_idle_enter_bm(struct cpu

 	if (acpi_idle_bm_check()) {
 		if (dev->safe_state) {
+			dev->last_state = dev->safe_state;
 			return dev->safe_state->enter(dev, dev->safe_state);
 		} else {
 			local_irq_disable();

--

-- 


Gmane