Messages in this thread Patch in this message | | | Subject | Re: [PATCH] Fix TSC calibration issues | From | Alok Kataria <> | Date | Wed, 03 Sep 2008 18:14:44 -0700 |
| |
On Wed, 2008-09-03 at 02:11 -0700, Thomas Gleixner wrote: > On Tue, 2 Sep 2008, Linus Torvalds wrote: > > This is "wrongish". > > > > You really should do the > > > > tsc1 = tsc_read_refs(&pm1, hpet ? &hpet1 : NULL); > > ... > > tsc2 = tsc_read_refs(&pm2, hpet ? &hpet2 : NULL); > > > > around the whole loop, because they get more exact with more time inside, > > and they don't improve from looping around. > > True. Just kept them at the place where my debug patches had left them.
Hi Thomas,
I agree with Linus that we should move the tsc_read_refs call outside of the loop. I did those changes and ran some boot-halt tests at my end. The frequency calibration against the pmtimer/hpet was surely more fine tuned with this change, the variance that i see now in repeated reboots is very minimal.
Please have a look at the patch below.
-- x86: Fine tune TSC calibration.
From: Alok N Kataria <akataria@vmware.com>
As Linus suggested, we should be moving the tsc_read_refs outside of the loop, this gives us more accurate TSC calibration when calibrating against hpet/pmtimer, since we are now calibrating over a period of 250ms.
With SMI_THRESHOLD equals to 50000, in the worst case, tsc values read by tsc_read_refs, could be off by 50000 ticks. On a 2Ghz processor this would mean an error of 25us. The tsc frequency is calibrated over a period of 250ms with this patch, hence the worst case error would be around 100ppm down from 500ppm previously without the patch.
Signed-off-by: Alok N Kataria <akataria@vmware.com> ---
arch/x86/kernel/tsc.c | 54 ++++++++++++++++++++++++++----------------------- 1 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c index 346cae5..bc2c1a2 100644 --- a/arch/x86/kernel/tsc.c +++ b/arch/x86/kernel/tsc.c @@ -188,7 +188,7 @@ static unsigned long pit_calibrate_tsc(void) unsigned long native_calibrate_tsc(void) { u64 tsc1, tsc2, delta, pm1, pm2, hpet1, hpet2; - unsigned long tsc_pit_min = ULONG_MAX, tsc_ref_min = ULONG_MAX; + unsigned long tsc_pit_min = ULONG_MAX, tsc_ref = ULONG_MAX; unsigned long flags; int hpet = is_hpet_enabled(), i; @@ -216,31 +216,35 @@ unsigned long native_calibrate_tsc(void) * calibration delay loop as we have to wait for a certain * amount of time anyway. */ + + local_irq_save(flags); + + /* + * Read the start value and the reference count of + * hpet/pmtimer when available. Then do the PIT + * calibration iteratively, which will take at least 250ms, + * and read the end value. + */ + tsc1 = tsc_read_refs(&pm1, hpet ? &hpet1 : NULL); + for (i = 0; i < 5; i++) { unsigned long tsc_pit_khz; - /* - * Read the start value and the reference count of - * hpet/pmtimer when available. Then do the PIT - * calibration, which will take at least 50ms, and - * read the end value. - */ - local_irq_save(flags); - tsc1 = tsc_read_refs(&pm1, hpet ? &hpet1 : NULL); tsc_pit_khz = pit_calibrate_tsc(); - tsc2 = tsc_read_refs(&pm2, hpet ? &hpet2 : NULL); - local_irq_restore(flags); /* Pick the lowest PIT TSC calibration so far */ tsc_pit_min = min(tsc_pit_min, tsc_pit_khz); - /* hpet or pmtimer available ? */ - if (!hpet && !pm1 && !pm2) - continue; + } + + tsc2 = tsc_read_refs(&pm2, hpet ? &hpet2 : NULL); + local_irq_restore(flags); - /* Check, whether the sampling was disturbed by an SMI */ - if (tsc1 == ULLONG_MAX || tsc2 == ULLONG_MAX) - continue; + /* + * Check that, either HPET or PM timer is available and, + * the sampling was not disturbed by an SMI. + */ + if ((hpet || pm1) && (tsc1 != ULLONG_MAX && tsc2 != ULLONG_MAX)) { tsc2 = (tsc2 - tsc1) * 1000000LL; @@ -259,7 +263,7 @@ unsigned long native_calibrate_tsc(void) } do_div(tsc2, tsc1); - tsc_ref_min = min(tsc_ref_min, (unsigned long) tsc2); + tsc_ref = tsc2; } /* @@ -277,7 +281,7 @@ unsigned long native_calibrate_tsc(void) } /* The alternative source failed as well, disable TSC */ - if (tsc_ref_min == ULONG_MAX) { + if (tsc_ref == ULONG_MAX) { printk(KERN_WARNING "TSC: HPET/PMTIMER calibration " "failed due to SMI disturbance.\n"); return 0; @@ -287,7 +291,7 @@ unsigned long native_calibrate_tsc(void) printk(KERN_INFO "TSC: using %s reference calibration\n", hpet ? "HPET" : "PMTIMER"); - return tsc_ref_min; + return tsc_ref; } /* We don't have an alternative source, use the PIT calibration value */ @@ -297,7 +301,7 @@ unsigned long native_calibrate_tsc(void) } /* The alternative source failed, use the PIT calibration value */ - if (tsc_ref_min == ULONG_MAX) { + if (tsc_ref == ULONG_MAX) { printk(KERN_WARNING "TSC: HPET/PMTIMER calibration failed due " "to SMI disturbance. Using PIT calibration\n"); return tsc_pit_min; @@ -305,7 +309,7 @@ unsigned long native_calibrate_tsc(void) /* Check the reference deviation */ delta = ((u64) tsc_pit_min) * 100; - do_div(delta, tsc_ref_min); + do_div(delta, tsc_ref); /* * If both calibration results are inside a 5% window, the we @@ -316,13 +320,13 @@ unsigned long native_calibrate_tsc(void) printk(KERN_INFO "TSC: PIT calibration confirmed by %s.\n", hpet ? "HPET" : "PMTIMER"); printk(KERN_INFO "TSC: using %s calibration value\n", - tsc_pit_min <= tsc_ref_min ? "PIT" : + tsc_pit_min <= tsc_ref ? "PIT" : hpet ? "HPET" : "PMTIMER"); - return tsc_pit_min <= tsc_ref_min ? tsc_pit_min : tsc_ref_min; + return tsc_pit_min <= tsc_ref ? tsc_pit_min : tsc_ref; } printk(KERN_WARNING "TSC: PIT calibration deviates from %s: %lu %lu.\n", - hpet ? "HPET" : "PMTIMER", tsc_pit_min, tsc_ref_min); + hpet ? "HPET" : "PMTIMER", tsc_pit_min, tsc_ref); /* * The calibration values differ too much. In doubt, we use
| |