Re: [RFC patch 0/4] TSC calibration improvements

[Thomas Gleixner]

On Sat, 6 Sep 2008, Thomas Gleixner wrote:
> On Sat, 6 Sep 2008, Linus Torvalds wrote:
> > 
> > On Sat, 6 Sep 2008, Thomas Gleixner wrote:
> > > 
> > > Adding another check after the second get_cycles() makes it reliable:
> > 
> > My original patch actually had that, and Ingo added it to the -tip tree 
> > too (I think), so that should be the one we're discussing anyway. I just 
> > didn't think it would ever trigger in practice, which is why I removed it. 
> > Mea culpa.
> 
> My bad. Did not test that against -tip, just applied it from mail :)
> 
> Just checked. The -tip version still has the expect-- in the for()
> which might lead to stupid results depending on the gcc madness level.

If Alok has the second check in place and is actually worried about
that 288us impact, then we can add the following (untested), which
does not impact the speed of the check.

Against -tip x86/tsc

Thanks,

	tglx
---

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 6dab90f..3bfe083 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -310,8 +310,8 @@ static unsigned long quick_pit_calibrate(void)
 		unsigned char expect = 0xfe;
 
 		t1 = get_cycles();
-		for (i = 0; i < QUICK_PIT_ITERATIONS; i++, expect--) {
-			if (!pit_expect_msb(expect))
+		for (i = 0; i < QUICK_PIT_ITERATIONS; i++) {
+			if (!pit_expect_msb(expect--))
 				goto failed;
 		}
 		t2 = get_cycles();
@@ -319,7 +319,7 @@ static unsigned long quick_pit_calibrate(void)
 		/*
 		 * Make sure we can rely on the second TSC timestamp:
 		 */
-		if (!pit_expect_msb(--expect))
+		if (!pit_expect_msb(expect))
 			goto failed;
 
 		/*
@@ -338,7 +338,6 @@ static unsigned long quick_pit_calibrate(void)
 		 */
 		delta = (t2 - t1)*PIT_TICK_RATE;
 		do_div(delta, QUICK_PIT_ITERATIONS*256*1000);
-		printk("Fast TSC calibration using PIT\n");
 		return delta;
 	}
 failed:
@@ -356,10 +355,42 @@ unsigned long native_calibrate_tsc(void)
 	int hpet = is_hpet_enabled(), i, loopmin;
 
 	local_irq_save(flags);
+	tsc1 = tsc_read_refs(&ref1, hpet);
 	fast_calibrate = quick_pit_calibrate();
+	tsc2 = tsc_read_refs(&ref1, hpet);
 	local_irq_restore(flags);
-	if (fast_calibrate)
-		return fast_calibrate;
+
+	if (fast_calibrate) {
+		/*
+		 * Return the fast_calibrate value when neither hpet
+		 * nor pmtimer are available.
+		 */
+		if (!hpet && !ref1 && !ref2) {
+			printk("Fast TSC calibration using PIT\n");
+			return fast_calibrate;
+		}
+
+		/* Check, whether the sampling was disturbed by an SMI */
+		if (tsc1 == ULLONG_MAX || tsc2 == ULLONG_MAX)
+			goto slowpath;
+
+		tsc2 = (tsc2 - tsc1) * 1000000LL;
+		if (hpet)
+			tsc2 = calc_hpet_ref(tsc2, ref1, ref2);
+		else
+			tsc2 = calc_pmtimer_ref(tsc2, ref1, ref2);
+
+		/* Check the reference deviation */
+		delta = ((u64) fast_calibrate) * 100;
+		do_div(delta, tsc2);
+
+		if (delta >= 90 && delta <= 110) {
+			printk("Fast TSC calibration using PIT\n");
+			return fast_calibrate;
+		}
+	}
+
+slowpath:
 
 	/*
 	 * Run 5 calibration loops to get the lowest frequency value

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/