Skip to contentSkip to conversation

Use 64bit timer values while calibrating BogoMips

PATCH In review Andi Kleen 2 participants 3 messages Oct 31, 2007 x86

lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH 1/1 Use 64bit timer values while calibrating BogoMips

When calibrating BogoMips 32bit architectures using read_current_timer will truncate the timer value to 32bit. This is a problem on x86 when the TSC timer is already beyond 2^32-1 or wraps during the calibration. The current code has no wrap handling.

This could potentially lead to wrong BogoMips, causing incorrect delays e.g. in hardware drivers and then difficult to track down bugs.

Change read_current_timer() to always return a 64bit value even on 32bit and do the computation in 64bit.

Only found by code inspection; i haven't seen a failure in the wild.

avr32/sparc64 read_current_timer() updated, but I haven't compiled/tested them.

Signed-off-by Andi Kleen <[email protected]>
arch/avr32/lib/delay.c +1 -1
Index: linux-2.6.24-rc1-hack/arch/avr32/lib/delay.c
===================================================================
@@ -18,7 +18,7 @@
 #include <asm/processor.h>
 #include <asm/sysreg.h>
 
-int read_current_timer(unsigned long *timer_value)
+int read_current_timer(unsigned long long *timer_value)
 {
 	*timer_value = sysreg_read(COUNT);
 	return 0;
arch/x86/lib/delay_32.c +2 -2
Index: linux-2.6.24-rc1-hack/arch/x86/lib/delay_32.c
===================================================================
@@ -60,10 +60,10 @@ void use_tsc_delay(void)
 	delay_fn = delay_tsc;
 }
 
-int read_current_timer(unsigned long *timer_val)
+int read_current_timer(unsigned long long *timer_val)
 {
 	if (delay_fn == delay_tsc) {
-		rdtscl(*timer_val);
+		rdtscll(*timer_val);
 		return 0;
 	}
 	return -1;
arch/x86/lib/delay_64.c +1 -1
Index: linux-2.6.24-rc1-hack/arch/x86/lib/delay_64.c
===================================================================
@@ -18,7 +18,7 @@
 #include <asm/smp.h>
 #endif
 
-int read_current_timer(unsigned long *timer_value)
+int read_current_timer(unsigned long long *timer_value)
 {
 	rdtscll(*timer_value);
 	return 0;
include/asm-avr32/timex.h +1 -1
Index: linux-2.6.24-rc1-hack/include/asm-avr32/timex.h
===================================================================
@@ -34,7 +34,7 @@ static inline cycles_t get_cycles (void)
 	return 0;
 }
 
-extern int read_current_timer(unsigned long *timer_value);
+extern int read_current_timer(unsigned long long *timer_value);
 #define ARCH_HAS_READ_CURRENT_TIMER	1
 
 #endif /* __ASM_AVR32_TIMEX_H */
include/asm-x86/timex.h +1 -1
Index: linux-2.6.24-rc1-hack/include/asm-x86/timex.h
===================================================================
@@ -12,7 +12,7 @@
 #endif
 #define CLOCK_TICK_RATE	PIT_TICK_RATE
 
-extern int read_current_timer(unsigned long *timer_value);
+extern int read_current_timer(unsigned long long *timer_value);
 #define ARCH_HAS_READ_CURRENT_TIMER	1
 
 #endif
init/calibrate.c +2 -2
Index: linux-2.6.24-rc1-hack/init/calibrate.c
===================================================================
@@ -31,8 +31,8 @@ __setup("lpj=", lpj_setup);
 
 static unsigned long __devinit calibrate_delay_direct(void)
 {
-	unsigned long pre_start, start, post_start;
-	unsigned long pre_end, end, post_end;
+	unsigned long long pre_start, start, post_start;
+	unsigned long long pre_end, end, post_end;
 	unsigned long start_jiffies;
 	unsigned long tsc_rate_min, tsc_rate_max;
 	unsigned long good_tsc_sum = 0;
#1 lore raw
Quoted 10 lines · On Wed, 31 Oct 2007 16:52:16 +0100 Andi Kleen <[email protected]> wrote:

When calibrating BogoMips 32bit architectures using read_current_timer will truncate the timer value to 32bit. This is a problem on x86 when the TSC timer is already beyond 2^32-1 or wraps during the calibration. The current code has no wrap handling.

This could potentially lead to wrong BogoMips, causing incorrect delays e.g. in hardware drivers and then difficult to track down bugs.

bogomips tends to not be used for that nowadays

Quoted 9 lines

Change read_current_timer() to always return a 64bit value even on 32bit and do the computation in 64bit.

Only found by code inspection; i haven't seen a failure in the wild.

avr32/sparc64 read_current_timer() updated, but I haven't compiled/tested them.

one general comment, if you really want 64 bit... you're using "unsigned long long" all over, just use u64 instead.

Signature · 3 lines
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
author
#2 lore raw
Quoted 2 lines

bogomips tends to not be used for that nowadays

udelay certainly uses loops_per_jiffy (= BogoMips), even though it can be scaled.

-Andi