[PATCH for 2.6.18] [2/8] x86_64: On Intel systems when CPU has C3 don't use TSC

[PATCH for 2.6.18] [2/8] x86_64: On Intel systems when CPU has C3 don't use TSC

[Andi Kleen]

On Intel systems generally the TSC stops in C3 or deeper, 
so don't use it there. Follows similar logic on i386.

This should fix problems on Meroms.

Signed-off-by: Andi Kleen <ak@suse.de>

---
 arch/x86_64/kernel/time.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

Index: linux-2.6.18-rc2-git7/arch/x86_64/kernel/time.c
===================================================================
--- linux-2.6.18-rc2-git7.orig/arch/x86_64/kernel/time.c
+++ linux-2.6.18-rc2-git7/arch/x86_64/kernel/time.c
@@ -28,6 +28,7 @@
 #include <linux/acpi.h>
 #ifdef CONFIG_ACPI
 #include <acpi/achware.h>	/* for PM timer frequency */
+#include <acpi/acpi_bus.h>
 #endif
 #include <asm/8253pit.h>
 #include <asm/pgtable.h>
@@ -953,11 +954,18 @@ __cpuinit int unsynchronized_tsc(void)
 #ifdef CONFIG_SMP
 	if (apic_is_clustered_box())
 		return 1;
- 	/* Intel systems are normally all synchronized. Exceptions
- 	   are handled in the check above. */
- 	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
- 		return 0;
 #endif
+	/* Most intel systems have synchronized TSCs except for
+	   multi node systems */
+ 	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
+#ifdef CONFIG_ACPI
+		/* But TSC doesn't tick in C3 so don't use it there */
+		if (acpi_fadt.length > 0 && acpi_fadt.plvl3_lat < 100)
+			return 1;
+#endif
+ 		return 0;
+	}
+
  	/* Assume multi socket systems are not synchronized */
  	return num_present_cpus() > 1;
 }

Re: [PATCH for 2.6.18] [2/8] x86_64: On Intel systems when CPU has C3 don't use TSC

[Sergio Monteiro Basto]

On Sat, 2006-07-29 at 21:42 +0200, Andi Kleen wrote:
> On Intel systems generally the TSC stops in C3 or deeper, 
> so don't use it there. Follows similar logic on i386.
> 
> This should fix problems on Meroms.
> 
> Signed-off-by: Andi Kleen <ak@suse.de>
> 
...
> +	/* Most intel systems have synchronized TSCs except for
> +	   multi node systems */
> + 	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
> +#ifdef CONFIG_ACPI
> +		/* But TSC doesn't tick in C3 so don't use it there */
> +		if (acpi_fadt.length > 0 && acpi_fadt.plvl3_lat < 100)
> +			return 1;
> +#endif
> + 		return 0;
> +	}
> +
>   	/* Assume multi socket systems are not synchronized */
>   	return num_present_cpus() > 1;
>  }

Hi, 

I had some faith in this patch , but this just enable boot parameter
notsc (which I already use). And "just" disable tsc don't solve all the
problems. 

After "Using ACPI (MADT) for SMP configuration information"
my acpi_fadt.length is great than  0
acpi_fadt.plvl3_lat is 1001

On BIOS 1.40 update description of ASRock, claims this VIA chipset have
C1 stepping support.

this is a Pentium Dual Core on a 775Dual-880Pro
http://www.asrock.com/product/775Dual-880Pro.htm
http://bugme.osdl.org/show_bug.cgi?id=6419

Thanks,
Sérgio M. B.

Re: [discuss] Re: [PATCH for 2.6.18] [2/8] x86_64: On Intel systems when CPU has C3 don't use TSC

[Andi Kleen]

> I had some faith in this patch , but this just enable boot parameter
> notsc (which I already use). And "just" disable tsc don't solve all the
> problems.

What problems do you have?

> 
> 
> After "Using ACPI (MADT) for SMP configuration information"
> my acpi_fadt.length is great than  0
> acpi_fadt.plvl3_lat is 1001

You don't have C3 support so the patch doesn't apply to you.

> On BIOS 1.40 update description of ASRock, claims this VIA chipset have
> C1 stepping support.

C1 stepping is a processor revision; it has nothing to do with
ACPI C* power states.

-Andi

Re: [discuss] Re: [PATCH for 2.6.18] [2/8] x86_64: On Intel systems when CPU has C3 don't use TSC

[Sergio Monteiro Basto]

[-- Attachment #1: Type: text/plain, Size: 704 bytes --]

On Tue, 2006-08-01 at 19:10 +0200, Andi Kleen wrote:
> > I had some faith in this patch , but this just enable boot parameter
> > notsc (which I already use). And "just" disable tsc don't solve all
> the
> > problems.
> 
> What problems do you have?
> 
Hi Andi ,
if I boot without notsc , I have many lost timer tickets.
if I boot with notsc , I have,at begging, a few lost timer ticket, and
after some stress usb modem I got 
uhci_hcd 0000:00:10.1: host controller process error, something bad
happened!
uhci_hcd 0000:00:10.1: host controller halted, very bad!
uhci_hcd 0000:00:10.1: HC died; cleaning up
usb 3-2: USB disconnect, address 2


Thanks for your reply
-- 
Sérgio M. B.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2166 bytes --]

Re: [discuss] Re: [PATCH for 2.6.18] [2/8] x86_64: On Intel systems when CPU has C3 don't use TSC

[Andi Kleen]

On Tuesday 01 August 2006 23:40, Sergio Monteiro Basto wrote:
> On Tue, 2006-08-01 at 19:10 +0200, Andi Kleen wrote:
> > > I had some faith in this patch , but this just enable boot parameter
> > > notsc (which I already use). And "just" disable tsc don't solve all
> > the
> > > problems.
> > 
> > What problems do you have?
> > 
> Hi Andi ,
> if I boot without notsc , I have many lost timer tickets.

Lost timer ticks print a rip. Do you have some samples?

Anyways, for a single socket Intel dual core TSCs should be definitely
synced so I don't know what could be wrong on your system. 
Please post full boot.msg at least.

-Andi

Re: [discuss] Re: [PATCH for 2.6.18] [2/8] x86_64: On Intel systems when CPU has C3 don't use TSC

[Andi Kleen]

On Wednesday 02 August 2006 00:06, Sergio Monteiro Basto wrote:
> On Tue, 2006-08-01 at 23:56 +0200, Andi Kleen wrote:
> > Lost timer ticks print a rip. Do you have some samples?

Can you send dmesg with the following patch applied too?

cc'ing Suresh because he might have an explanation too then.

-Andi

Index: linux-2.6.18-rc3-work/arch/x86_64/kernel/smpboot.c
===================================================================
--- linux-2.6.18-rc3-work.orig/arch/x86_64/kernel/smpboot.c
+++ linux-2.6.18-rc3-work/arch/x86_64/kernel/smpboot.c
@@ -345,7 +345,7 @@ static void __cpuinit tsc_sync_wait(void
 	 * mess up a possible perfect synchronization with a
 	 * not-quite-perfect algorithm.
 	 */
-	if (notscsync || !cpu_has_tsc || !unsynchronized_tsc())
+	if (0 && (notscsync || !cpu_has_tsc || !unsynchronized_tsc()))
 		return;
 	sync_tsc(0);
 }

Re: [discuss] Re: [PATCH for 2.6.18] [2/8] x86_64: On Intel systems when CPU has C3 don't use TSC

[Sergio Monteiro Basto]

[-- Attachment #1.1: Type: text/plain, Size: 2043 bytes --]

ok , here it is dmesg version 16 
I just add this printtk 
--- linux-2.6.17.x86_64/arch/x86_64/kernel/time.c.orig	2006-08-01
01:34:29.000000000 +0100
+++ linux-2.6.17.x86_64/arch/x86_64/kernel/time.c	2006-08-01 23:31:29.000000000 +0100
@@ -959,6 +959,9 @@ __cpuinit int unsynchronized_tsc(void)
 	   multi node systems */
  	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
 #ifdef CONFIG_ACPI
+		printk(KERN_INFO "time.c:SB acpi_fadt.length=%d, acpi_fadt.plvl3_lat=%d unsynchronized_tsc=%d\n",
+					            acpi_fadt.length, acpi_fadt.plvl3_lat,(acpi_fadt.length > 0 && acpi_fadt.plvl3_lat <
+									100) );
 		/* But TSC doesn't tick in C3 so don't use it there */
 		if (acpi_fadt.length > 0 && acpi_fadt.plvl3_lat < 100)
 			return 1;

but this time the function unsynchronized_tsc was just call one time.

time.c:SB acpi_fadt.length=0, acpi_fadt.plvl3_lat=0 unsynchronized_tsc=0

The patch works like I have use boot parameter notsc, which is better.
Now I don't need any parameter to boot.

Big Thanks 

On Wed, 2006-08-02 at 00:21 +0200, Andi Kleen wrote:
> On Wednesday 02 August 2006 00:06, Sergio Monteiro Basto wrote:
> > On Tue, 2006-08-01 at 23:56 +0200, Andi Kleen wrote:
> > > Lost timer ticks print a rip. Do you have some samples?
> 
> Can you send dmesg with the following patch applied too?
> 
> cc'ing Suresh because he might have an explanation too then.
> 
> -Andi
> 
> Index: linux-2.6.18-rc3-work/arch/x86_64/kernel/smpboot.c
> ===================================================================
> --- linux-2.6.18-rc3-work.orig/arch/x86_64/kernel/smpboot.c
> +++ linux-2.6.18-rc3-work/arch/x86_64/kernel/smpboot.c
> @@ -345,7 +345,7 @@ static void __cpuinit tsc_sync_wait(void
>  	 * mess up a possible perfect synchronization with a
>  	 * not-quite-perfect algorithm.
>  	 */
> -	if (notscsync || !cpu_has_tsc || !unsynchronized_tsc())
> +	if (0 && (notscsync || !cpu_has_tsc || !unsynchronized_tsc()))
>  		return;
>  	sync_tsc(0);
>  }
-- 
Sérgio M. B.

[-- Attachment #1.2: dmesg16.bz2 --]
[-- Type: application/x-bzip, Size: 7845 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2166 bytes --]