[PATCH] ACPI: Fix the incorrect calculation about C-state idle time

[PATCH] ACPI: Fix the incorrect calculation about C-state idle time

[yakui.zhao]

From: Zhao Yakui <yakui.zhao@intel.com>

The C-state idle time is not calculated correctly, which will return the wrong
residency time in C-state. It will have the following effects:
   1.  The system can't choose the deeper C-state when it is idle next time.
Of course the system power is increased. E.g. On one server machine about 40W
idle power is increased.
   2.  The powertop shows that it will stay in C0 running state about 95% time
although the system is idle at most time.

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Reported-by: Yu Zhidong <zhidong.yu@intel.com>
Tested-by: Yu Zhidong <zhidong.yu@intel.com>
CC: Venkatesh Pallipadi <venki@google.com>
---
 drivers/acpi/processor_idle.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 2e8c27d..6b38a6b 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1022,7 +1022,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 		spin_unlock(&c3_lock);
 	}
 	kt2 = ktime_get_real();
-	idle_time_ns = ktime_to_us(ktime_sub(kt2, kt1));
+	idle_time_ns = ktime_to_ns(ktime_sub(kt2, kt1));
 	idle_time = idle_time_ns;
 	do_div(idle_time, NSEC_PER_USEC);
 
-- 
1.5.4.5

Re: [PATCH] ACPI: Fix the incorrect calculation about C-state idle time

[Matthew Garrett]

On Wed, Jun 02, 2010 at 11:04:09AM +0800, yakui.zhao@intel.com wrote:
> From: Zhao Yakui <yakui.zhao@intel.com>
> 
> The C-state idle time is not calculated correctly, which will return the wrong
> residency time in C-state. It will have the following effects:
>    1.  The system can't choose the deeper C-state when it is idle next time.
> Of course the system power is increased. E.g. On one server machine about 40W
> idle power is increased.
>    2.  The powertop shows that it will stay in C0 running state about 95% time
> although the system is idle at most time.

What kind of system was this seen on?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] ACPI: Fix the incorrect calculation about C-state idle time

[ykzhao]

On Wed, 2010-06-02 at 23:06 +0800, Matthew Garrett wrote:
> On Wed, Jun 02, 2010 at 11:04:09AM +0800, yakui.zhao@intel.com wrote:
> > From: Zhao Yakui <yakui.zhao@intel.com>
> > 
> > The C-state idle time is not calculated correctly, which will return the wrong
> > residency time in C-state. It will have the following effects:
> >    1.  The system can't choose the deeper C-state when it is idle next time.
> > Of course the system power is increased. E.g. On one server machine about 40W
> > idle power is increased.
> >    2.  The powertop shows that it will stay in C0 running state about 95% time
> > although the system is idle at most time.
> 
> What kind of system was this seen on?

This can be seen on laptops. But maybe the idle power effect is not
significant on laptops like that on server machine. 
     I also test the power on one T400 laptop and test shows that about
0.3W idle power is increased. 

thanks.
    Yakui 
> 

Re: [PATCH] ACPI: Fix the incorrect calculation about C-state idle time

[Matthew Garrett]

On Thu, Jun 03, 2010 at 09:06:54AM +0800, ykzhao wrote:
> On Wed, 2010-06-02 at 23:06 +0800, Matthew Garrett wrote:
> > What kind of system was this seen on?
> 
> This can be seen on laptops. But maybe the idle power effect is not
> significant on laptops like that on server machine. 
>      I also test the power on one T400 laptop and test shows that about
> 0.3W idle power is increased. 

Thanks, I was able to reproduce this behaviour. Your patch appears to 
fix it.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] ACPI: Fix the incorrect calculation about C-state idle time

[Venkatesh Pallipadi]

On Tue, Jun 1, 2010 at 8:04 PM,  <yakui.zhao@intel.com> wrote:
> From: Zhao Yakui <yakui.zhao@intel.com>
>
> The C-state idle time is not calculated correctly, which will return the wrong
> residency time in C-state. It will have the following effects:
>   1.  The system can't choose the deeper C-state when it is idle next time.
> Of course the system power is increased. E.g. On one server machine about 40W
> idle power is increased.
>   2.  The powertop shows that it will stay in C0 running state about 95% time
> although the system is idle at most time.

This was a bug from my recent patch here :-(
http://marc.info/?l=linux-acpi&m=127198016715509&w=2

Thanks for catching this..

Acked-by: Venkatesh Pallipadi <venki@google.com>

>
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> Reported-by: Yu Zhidong <zhidong.yu@intel.com>
> Tested-by: Yu Zhidong <zhidong.yu@intel.com>
> CC: Venkatesh Pallipadi <venki@google.com>
> ---
>  drivers/acpi/processor_idle.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 2e8c27d..6b38a6b 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1022,7 +1022,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
>                spin_unlock(&c3_lock);
>        }
>        kt2 = ktime_get_real();
> -       idle_time_ns = ktime_to_us(ktime_sub(kt2, kt1));
> +       idle_time_ns = ktime_to_ns(ktime_sub(kt2, kt1));
>        idle_time = idle_time_ns;
>        do_div(idle_time, NSEC_PER_USEC);
>
> --
> 1.5.4.5
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI: Fix the incorrect calculation about C-state idle time

[Len Brown]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1182 bytes --]



thanks,
Len Brown, Intel Open Source Technology Center

On Wed, 2 Jun 2010, Venkatesh Pallipadi wrote:

> On Tue, Jun 1, 2010 at 8:04 PM,  <yakui.zhao@intel.com> wrote:
> > From: Zhao Yakui <yakui.zhao@intel.com>
> >
> > The C-state idle time is not calculated correctly, which will return the wrong
> > residency time in C-state. It will have the following effects:
> >   1.  The system can't choose the deeper C-state when it is idle next time.
> > Of course the system power is increased. E.g. On one server machine about 40W
> > idle power is increased.
> >   2.  The powertop shows that it will stay in C0 running state about 95% time
> > although the system is idle at most time.
> 
> This was a bug from my recent patch here :-(
> http://marc.info/?l=linux-acpi&m=127198016715509&w=2
> 
> Thanks for catching this..
> 
> Acked-by: Venkatesh Pallipadi <venki@google.com>

Another question about that patch...
regarding sched_clock_idle_wakeup_event().
Why do we call it for _simple and _bm, but not for _c1?
Why do we bother calculating its sleep_ticks*PM_TIMER_TICK_NS
argument, when that argument is never accessed?

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH] ACPI: Fix the incorrect calculation about C-state idle time

[Venkatesh Pallipadi]

On Tue, Jun 1, 2010 at 8:04 PM,  <yakui.zhao@intel.com> wrote:
> From: Zhao Yakui <yakui.zhao@intel.com>
>
> The C-state idle time is not calculated correctly, which will return the wrong
> residency time in C-state. It will have the following effects:
>   1.  The system can't choose the deeper C-state when it is idle next time.
> Of course the system power is increased. E.g. On one server machine about 40W
> idle power is increased.
>   2.  The powertop shows that it will stay in C0 running state about 95% time
> although the system is idle at most time.

This was a bug from my recent patch here :-(
http://marc.info/?l=linux-acpi&m=127198016715509&w=2

Thanks for catching this..

Acked-by: Venkatesh Pallipadi <venki@google.com>

>
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> Reported-by: Yu Zhidong <zhidong.yu@intel.com>
> Tested-by: Yu Zhidong <zhidong.yu@intel.com>
> CC: Venkatesh Pallipadi <venki@google.com>
> ---
>  drivers/acpi/processor_idle.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 2e8c27d..6b38a6b 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1022,7 +1022,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
>                spin_unlock(&c3_lock);
>        }
>        kt2 = ktime_get_real();
> -       idle_time_ns = ktime_to_us(ktime_sub(kt2, kt1));
> +       idle_time_ns = ktime_to_ns(ktime_sub(kt2, kt1));
>        idle_time = idle_time_ns;
>        do_div(idle_time, NSEC_PER_USEC);
>
> --
> 1.5.4.5
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI: Fix the incorrect calculation about C-state idle time

[Len Brown]

applied

thanks,
Len Brown, Intel Open Source Technology Center