Messages in this thread | | | Date | Mon, 1 Sep 2008 15:16:47 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: Regression in 2.6.27 caused by commit bfc0f59 |
| |
On Mon, 1 Sep 2008, Thomas Gleixner wrote: > > If the PIT interrupts are delayed by SMM code
Btw, this sentence of yours just doesn't seem to make much sense.
The thing is, the calibration code doesn't even use interrupts. It just reads the PIT timer value.
And the thing is, the 64-bit code really does things that the 32-bit code does _not_ do.
Just as an example, the old 32-bit code (the thing that was deleted) tried to actually round things correctly, while the 64-bit code does not. Look at what the 64-bit code does:
outb((inb(0x61) & ~0x02) | 0x01, 0x61);
outb(0xb0, 0x43); outb((CLOCK_TICK_RATE / (1000 / 50)) & 0xff, 0x42); outb((CLOCK_TICK_RATE / (1000 / 50)) >> 8, 0x42); tr1 = get_cycles(); while ((inb(0x61) & 0x20) == 0); tr2 = get_cycles();
and notice three things:
- no comments (can you actually see what it does?)
- no rounding of the inevitable errors of trying to wait 1/50th of a second
- not a single try to even account for the fact that there might be things going on that perturb the value.
Now, look at what the 32-bit code _used_ to do. The good code. The code that was _deleted_.
Really. I don't think you really even looked. It did:
/* run 3 times to ensure the cache is warm and to get an accurate reading */ for (i = 0; i < 3; i++) { mach_prepare_counter(); rdtscll(start); mach_countup(&count); rdtscll(end);
.. ignore bad values ..
/* * We want the minimum time of all runs in case one of them * is inaccurate due to SMI or other delay */ delta64 = min(delta64, (end - start)); }
and if you actually look at those counter things, you'll see:
#define CALIBRATE_TIME_MSEC 30 /* 30 msecs */ #define CALIBRATE_LATCH \ ((CLOCK_TICK_RATE * CALIBRATE_TIME_MSEC + 1000/2)/1000) static inline void mach_prepare_counter(void) { /* Set the Gate high, disable speaker */ outb((inb(0x61) & ~0x02) | 0x01, 0x61);
/* * Now let's take care of CTC channel 2 * * Set the Gate high, program CTC channel 2 for mode 0, * (interrupt on terminal count mode), binary count, * load 5 * LATCH count, (LSB and MSB) to begin countdown. * * Some devices need a delay here. */ outb(0xb0, 0x43); /* binary, mode 0, LSB/MSB, Ch 2 */ outb_p(CALIBRATE_LATCH & 0xff, 0x42); /* LSB of count */ outb_p(CALIBRATE_LATCH >> 8, 0x42); /* MSB of count */ }
ie look how it actually tries to round to the nearest latch value, an how it actually comments on what it is doing.
Now, which piece of code is better?
Honestly?
Have you tried the better version (for example, boot a 32-bit kernel _before_ the unification on that machine to try).
Linus
| |