regression in TSC unstable?

regression in TSC unstable?

[Steven Rostedt]

The ring-buffer-benchmark module creates a producer and consumer and loops 
on do_gettimeofday until it hits about 10 seconds. Then it calculates the 
number of events recorded / time running.

After Ingo merged Linus's latest my tests when from 330ns per entry to 
880ns. After bisecting it I came down to this change:

   commit a71e4917dc0ebbcb5a0ecb7ca3486643c1c9a6e2
   Author: Len Brown <len.brown@intel.com>
   Date:   Tue Apr 21 00:50:11 2009 -0400

    ACPI: idle: mark_tsc_unstable() at init-time, not run-time


I've never had a problem with the TSC with this box before. It passes the 
synchronization phase, and until this commit, the TSC never was marked as 
unstable.

Should it now be unstable? Or is this a false positive?

# cat /proc/cpuinfo 
processor	: 0
vendor_id	: GenuineIntel
cpu family	: 15
model		: 4
model name	: Intel(R) Xeon(TM) CPU 2.80GHz
stepping	: 1
cpu MHz		: 2793.272
cache size	: 1024 KB
physical id	: 0
siblings	: 2
core id		: 0
cpu cores	: 1
apicid		: 0
initial apicid	: 0
fpu		: yes
fpu_exception	: yes
cpuid level	: 5
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx 
lm constant_tsc pebs bts pni dtes64 monitor ds_cpl cid cx16 xtpr
bogomips	: 5586.54
clflush size	: 64
cache_alignment	: 128
address sizes	: 36 bits physical, 48 bits virtual
power management:

Anything else I might need to know?

-- Steve

Re: regression in TSC unstable?

[Len Brown]

On Fri, 8 May 2009, Steven Rostedt wrote:

> The ring-buffer-benchmark module creates a producer and consumer and loops 
> on do_gettimeofday until it hits about 10 seconds. Then it calculates the 
> number of events recorded / time running.
> 
> After Ingo merged Linus's latest

merged what into what?
are you referring to the upstream kernel or something else?

> my tests when from 330ns per entry to 
> 880ns. After bisecting it I came down to this change:
> 
>    commit a71e4917dc0ebbcb5a0ecb7ca3486643c1c9a6e2
>    Author: Len Brown <len.brown@intel.com>
>    Date:   Tue Apr 21 00:50:11 2009 -0400
> 
>     ACPI: idle: mark_tsc_unstable() at init-time, not run-time
> 
> 
> I've never had a problem with the TSC with this box before. It passes the 
> synchronization phase, and until this commit, the TSC never was marked as 
> unstable.
> 
> Should it now be unstable? Or is this a false positive?

If you revert that patch the issue goes away?

Does your Xeon even export any C-states?
(cat /proc/acpi/processor/*/power)

does your dmesg before or afer the patch actually print
"TSC halts in idle"?

thanks,
-Len

> # cat /proc/cpuinfo 
> processor	: 0
> vendor_id	: GenuineIntel
> cpu family	: 15
> model		: 4
> model name	: Intel(R) Xeon(TM) CPU 2.80GHz
> stepping	: 1
> cpu MHz		: 2793.272
> cache size	: 1024 KB
> physical id	: 0
> siblings	: 2
> core id		: 0
> cpu cores	: 1
> apicid		: 0
> initial apicid	: 0
> fpu		: yes
> fpu_exception	: yes
> cpuid level	: 5
> wp		: yes
> flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
> cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx 
> lm constant_tsc pebs bts pni dtes64 monitor ds_cpl cid cx16 xtpr
> bogomips	: 5586.54
> clflush size	: 64
> cache_alignment	: 128
> address sizes	: 36 bits physical, 48 bits virtual
> power management:
> 
> Anything else I might need to know?
> 
> -- Steve
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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: regression in TSC unstable?

[Ingo Molnar]

* Len Brown <lenb@kernel.org> wrote:

> On Fri, 8 May 2009, Steven Rostedt wrote:
> 
> > The ring-buffer-benchmark module creates a producer and consumer and loops 
> > on do_gettimeofday until it hits about 10 seconds. Then it calculates the 
> > number of events recorded / time running.
> > 
> > After Ingo merged Linus's latest
> 
> merged what into what?
> are you referring to the upstream kernel or something else?

he's describing how he found it: a not-yet-upstream kernel-benchmark 
module showed a performance regression that came in via upstream. 
The regression is upstream too, just harder to notice.

	Ingo

Re: Regression in TSC unstable -- confirmed

[Frans Pop]

I'm seeing the same issue with 2.6.30-rc5 (not tested previous RCs on that 
box) on a system for which TSC has always been usable.

$ dmesg | grep TSC
Fast TSC calibration using PIT
checking TSC synchronization [CPU#0 -> CPU#1]: passed.
Marking TSC unstable due to TSC halts in idle

The last line is new with 2.6.30. After reverting a71e4917dc0 the TSC 
works fine again.

The system has a Intel D945GCZ mainboard with dual core Pentium D 
(3.20GHz) processor running x86_64 and Debian Lenny.

Cheers,
FJP

(Thanks to Steven for doing the bisection.)

[PATCH] ACPI: do not mark TSC unstable for invalid C-states

[Thomas Gleixner]

commit a71e4917dc0ebbcb5a0ecb7ca3486643c1c9a6e2 (ACPI: idle:
mark_tsc_unstable() at init-time, not run-time) marks TSC unstable
even on non affected systems.

The reason is that the state enumeration does not check the cx->valid
flag before calling tsc_halts_in_c(), so it marks TSC unstable for any
enumerated albeit invalid C state which is known to turn off TSC.

Check cx->valid before checking whether the TSC might become unstable.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/acpi/processor_idle.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/drivers/acpi/processor_idle.c
===================================================================
--- linux-2.6.orig/drivers/acpi/processor_idle.c
+++ linux-2.6/drivers/acpi/processor_idle.c
@@ -583,7 +583,7 @@ static int acpi_processor_power_verify(s
 
 #if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86)
 		/* TSC could halt in idle, so notify users */
-		if (tsc_halts_in_c(cx->type))
+		if (cx->valid && tsc_halts_in_c(cx->type))
 			mark_tsc_unstable("TSC halts in idle");;
 #endif
 		switch (cx->type) {

Re: [PATCH] ACPI: do not mark TSC unstable for invalid C-states

[Steven Rostedt]

On Mon, 11 May 2009, Thomas Gleixner wrote:

> commit a71e4917dc0ebbcb5a0ecb7ca3486643c1c9a6e2 (ACPI: idle:
> mark_tsc_unstable() at init-time, not run-time) marks TSC unstable
> even on non affected systems.
> 
> The reason is that the state enumeration does not check the cx->valid
> flag before calling tsc_halts_in_c(), so it marks TSC unstable for any
> enumerated albeit invalid C state which is known to turn off TSC.
> 
> Check cx->valid before checking whether the TSC might become unstable.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Tested-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

> ---
>  drivers/acpi/processor_idle.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6/drivers/acpi/processor_idle.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/processor_idle.c
> +++ linux-2.6/drivers/acpi/processor_idle.c
> @@ -583,7 +583,7 @@ static int acpi_processor_power_verify(s
>  
>  #if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86)
>  		/* TSC could halt in idle, so notify users */
> -		if (tsc_halts_in_c(cx->type))
> +		if (cx->valid && tsc_halts_in_c(cx->type))
>  			mark_tsc_unstable("TSC halts in idle");;
>  #endif
>  		switch (cx->type) {
> 
> 
> 

Re: [PATCH] ACPI: do not mark TSC unstable for invalid C-states

[Frans Pop]

On Monday 11 May 2009, Thomas Gleixner wrote:
> commit a71e4917dc0ebbcb5a0ecb7ca3486643c1c9a6e2 (ACPI: idle:
> mark_tsc_unstable() at init-time, not run-time) marks TSC unstable
> even on non affected systems.
>
> The reason is that the state enumeration does not check the cx->valid
> flag before calling tsc_halts_in_c(), so it marks TSC unstable for any
> enumerated albeit invalid C state which is known to turn off TSC.
>
> Check cx->valid before checking whether the TSC might become unstable.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Tested-by: Frans Pop <elendil@planet.nl>

Thanks Thomas.

Re: [PATCH] ACPI: do not mark TSC unstable for invalid C-states

[Janne Kulmala]

Thomas Gleixner wrote:
> commit a71e4917dc0ebbcb5a0ecb7ca3486643c1c9a6e2 (ACPI: idle:
> mark_tsc_unstable() at init-time, not run-time) marks TSC unstable
> even on non affected systems.
> 
> The reason is that the state enumeration does not check the cx->valid
> flag before calling tsc_halts_in_c(), so it marks TSC unstable for any
> enumerated albeit invalid C state which is known to turn off TSC.
> 
> Check cx->valid before checking whether the TSC might become unstable.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  drivers/acpi/processor_idle.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6/drivers/acpi/processor_idle.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/processor_idle.c
> +++ linux-2.6/drivers/acpi/processor_idle.c
> @@ -583,7 +583,7 @@ static int acpi_processor_power_verify(s
>  
>  #if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86)
>  		/* TSC could halt in idle, so notify users */
> -		if (tsc_halts_in_c(cx->type))
> +		if (cx->valid && tsc_halts_in_c(cx->type))
>  			mark_tsc_unstable("TSC halts in idle");;

Small suggestion: can we fix the double ';' here at the same time?

>  #endif
>  		switch (cx->type) {
> 
> 

Janne

Re: [PATCH] ACPI: do not mark TSC unstable for invalid C-states

[Len Brown]

applied (with ;; fixed:-)

sorry for causing the regression,
and thanks for quickly isolating it!

Len Brown, Intel Open Source Technology Center

Re: [PATCH] ACPI: do not mark TSC unstable for invalid C-states

[Len Brown]

hang on, that patch will fail for the case where valid
is (about to be) set to 1 and we'll fail to disable the TSC
for systems which _do_ have c3.  I'll send an updated patch shortly.

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH] ACPI: do not mark TSC unstable for invalid C-states

[Thomas Gleixner]

On Thu, 14 May 2009, Len Brown wrote:

> hang on, that patch will fail for the case where valid
> is (about to be) set to 1 and we'll fail to disable the TSC
> for systems which _do_ have c3.  I'll send an updated patch shortly.

Yep, you are right. I missed the cx->valid update in
apic_processor_power_verify_cX(). So my patch would break the laptops
instead of breaking desktops and servers :)

Moving it to acpi_timer_check_state() is the best we can do and that
saves us ifdeffery as well as acpi_timer_check_state() is already x86
only.

Thanks,

	tglx

[PATCH] ACPI: idle: fix init-time TSC check regression

[Len Brown]

From: Len Brown <len.brown@intel.com>

A previous 2.6.30 patch, a71e4917dc0ebbcb5a0ecb7ca3486643c1c9a6e2,
(ACPI: idle: mark_tsc_unstable() at init-time, not run-time)
erroneously disabled the TSC on systems that did not actually
have valid deep C-states.

Move the check after the deep-C-states are validated,
via new helper, tsc_check_state(), hich replaces tsc_halts_in_c().

Signed-off-by: Len Brown <len.brown@intel.com>
Acked-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
---
Thomas,
I decided to use a tsc-specific helper
rather than piggyback on acpi_timer_check_state(),
since that is not general, but rather LAPIC timer specific.
(and I'll re-name that to reflect its specific function
 in a cosmetic follow-up patch)

 drivers/acpi/processor_idle.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index f7ca8c5..421cc5c 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -216,7 +216,7 @@ int acpi_processor_resume(struct acpi_device * device)
 }
 
 #if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86)
-static int tsc_halts_in_c(int state)
+static void tsc_check_state(int state)
 {
 	switch (boot_cpu_data.x86_vendor) {
 	case X86_VENDOR_AMD:
@@ -226,13 +226,17 @@ static int tsc_halts_in_c(int state)
 		 * C/P/S0/S1 states when this bit is set.
 		 */
 		if (boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
-			return 0;
+			return;
 
 		/*FALL THROUGH*/
 	default:
-		return state > ACPI_STATE_C1;
+		/* TSC could halt in idle, so notify users */
+		if (state > ACPI_STATE_C1)
+			mark_tsc_unstable("TSC halts in idle");
 	}
 }
+#else
+static void tsc_check_state(int state) { return; }
 #endif
 
 static int acpi_processor_get_power_info_fadt(struct acpi_processor *pr)
@@ -581,11 +585,6 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
 	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER; i++) {
 		struct acpi_processor_cx *cx = &pr->power.states[i];
 
-#if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86)
-		/* TSC could halt in idle, so notify users */
-		if (tsc_halts_in_c(cx->type))
-			mark_tsc_unstable("TSC halts in idle");;
-#endif
 		switch (cx->type) {
 		case ACPI_STATE_C1:
 			cx->valid = 1;
@@ -603,6 +602,8 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
 				acpi_timer_check_state(i, pr, cx);
 			break;
 		}
+		if (cx->valid)
+			tsc_check_state(cx->type);
 
 		if (cx->valid)
 			working++;
-- 
1.6.3.1.9.g95405b

Re: [PATCH] ACPI: idle: fix init-time TSC check regression

[Thomas Gleixner]

On Thu, 14 May 2009, Len Brown wrote:
> From: Len Brown <len.brown@intel.com>
> 
> A previous 2.6.30 patch, a71e4917dc0ebbcb5a0ecb7ca3486643c1c9a6e2,
> (ACPI: idle: mark_tsc_unstable() at init-time, not run-time)
> erroneously disabled the TSC on systems that did not actually
> have valid deep C-states.
> 
> Move the check after the deep-C-states are validated,
> via new helper, tsc_check_state(), hich replaces tsc_halts_in_c().
> 
> Signed-off-by: Len Brown <len.brown@intel.com>
> Acked-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> ---
> Thomas,
> I decided to use a tsc-specific helper
> rather than piggyback on acpi_timer_check_state(),
> since that is not general, but rather LAPIC timer specific.
> (and I'll re-name that to reflect its specific function
>  in a cosmetic follow-up patch)

I have no strong opinion on that :)

> +		if (cx->valid)
> +			tsc_check_state(cx->type);
>  
>  		if (cx->valid)
>  			working++;

Could do with

      	 if (cx->valid) {
            	tsc_check_state(cx->type);
                working++;
      	 }

Otherwise Acked-by: Thomas Gleixner <tglx@linutronix.de>

Re: [PATCH] ACPI: idle: fix init-time TSC check regression

[Len Brown]

> > +		if (cx->valid)
> > +			tsc_check_state(cx->type);
> >  
> >  		if (cx->valid)
> >  			working++;
> 
> Could do with
> 
>       	 if (cx->valid) {
>             	tsc_check_state(cx->type);
>                 working++;
>       	 }

right, i deferred that to a cosmetic-only patch
that will also rename and move acpi_timer_check_state.

> Otherwise Acked-by: Thomas Gleixner <tglx@linutronix.de>

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH] ACPI: idle: fix init-time TSC check regression

[Frans Pop]

On Thursday 14 May 2009, Len Brown wrote:
> From: Len Brown <len.brown@intel.com>
>
> A previous 2.6.30 patch, a71e4917dc0ebbcb5a0ecb7ca3486643c1c9a6e2,
> (ACPI: idle: mark_tsc_unstable() at init-time, not run-time)
> erroneously disabled the TSC on systems that did not actually
> have valid deep C-states.
>
> Move the check after the deep-C-states are validated,
> via new helper, tsc_check_state(), hich replaces tsc_halts_in_c().

[...]

> @@ -603,6 +602,8 @@ static int acpi_processor_power_verify(struct
> acpi_processor *pr) 
>                                acpi_timer_check_state(i, pr, cx);
>                        break;
>                }
>+               if (cx->valid)
>+                       tsc_check_state(cx->type);
> 
>                if (cx->valid)
>                        working++;

Shouldn't those last two ifs not simply be combined?
Hmm. I guess not if tsc_check_state can change cx->valid, but that might 
be worth a comment.

But even with that I'll give the patch a try tomorrow.

Cheers,
FJP

[PATCH] ACPI: Idle C-states disabled by max_cstate should not disable the TSC

[Len Brown]

From: Len Brown <len.brown@intel.com>

Processor idle power states C2 and C3 stop the TSC on many machines.
Linux recognizes this situation and marks the TSC as unstable:

Marking TSC unstable due to TSC halts in idle

But if those same machines are booted with "processor.max_cstate=1",
then there is no need to validate C2 and C3, and no need to
disable the TSC, which can be reliably used as a clocksource.

Signed-off-by: Len Brown <len.brown@intel.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 e39a40a..e65476f 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -582,7 +582,7 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
 
 	pr->power.timer_broadcast_on_state = INT_MAX;
 
-	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER; i++) {
+	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
 		struct acpi_processor_cx *cx = &pr->power.states[i];
 
 		switch (cx->type) {
-- 
1.6.3.1.9.g95405b

[PATCH] ACPI: idle: rename lapic timer workaround routines

[Len Brown]

From: Len Brown <len.brown@intel.com>

cosmetic only.  The lapic_timer workaround routines
are specific to the lapic_timer, and are not acpi-generic.

old:

acpi_timer_check_state()
acpi_propagate_timer_broadcast()
acpi_state_timer_broadcast()

new:

lapic_timer_check_state()
lapic_timer_propagate_broadcast()
lapic_timer_state_broadcast()

also, simplify the code in acpi_processor_power_verify()
so that lapic_timer_check_state() is simply called
from one place for all valid C-states, including C1.

Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/processor_idle.c |   35 ++++++++++++++++-------------------
 1 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index e65476f..60c3bcd 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -139,7 +139,7 @@ static void acpi_safe_halt(void)
  * are affected too. We pick the most conservative approach: we assume
  * that the local APIC stops in both C2 and C3.
  */
-static void acpi_timer_check_state(int state, struct acpi_processor *pr,
+static void lapic_timer_check_state(int state, struct acpi_processor *pr,
 				   struct acpi_processor_cx *cx)
 {
 	struct acpi_processor_power *pwr = &pr->power;
@@ -159,7 +159,7 @@ static void acpi_timer_check_state(int state, struct acpi_processor *pr,
 		pr->power.timer_broadcast_on_state = state;
 }
 
-static void acpi_propagate_timer_broadcast(struct acpi_processor *pr)
+static void lapic_timer_propagate_broadcast(struct acpi_processor *pr)
 {
 	unsigned long reason;
 
@@ -170,7 +170,7 @@ static void acpi_propagate_timer_broadcast(struct acpi_processor *pr)
 }
 
 /* Power(C) State timer broadcast control */
-static void acpi_state_timer_broadcast(struct acpi_processor *pr,
+static void lapic_timer_state_broadcast(struct acpi_processor *pr,
 				       struct acpi_processor_cx *cx,
 				       int broadcast)
 {
@@ -187,10 +187,10 @@ static void acpi_state_timer_broadcast(struct acpi_processor *pr,
 
 #else
 
-static void acpi_timer_check_state(int state, struct acpi_processor *pr,
+static void lapic_timer_check_state(int state, struct acpi_processor *pr,
 				   struct acpi_processor_cx *cstate) { }
-static void acpi_propagate_timer_broadcast(struct acpi_processor *pr) { }
-static void acpi_state_timer_broadcast(struct acpi_processor *pr,
+static void lapic_timer_propagate_broadcast(struct acpi_processor *pr) { }
+static void lapic_timer_state_broadcast(struct acpi_processor *pr,
 				       struct acpi_processor_cx *cx,
 				       int broadcast)
 {
@@ -592,24 +592,21 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
 
 		case ACPI_STATE_C2:
 			acpi_processor_power_verify_c2(cx);
-			if (cx->valid)
-				acpi_timer_check_state(i, pr, cx);
 			break;
 
 		case ACPI_STATE_C3:
 			acpi_processor_power_verify_c3(pr, cx);
-			if (cx->valid)
-				acpi_timer_check_state(i, pr, cx);
 			break;
 		}
-		if (cx->valid)
-			tsc_check_state(cx->type);
+		if (!cx->valid)
+			continue;
 
-		if (cx->valid)
-			working++;
+		lapic_timer_check_state(i, pr, cx);
+		tsc_check_state(cx->type);
+		working++;
 	}
 
-	acpi_propagate_timer_broadcast(pr);
+	lapic_timer_propagate_broadcast(pr);
 
 	return (working);
 }
@@ -863,7 +860,7 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
 	 * Must be done before busmaster disable as we might need to
 	 * access HPET !
 	 */
-	acpi_state_timer_broadcast(pr, cx, 1);
+	lapic_timer_state_broadcast(pr, cx, 1);
 
 	if (cx->type == ACPI_STATE_C3)
 		ACPI_FLUSH_CPU_CACHE();
@@ -885,7 +882,7 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
 
 	cx->usage++;
 
-	acpi_state_timer_broadcast(pr, cx, 0);
+	lapic_timer_state_broadcast(pr, cx, 0);
 	cx->time += sleep_ticks;
 	return idle_time;
 }
@@ -952,7 +949,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 	 * Must be done before busmaster disable as we might need to
 	 * access HPET !
 	 */
-	acpi_state_timer_broadcast(pr, cx, 1);
+	lapic_timer_state_broadcast(pr, cx, 1);
 
 	kt1 = ktime_get_real();
 	/*
@@ -997,7 +994,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 
 	cx->usage++;
 
-	acpi_state_timer_broadcast(pr, cx, 0);
+	lapic_timer_state_broadcast(pr, cx, 0);
 	cx->time += sleep_ticks;
 	return idle_time;
 }
-- 
1.6.3.1.9.g95405b

Re: [PATCH] ACPI: idle: rename lapic timer workaround routines

[Thomas Gleixner]

On Fri, 15 May 2009, Len Brown wrote:

> From: Len Brown <len.brown@intel.com>
> 
> cosmetic only.  The lapic_timer workaround routines
> are specific to the lapic_timer, and are not acpi-generic.
> 
> old:
> 
> acpi_timer_check_state()
> acpi_propagate_timer_broadcast()
> acpi_state_timer_broadcast()
> 
> new:
> 
> lapic_timer_check_state()
> lapic_timer_propagate_broadcast()
> lapic_timer_state_broadcast()
> 
> also, simplify the code in acpi_processor_power_verify()
> so that lapic_timer_check_state() is simply called
> from one place for all valid C-states, including C1.
> 
> Signed-off-by: Len Brown <len.brown@intel.com>

Acked-by: Thomas Gleixner <tglx@linutronix.de>

Re: [PATCH] ACPI: Idle C-states disabled by max_cstate should not disable the TSC

[Thomas Gleixner]

On Fri, 15 May 2009, Len Brown wrote:

> From: Len Brown <len.brown@intel.com>
> 
> Processor idle power states C2 and C3 stop the TSC on many machines.
> Linux recognizes this situation and marks the TSC as unstable:
> 
> Marking TSC unstable due to TSC halts in idle
> 
> But if those same machines are booted with "processor.max_cstate=1",
> then there is no need to validate C2 and C3, and no need to
> disable the TSC, which can be reliably used as a clocksource.
> 
> Signed-off-by: Len Brown <len.brown@intel.com>

Acked-by: Thomas Gleixner <tglx@linutronix.de>

> ---
>  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 e39a40a..e65476f 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -582,7 +582,7 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
>  
>  	pr->power.timer_broadcast_on_state = INT_MAX;
>  
> -	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER; i++) {
> +	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
>  		struct acpi_processor_cx *cx = &pr->power.states[i];
>  
>  		switch (cx->type) {
> -- 
> 1.6.3.1.9.g95405b
> 

Re: [PATCH] ACPI: idle: fix init-time TSC check regression

[Frans Pop]

On Thursday 14 May 2009, Len Brown wrote:
> From: Len Brown <len.brown@intel.com>
>
> A previous 2.6.30 patch, a71e4917dc0ebbcb5a0ecb7ca3486643c1c9a6e2,
> (ACPI: idle: mark_tsc_unstable() at init-time, not run-time)
> erroneously disabled the TSC on systems that did not actually
> have valid deep C-states.
>
> Move the check after the deep-C-states are validated,
> via new helper, tsc_check_state(), hich replaces tsc_halts_in_c().
>
> Signed-off-by: Len Brown <len.brown@intel.com>
> Acked-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

OK on both my desktop (which had the regression) and laptop.

Tested-by: Frans Pop <elendil@planet.nl>

(Tested together with the 2 patches you sent later.)

Cheers,
FJP