Re: [PATCH] Fix TSC calibration issues

[Willy Tarreau <w@1wt.eu>]

On Wed, Sep 03, 2008 at 09:53:35PM -0700, Linus Torvalds wrote:
> 
> 
> On Thu, 4 Sep 2008, Willy Tarreau wrote:
> >
> > Basically, I would do this :
> > 
> >     pit1 = readpit();
> >     while (readpit() == pit1);
> >     t1 = rdtsc(); // precise beginning of tick 0
> >     while (readpit() != pit1 - 5000);
> >     t2 = rdtsc(); // precise beginning of tick 5000
> 
> There's a few caveats here:
> 
>  - the "readpit()" has to read without actually latching the value
> 
>    latching the PIT value will stop counting.
> 
>  - and all the docs say that you have to be careful about reading the PIT 
>    without latching it because the two 8-bit accesses aren't atomic.

Ah yes you're right, I remember having been doing crappy stuff like re-reading
and checking for difference bigger than 1.

> so the above will work in practice, but there are dangers.
> 
> The best way to fix most of the dangers is probably to only care about the 
> *high* byte, so that it doesn't matter if the low byte doesn't match the 
> high byte.
> 
> So you could probably change your version to wait for 4096 cycles (a 
> change of 16 in the high byte):
> 
> 	static unsigned char read_pit_msb(void)
> 	{
> 		/* Read but throw away the LSB */
> 		inb(0x42);
> 		return inb(0x42);
> 	}
> 
> 	..
> 	/* PIT ch2: square wave, full 16-bit count */
> 	outb(0xb6, 0x43);
> 	outb(0, 0x42);
> 	outb(0, 0x42);
> 	..
> 
> 	unsigned char pit = read_pit_msb();
> 	/* Wait until the MSB changes */
> 	while (read_pit_msb() == pit1);
> 	t1 = rdtsc();
> 	while ((unsigned char) (pit - read_pit_msb()) < 9);
> 	t2 = rdtsc();
> 
> and it might work out ok without explicit latching, and without having to 
> worry about low/high bytes being out of sync.

I like this variation.

> > If someone wants to test this, I'd be interested in the number of
> > ticks required to get a good accuracy, I bet that even with a few
> > hundred ones it's already precise by a few ppm (about the precision
> > of the input clock in fact).
> 
> I actually tested a patch with a counter value of just 1024, and I got the 
> right answer. 
> 
> But if the busy loops aren't busy (due to MSI or virtualization), then all 
> those things fly out the window.

100% agreed, though the problem is already the same with any calibration code,
with more or less sensitivity.

Willy