Messages in this thread Patch in this message | | | Date | Sun, 14 Mar 1999 22:35:19 +0100 (CET) | From | Ingo Molnar <> | Subject | Re: [patch] recover losed timer interrupt using the TSC [Re: [patch] kstat change to see how much Linux SMP really scale well] |
| |
On Sun, 14 Mar 1999, Andrea Arcangeli wrote:
> >+ __asm__("rdtsc" : "=a" (last_tsc_low), "=d" (last_tsc_high)); > ^^^^^^^ this is a bug: it has to be declared __volatile__
it's confusing not to have it there, i've fixed this. (patch attached) [in that particular source context it makes no difference (this is why the original asm had no volatile and it didnt show up), but yep you are right]
> >+extern inline unsigned int get_8254_timer_count(void) > > I' don't like this because I want to do other useful things between the > two inb_p. If you look at my patches you'll notice I am doing things in > the middle.
some of my old code in time.c was doing something like that, and you'll notice that my patch changes it back. I found it to be a wrong assumption to 'hand-schedule' IO (especially _p IO) with whatever other instructions, the CPU will not do anything better. I thought there might be some instruction overlap, but there isnt. I'm not sure what things you are 'doing in the middle', maybe you have a different goal. Having one function to access the hw makes the code somewhat clearer. When we touch the timer chip we already lose 5 usecs latency anyway.
-- mingo
--- linux/arch/i386/kernel/time.c.orig3 Tue Mar 9 11:03:40 1999 +++ linux/arch/i386/kernel/time.c Sun Mar 14 22:31:33 1999 @@ -66,7 +66,8 @@ /* Number of usecs that the last interrupt was delayed */ static int delay_at_last_interrupt; -static unsigned long last_tsc_low; /* lsb 32 bits of Time Stamp Counter */ +static unsigned long last_tsc_low, /* lsb 32 bits of Time Stamp Counter */ + last_tsc_high; /* Cached *multiplier* to convert TSC counts to microseconds. * (see the equation below). @@ -156,18 +157,10 @@ unsigned long jiffies_t; /* timer count may underflow right here */ - outb_p(0x00, 0x43); /* latch the count ASAP */ + count = get_8254_timer_count(); - count = inb_p(0x40); /* read the latched count */ - - /* - * We do this guaranteed double memory access instead of a _p - * postfix in the previous port access. Wheee, hackady hack - */ jiffies_t = jiffies; - count |= inb_p(0x40) << 8; - /* * avoiding timer inconsistencies (they are rare, but they happen)... * there are two kinds of problems that must be avoided here: @@ -431,30 +424,45 @@ if (use_tsc) { + unsigned int prev_high, prev_low; + unsigned int delta_high, delta_low; + /* - * It is important that these two operations happen almost at - * the same time. We do the RDTSC stuff first, since it's - * faster. To avoid any inconsistencies, we need interrupts - * disabled locally. + * We want to detect time inconsistencies */ + prev_high = last_tsc_high; + prev_low = last_tsc_low; /* - * Interrupts are just disabled locally since the timer irq - * has the SA_INTERRUPT flag set. -arca + * It is important that these two operations happen almost at + * the same time. We do the RDTSC stuff first, since it's + * faster. */ - - /* read Pentium cycle counter */ - __asm__("rdtsc" : "=a" (last_tsc_low) : : "edx"); - - outb_p(0x00, 0x43); /* latch the count ASAP */ - - count = inb_p(0x40); /* read the latched count */ - count |= inb(0x40) << 8; + asm volatile ("rdtsc" : "=a"(last_tsc_low),"=d"(last_tsc_high)); + count = get_8254_timer_count(); count = ((LATCH-1) - count) * TICK_SIZE; delay_at_last_interrupt = (count + LATCH/2) / LATCH; + + delta_low = last_tsc_low - prev_low; + delta_high = last_tsc_high - prev_high; + if (last_tsc_low < prev_low) + delta_high--; + + /* + * did we have a too big timer IRQ delay? The limit is + * 1.5*1/HZ. This detects big delays too, not just + * max 4G*1cycle ones, but breaks together with cpu_hz + * at the 4GHz barrier. + */ + if (delta_high || (delta_low > cpu_hz/HZ*3/2)) + printk("bug: %08x%08x cycles timer IRQ delay from %p\n", + delta_high, delta_low, (void *)regs->eip); + /* + * (we do not try to recover 'lost ticks', because lost + * ticks are a bug on a TSC-capable system.) + */ } - do_timer_interrupt(irq, NULL, regs); write_unlock(&xtime_lock); @@ -574,6 +582,7 @@ } while ((inb(0x61) & 0x20) == 0); __asm__ __volatile__("rdtsc":"=a" (endlow),"=d" (endhigh)); + last_tsc_high = endhigh; last_tsc_low = endlow; /* Error: ECTCNEVERSET */ --- linux/arch/i386/kernel/irq.h.orig3 Tue Mar 9 11:03:39 1999 +++ linux/arch/i386/kernel/irq.h Sun Mar 14 12:25:17 1999 @@ -251,4 +251,18 @@ } } +/* + * Get the timer chip's counter. + */ +extern inline unsigned int get_8254_timer_count(void) +{ + unsigned int count; + + outb_p(0x00, 0x43); /* latch the counter */ + count = inb_p(0x40); /* get counter LSB */ + count |= inb_p(0x40) << 8; /* get counter MSB */ + + return count; +} + #endif --- linux/arch/i386/kernel/smp.c.orig3 Tue Mar 9 14:54:49 1999 +++ linux/arch/i386/kernel/smp.c Sun Mar 14 10:51:16 1999 @@ -847,10 +847,9 @@ /* * Set up our APIC timer. */ + sti(); setup_APIC_clock(); - __sti(); - #ifdef CONFIG_MTRR /* Must be done before calibration delay is computed */ mtrr_init_secondary_cpu (); @@ -1831,22 +1830,6 @@ */ /* - * The timer chip is already set up at HZ interrupts per second here, - * but we do not accept timer interrupts yet. We only allow the BP - * to calibrate. - */ -static unsigned int __init get_8254_timer_count(void) -{ - unsigned int count; - - outb_p(0x00, 0x43); - count = inb_p(0x40); - count |= inb_p(0x40) << 8; - - return count; -} - -/* * This function sets up the local APIC timer, with a timeout of * 'clocks' APIC bus clock. During calibration we actually call * this function twice, once with a bogus timeout value, second @@ -1887,25 +1870,16 @@ apic_write(APIC_TMICT, clocks/APIC_DIVISOR); } +/* + * This is more exact than it looks like. Yep we have the timer IRQ's + * latency added, _but_ we have it always and it's a constant, thus + * it doesnt matter. + */ void __init wait_8254_wraparound(void) { - unsigned int curr_count, prev_count=~0; - int delta; - - curr_count = get_8254_timer_count(); - - do { - prev_count = curr_count; - curr_count = get_8254_timer_count(); - delta = curr_count-prev_count; - - /* - * This limit for delta seems arbitrary, but it isn't, it's - * slightly above the level of error a buggy Mercury/Neptune - * chipset timer can cause. - */ + unsigned long prev_jiffies = jiffies; - } while (delta<300); + while (jiffies == prev_jiffies) mb(); } /* @@ -1937,12 +1911,12 @@ */ setup_APIC_timer(1000000000); + sti(); /* * The timer chip counts down to zero. Let's wait * for a wraparound to start exact measurement: * (the current tick might have been already half done) */ - wait_8254_wraparound (); /* @@ -1994,13 +1968,8 @@ void __init setup_APIC_clock(void) { - unsigned long flags; - static volatile int calibration_lock; - __save_flags(flags); - __cli(); - SMP_PRINTK(("setup_APIC_clock() called.\n")); /* @@ -2028,16 +1997,7 @@ /* * Now set up the timer for real. */ - setup_APIC_timer (calibration_result); - - /* - * We ACK the APIC, just in case there is something pending. - */ - - ack_APIC_irq (); - - __restore_flags(flags); } /*
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |