tsc-disable_A5

Hey Marcelo,
I know its probably poor form to send this out so close to -rc, but I
figured I might as well give it a shot. I'll happily resubmit this for the .20pre series later if you'd prefer.

This patch disables the TSCs when compiled for Multiquad NUMA hardware. Due to the slower interconnect, the TSCs aren't being synced properly at boot time. Even if they were synced, since the different nodes are driven by different crystals, the TSCs still drift.

This results in sequential calls to gettimeofday to return non-sequential time values. By disabling the TSCs on these boxes, it forces gettimeofday to use the PIC clock instead, fixing the problem.

Let me know if you have any comments.

Thanks
-john

diff -Nru a/arch/i386/config.in b/arch/i386/config.in
@@ -198,7 +198,17 @@
       define_bool CONFIG_X86_IO_APIC y
    fi
 else
-   bool 'Multiquad NUMA system' CONFIG_MULTIQUAD
+	bool 'Multiquad NUMA support' CONFIG_X86_NUMA
+	if [ "$CONFIG_X86_NUMA" = "y" ]; then
+		choice 'NUMA Hardware Selection' \
+		"NUMAQ					CONFIG_X86_NUMAQ \
+		 x440					CONFIG_X86_X440" NUMAQ
+		if [ "$CONFIG_X86_NUMAQ" = "y" ]; then
+			define_bool CONFIG_MULTIQUAD y
+		fi
+		# Multiquad NUMA boxes can't keep their TSCs in sync
+		define_bool CONFIG_TSC_DISABLE y
+	fi
 fi
 
 if [ "$CONFIG_SMP" = "y" -a "$CONFIG_X86_CMPXCHG" = "y" ]; then

diff -Nru a/arch/i386/kernel/setup.c b/arch/i386/kernel/setup.c
@@ -1127,8 +1127,12 @@
 __setup("cachesize=", cachesize_setup);
 
 
-#ifndef CONFIG_X86_TSC
+#if !defined(CONFIG_X86_TSC)||defined(CONFIG_TSC_DISABLE)
+#ifdef CONFIG_TSC_DISABLE
+static int tsc_disable __initdata = 1;	
+#else /*CONFIG_TSC_DISABLE*/
 static int tsc_disable __initdata = 0;
+#endif /*CONFIG_TSC_DISABLE*/
 
 static int __init tsc_setup(char *str)
 {

@@ -2733,7 +2737,7 @@
 	 */
 
 	/* TSC disabled? */
-#ifndef CONFIG_X86_TSC
+#if !defined(CONFIG_X86_TSC)||defined(CONFIG_TSC_DISABLE)
 	if ( tsc_disable )
 		clear_bit(X86_FEATURE_TSC, &c->x86_capability);
 #endif

@@ -2978,7 +2982,7 @@
 
 	if (cpu_has_vme || cpu_has_tsc || cpu_has_de)
 		clear_in_cr4(X86_CR4_VME|X86_CR4_PVI|X86_CR4_TSD|X86_CR4_DE);
-#ifndef CONFIG_X86_TSC
+#if !defined(CONFIG_X86_TSC)||defined(CONFIG_TSC_DISABLE)
 	if (tsc_disable && cpu_has_tsc) {
 		printk(KERN_NOTICE "Disabling TSC...\n");
 		/**** FIX-HPA: DOES THIS REALLY BELONG HERE? ****/

diff -Nru a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c
@@ -118,7 +118,7 @@
 
 extern spinlock_t i8259A_lock;
 
-#ifndef CONFIG_X86_TSC
+#if !defined(CONFIG_X86_TSC)||defined(CONFIG_TSC_DISABLE)
 
 /* This function must be called with interrupts disabled 
  * It was inspired by Steve McCanne's microtime-i386 for BSD.  -- jrs

diff -Nru a/include/asm-i386/bugs.h b/include/asm-i386/bugs.h
@@ -173,7 +173,7 @@
 /*
  * If we configured ourselves for a TSC, we'd better have one!
  */
-#ifdef CONFIG_X86_TSC
+#if defined(CONFIG_X86_TSC)&&!defined(CONFIG_TSC_DISABLE)
 	if (!cpu_has_tsc)
 		panic("Kernel compiled for Pentium+, requires TSC feature!");
 #endif

diff -Nru a/include/asm-i386/timex.h b/include/asm-i386/timex.h
@@ -40,7 +40,7 @@
 
 static inline cycles_t get_cycles (void)
 {
-#ifndef CONFIG_X86_TSC
+#if !defined(CONFIG_X86_TSC) || defined(CONFIG_TSC_DISABLE)
 	return 0;
 #else
 	unsigned long long ret;

diff -Nru a/include/net/pkt_sched.h b/include/net/pkt_sched.h
@@ -12,7 +12,7 @@
 #include <linux/pkt_sched.h>
 #include <net/pkt_cls.h>
 
-#ifdef CONFIG_X86_TSC
+#if defined(CONFIG_X86_TSC)&&!defined(CONFIG_TSC_DISABLE)
 #include <asm/msr.h>
 #endif
 

@@ -253,7 +253,7 @@
 
 #define PSCHED_US2JIFFIE(delay) (((delay)+psched_clock_per_hz-1)/psched_clock_per_hz)
 
-#ifdef CONFIG_X86_TSC
+#if defined(CONFIG_X86_TSC)&&!defined(CONFIG_TSC_DISABLE)
 
 #define PSCHED_GET_TIME(stamp) \
 ({ u64 __cur; \

diff -Nru a/include/net/profile.h b/include/net/profile.h
@@ -9,7 +9,7 @@
 #include <linux/kernel.h>
 #include <asm/system.h>
 
-#ifdef CONFIG_X86_TSC
+#if defined(CONFIG_X86_TSC) && !defined(CONFIG_TSC_DISABLE)
 #include <asm/msr.h>
 #endif
 

@@ -29,7 +29,7 @@
 extern struct timeval net_profile_adjust;
 extern void net_profile_irq_adjust(struct timeval *entered, struct timeval* leaved);
 
-#ifdef CONFIG_X86_TSC
+#if defined(CONFIG_X86_TSC)&&!defined(CONFIG_TSC_DISABLE)
 
 static inline void  net_profile_stamp(struct timeval *pstamp)
 {

This seems to be yet another reason for supporting per-CPU TSC calibration, as that would fix machines with different speed cpus, too.

-ben

"You will be reincarnated as a toad; and you will be much happier."

Hi Ben,

I agree.
Maybe the patch I once made to support CPUs with different speeds can serve as a starting point?

http://www.uwsg.iu.edu/hypermail/linux/kernel/0203.1/0481.html

However, one would need to make sure that all CPUs occasionally do receive timer interrupts, otherwise your TSC may overflow. Depending on your hardware (APICs), this might be an issue. I've been told that Fosters do misbehave ...

Regards,

Kurt Garloff  <[email protected]>                          Eindhoven, NL
GPG key: See mail header, key servers         Linux kernel development
SuSE Linux AG, Nuernberg, DE                            SCSI, Security

Hmmm, I've skimmed your patch before, but really only just for the lowest common features bit. I didn't quite grasp it last time, but I really like what you're doing there. TSC overflow can be almost eliminated if we use the full 64bits, but if we can round robin the interrupts, that would help compensate for any skew in the clocks.

Thanks for the pointer!
-john

-#ifndef CONFIG_X86_TSC
+#if !defined(CONFIG_X86_TSC)||defined(CONFIG_TSC_DISABLE) +#ifdef CONFIG_TSC_DISABLE
+static int tsc_disable __initdata = 1;
+#else /*CONFIG_TSC_DISABLE*/
static int tsc_disable __initdata = 0;
+#endif /*CONFIG_TSC_DISABLE*/

This looks *really horrible*
Why not just unset CONFIG_X86_TSC for those machines ?

Dave

| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

True, I agree here.

I was actually hoping for a suggestion like this when I posted this patch earlier. Can one really just unset CONFIG_ options that have previously been set? I'd actually prefer this, but doing so generated a .config that looked like:

CONFIG_X86_TSC=y
...
# CONFIG_X86_TSC is not set

So I assumed CONFIG_X86_TSC would still hold. Am I wrong, or is there another way to do this?

Thanks
-john

Ugh, I hadn't realised the .config generation was so primitive. That's quite unfortunate. That needs fixing at some point.

Dave

| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

I suppose you could it rewrite like

...
CONFIG_X86_WANT_TSC=y (or whatever)
...

if [ some_condition ]; then
define_bool CONFIG_X86_TSC n
else
define_bool CONFIG_X86_TSC $CONFIG_X86_WANT_TSC
fi

Not exactly elegant, but it should work ;)

--Kai

Yep, my first release was done in a similar fashion, but Alan suggested the patch take on its current form. There may be cases where we want to know if we have a TSC even if we don't want to use them.

Thread link:
http://www.uwsg.iu.edu/hypermail/linux/kernel/0205.3/1188.html

Thanks for the suggestion,
-john

Hi!

Why so gly #ifdefs? We can disable it runtime already, you should need just *one* ifdef.
Pavel

(about SSSCA) "I don't say this lightly.  However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa