[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1220490884.22734.83.camel@alok-dev1>
Date: Wed, 03 Sep 2008 18:14:44 -0700
From: Alok Kataria <akataria@...are.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Larry Finger <Larry.Finger@...inger.net>,
LKML <linux-kernel@...r.kernel.org>,
"Rafael J. Wysocki" <rjw@...k.pl>, Michael Buesch <mb@...sch.de>,
Dan Hecht <dhecht@...are.com>
Subject: Re: [PATCH] Fix TSC calibration issues
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@...are.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@...are.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists