[RFC] linux-2.5.66_lost-ticks-fix_A0

[RFC] linux-2.5.66_lost-ticks-fix_A0

[john stultz]

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

All,
	I found a broken corner case with my earlier lost-ticks patch, so I'm
planning on submitting this fix to solve the issue.

The problem arises because we're doing two reads from the time source in
the timer_interrupt:
	detect_lost_tick();
	timer->mark_offset();

The problem with this being that even though we're in the interrupt
handler and properly locking our variables. The time sources keep on
ticking, so the two references to the time source are obviously not
atomic. 

For example

time	function
-------------------------
@1990	detect_lost_tick()
	- calculate # of lost ticks (none, we're < 2000)
@2003	timer->mark_offset()
	- overwrite last_tsc_low/last_cyclone_timer
	- write 3 into delay_at_last_interrupt


Thus jiffies is only incremented once, but delay_at_last_interrupt is
too small so we get a time inconsistency.

This patch resolves the issue by doing the lost-tick compensation inside
timer->mark_offset(), thus there is only one read of the time-source per
interrupt. 

Comments, flames and suggestions welcome.

thanks
-john


[-- Attachment #1.2: linux-2.5.66_lost-tick-fix_A0.patch --]
[-- Type: text/x-patch, Size: 3665 bytes --]

diff -Nru a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c
--- a/arch/i386/kernel/time.c	Tue Mar 25 12:08:26 2003
+++ b/arch/i386/kernel/time.c	Tue Mar 25 12:08:26 2003
@@ -268,41 +268,6 @@
 }
 
 /*
- * Lost tick detection and compensation
- */
-static inline void detect_lost_tick(void)
-{
-	/* read time since last interrupt */
-	unsigned long delta = timer->get_offset();
-	static unsigned long dbg_print;
-	
-	/* check if delta is greater then two ticks */
-	if(delta >= 2*(1000000/HZ)){
-
-		/*
-		 * only print debug info first 5 times
-		 */
-		/*
-		 * AKPM: disable this for now; it's nice, but irritating.
-		 */
-		if (0 && dbg_print < 5) {
-			printk(KERN_WARNING "\nWarning! Detected %lu "
-				"micro-second gap between interrupts.\n",
-				delta);
-			printk(KERN_WARNING "  Compensating for %lu lost "
-				"ticks.\n",
-				delta/(1000000/HZ)-1);
-			dump_stack();
-			dbg_print++;
-		}
-		/* calculate number of missed ticks */
-		delta = delta/(1000000/HZ)-1;
-		jiffies += delta;
-	}
-		
-}
-
-/*
  * This is the same as the above, except we _also_ save the current
  * Time Stamp Counter value at the time of the timer interrupt, so that
  * we later on can estimate the time of day more exactly.
@@ -318,7 +283,6 @@
 	 */
 	write_seqlock(&xtime_lock);
 
-	detect_lost_tick();
 	timer->mark_offset();
  
 	do_timer_interrupt(irq, NULL, regs);
diff -Nru a/arch/i386/kernel/timers/timer_cyclone.c b/arch/i386/kernel/timers/timer_cyclone.c
--- a/arch/i386/kernel/timers/timer_cyclone.c	Tue Mar 25 12:08:26 2003
+++ b/arch/i386/kernel/timers/timer_cyclone.c	Tue Mar 25 12:08:26 2003
@@ -17,6 +17,7 @@
 #include <asm/fixmap.h>
 
 extern spinlock_t i8253_lock;
+extern unsigned long jiffies;
 extern unsigned long calibrate_tsc(void);
 
 /* Number of usecs that the last interrupt was delayed */
@@ -35,6 +36,7 @@
 
 static void mark_offset_cyclone(void)
 {
+	u32 delta = last_cyclone_timer;
 	int count;
 	spin_lock(&i8253_lock);
 	/* quickly read the cyclone timer */
@@ -47,6 +49,13 @@
 	count = inb_p(0x40);    /* read the latched count */
 	count |= inb(0x40) << 8;
 	spin_unlock(&i8253_lock);
+
+	/* lost tick compensation */
+	delta = last_cyclone_timer - delta;	
+	delta /=(CYCLONE_TIMER_FREQ/1000000);
+	delta += delay_at_last_interrupt;
+	if(delta >= 2*(1000000/HZ))
+		jiffies += delta/(1000000/HZ)-1;
 
 	count = ((LATCH-1) - count) * TICK_SIZE;
 	delay_at_last_interrupt = (count + LATCH/2) / LATCH;
diff -Nru a/arch/i386/kernel/timers/timer_tsc.c b/arch/i386/kernel/timers/timer_tsc.c
--- a/arch/i386/kernel/timers/timer_tsc.c	Tue Mar 25 12:08:26 2003
+++ b/arch/i386/kernel/timers/timer_tsc.c	Tue Mar 25 12:08:26 2003
@@ -17,6 +17,7 @@
 int tsc_disable __initdata = 0;
 
 extern spinlock_t i8253_lock;
+extern unsigned long jiffies;
 
 static int use_tsc;
 /* Number of usecs that the last interrupt was delayed */
@@ -62,6 +63,7 @@
 
 static void mark_offset_tsc(void)
 {
+	unsigned long delta = last_tsc_low;
 	int count;
 	int countmp;
 	static int count1=0, count2=LATCH;
@@ -101,6 +103,21 @@
 			count2 = count1; count1 = count; count = count1;
 		}
 	}
+
+	/* lost tick compensation */
+	delta = last_tsc_low - delta;
+	{
+		register unsigned long eax, edx;
+		eax = delta;
+		__asm__("mull %2"
+		:"=a" (eax), "=d" (edx)
+		:"rm" (fast_gettimeoffset_quotient),
+		 "0" (eax));
+		delta = edx;
+	}
+	delta += delay_at_last_interrupt;
+	if(delta >= 2*(1000000/HZ))
+		jiffies += delta/(1000000/HZ)-1;
 
 	count = ((LATCH-1) - count) * TICK_SIZE;
 	delay_at_last_interrupt = (count + LATCH/2) / LATCH;

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 232 bytes --]