[PATCH] detect_lost_tick locking fixes

From: john stultz <johnstul@us.ibm.com>

This patch fixes a race in the timer_interrupt code caused by
detect_lost_tick().  Since we're doing lost-tick compensation outside
timer->mark_offset, time can pass between time-source reads which can cause
gettimeofday inconsistencies.

Additionally detect_lost_tick() was broken for the PIT case, since the whole
point of detect_lost_tick() is to interpolate between two time sources to
find inconsistencies.  Additionally this could cause xtime_lock seq_lock
reader starvation which has been causing machine hangs for SMP boxes that use
the PIT as a time source.

This patch fixes the described race by removing detect_lost_tick() and
instead implementing the lost tick detection code inside mark_offset().

Some of the divs and mods being added here might concern folks, but by not
calling timer->get_offset() in detect_lost_tick() we eliminate much of the
same math.  I did some simple cycle counting and the new code comes out on
average equivalent or faster.

diff --git a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c
index f5e9bf60dbb010e38c84cff54b509d59a774fa28..21f7f88a5daeec1bbdc37eb05c2c96eb5c19575d 100644 (file)
--- a/arch/i386/kernel/time.c
+++ b/arch/i386/kernel/time.c
@@ -240,41 +240,6 @@ static inline void do_timer_interrupt(int irq, void *dev_id, struct pt_regs *reg
 #endif
 }
 
-/*
- * 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
@@ -291,7 +256,6 @@ void timer_interrupt(int irq, void *dev_id, struct pt_regs *regs)
         */
        write_seqlock(&xtime_lock);
 
-       detect_lost_tick();
        timer->mark_offset();
  
        do_timer_interrupt(irq, NULL, regs);

diff --git a/arch/i386/kernel/timers/timer_cyclone.c b/arch/i386/kernel/timers/timer_cyclone.c
index cbe74fd464917a704e09a0651824db033ecb366e..7cdf4d7bca6fb636d44ab4afb0da3a201ebc121c 100644 (file)
--- a/arch/i386/kernel/timers/timer_cyclone.c
+++ b/arch/i386/kernel/timers/timer_cyclone.c
@@ -18,6 +18,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 */
@@ -46,6 +47,8 @@ static rwlock_t monotonic_lock = RW_LOCK_UNLOCKED;
 
 static void mark_offset_cyclone(void)
 {
+       unsigned long lost, delay;
+       unsigned long delta = last_cyclone_low;
        int count;
        unsigned long long this_offset, last_offset;
 
@@ -62,6 +65,15 @@ static void mark_offset_cyclone(void)
        count |= inb(0x40) << 8;
        spin_unlock(&i8253_lock);
 
+       /* lost tick compensation */
+       delta = last_cyclone_low - delta;       
+       delta /=(CYCLONE_TIMER_FREQ/1000000);
+       delta += delay_at_last_interrupt;
+       lost = delta/(1000000/HZ);
+       delay = delta%(1000000/HZ);
+       if(lost >= 2)
+               jiffies += lost-1;
+       
        /* update the monotonic base value */
        this_offset = ((unsigned long long)last_cyclone_high<<32)|last_cyclone_low;
        monotonic_base += (this_offset - last_offset) & CYCLONE_TIMER_MASK;
@@ -70,6 +82,12 @@ static void mark_offset_cyclone(void)
        /* calculate delay_at_last_interrupt */
        count = ((LATCH-1) - count) * TICK_SIZE;
        delay_at_last_interrupt = (count + LATCH/2) / LATCH;
+
+       /* catch corner case where tick rollover 
+        * occured between cyclone and pit reads
+        */
+       if(abs(delay - delay_at_last_interrupt) > 900)
+               jiffies++;
 }
 
 static unsigned long get_offset_cyclone(void)

diff --git a/arch/i386/kernel/timers/timer_tsc.c b/arch/i386/kernel/timers/timer_tsc.c
index fad90c000cc6c1ef08adfed9237d2ec99429630e..e7c126d30e421d884f8cb163ea89c79616ea4733 100644 (file)
--- a/arch/i386/kernel/timers/timer_tsc.c
+++ b/arch/i386/kernel/timers/timer_tsc.c
@@ -21,6 +21,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 */
@@ -117,6 +118,8 @@ static unsigned long long monotonic_clock_tsc(void)
 
 static void mark_offset_tsc(void)
 {
+       unsigned long lost,delay;
+       unsigned long delta = last_tsc_low;
        int count;
        int countmp;
        static int count1 = 0;
@@ -161,6 +164,23 @@ static void mark_offset_tsc(void)
                }
        }
 
+       /* 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;
+       lost = delta/(1000000/HZ);
+       delay = delta%(1000000/HZ);
+       if(lost >= 2)
+               jiffies += lost-1;
+
        /* update the monotonic base value */
        this_offset = ((unsigned long long)last_tsc_high<<32)|last_tsc_low;
        monotonic_base += cycles_2_ns(this_offset - last_offset);
@@ -169,6 +189,12 @@ static void mark_offset_tsc(void)
        /* calculate delay_at_last_interrupt */
        count = ((LATCH-1) - count) * TICK_SIZE;
        delay_at_last_interrupt = (count + LATCH/2) / LATCH;
+
+       /* catch corner case where tick rollover 
+        * occured between tsc and pit reads
+        */
+       if(abs(delay - delay_at_last_interrupt) > 900)
+               jiffies++;
 }
 
 static void delay_tsc(unsigned long loops)