x86: tsc: make TSC calibration more immune to interrupts

PATCH Kasper Pedersen 3 participants 11 messages Apr 20, 2011 – Apr 23, 2011 x86

Share & tools

Kasper Pedersen

author

2011-04-20 19:41 UTC #0 lore raw

When a SMI or plain interrupt occurs during the delayed part of TSC calibration, and the SMI/irq handler is good and fast so that is does not exceed SMI_TRESHOLD, tsc_khz can be a bit off (10-30ppm).

We should not depend on interrupts being longer than 50000 clocks, so always do the 5 tries, and use the best sample we get.
This should work always for any four periodic or rate-limited interrupt sources. If we get 5 interrupts with 500ns gaps in a row, behaviour should be as without this patch.

This costs us 20-100 microseconds in startup time, as tsc_read_refs is called 8 times.

measurements:
On a 700MHz P3 I see t2-t1=~22000, and 31ppm error. A Core2 is similar: http://n1.taur.dk/tscdeviat.png
(while mostly t2-t1=~1000, in about 1 of 3000 tests I see t2-t1=~20000 for both machines.) vmware ESX4 has t2-t1=~8000 and up.

Signed-off-by Kasper Pedersen <[email protected]>

---
 arch/x86/kernel/tsc.c |   24 +++++++++++++++++-------
 1 files changed, 17 insertions(+), 7 deletions(-)

arch/x86/kernel/tsc.c +17 -7

index ffe5755..e916f99 100644
@@ -117,7 +117,7 @@ static int __init tsc_setup(char *str)
 
 __setup("tsc=", tsc_setup);
 
-#define MAX_RETRIES     5
+#define BESTOF_SAMPLES      5
 #define SMI_TRESHOLD    50000
 
 /*

@@ -125,19 +125,29 @@ __setup("tsc=", tsc_setup);
  */
 static u64 tsc_read_refs(u64 *p, int hpet)
 {
-	u64 t1, t2;
+	u64 t1, t2, tp, best_uncertainty, uncertainty, best_t2;
 	int i;
 
-	for (i = 0; i < MAX_RETRIES; i++) {
+	best_uncertainty = SMI_TRESHOLD;
+	best_t2 = 0;
+	for (i = 0; i < BESTOF_SAMPLES; i++) {
 		t1 = get_cycles();
 		if (hpet)
-			*p = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
+			tp = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
 		else
-			*p = acpi_pm_read_early();
+			tp = acpi_pm_read_early();
 		t2 = get_cycles();
-		if ((t2 - t1) < SMI_TRESHOLD)
-			return t2;
+		uncertainty = t2 - t1;
+		if (uncertainty < best_uncertainty) {
+			best_uncertainty = uncertainty;
+			best_t2 = t2;
+			*p = tp;
+		}
 	}
+	if (best_uncertainty < SMI_TRESHOLD)
+		return best_t2;
+
+	*p = tp;
 	return ULLONG_MAX;
 }

john stultz

2011-04-20 19:16 UTC #1 lore raw

Quoted 15 lines · Kasper Pedersen

When a SMI or plain interrupt occurs during the delayed part of TSC calibration, and the SMI/irq handler is good and fast so that is does not exceed SMI_TRESHOLD, tsc_khz can be a bit off (10-30ppm).
We should not depend on interrupts being longer than 50000 clocks, so always do the 5 tries, and use the best sample we get.
This should work always for any four periodic or rate-limited interrupt sources. If we get 5 interrupts with 500ns gaps in a row, behaviour should be as without this patch.
This costs us 20-100 microseconds in startup time, as tsc_read_refs is called 8 times.

Quoted 6 lines

measurements:
On a 700MHz P3 I see t2-t1=~22000, and 31ppm error. A Core2 is similar: http://n1.taur.dk/tscdeviat.png
(while mostly t2-t1=~1000, in about 1 of 3000 tests I see t2-t1=~20000 for both machines.) vmware ESX4 has t2-t1=~8000 and up.

I guess I'm curious how useful this is with the refined TSC calibration that was added not too long ago:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=08ec0c58fb8a05d3191d5cb6f5d6f81adb419798

Are you saying that you see the same 10-30ppm variance in the dmesg
line: "Refined TSC clocksource calibration: XXXX.XXX MHz" ?

thanks
-john

Kasper Pedersen

author

2011-04-20 19:46 UTC #2 lore raw

Quoted 6 lines · john stultz

Quoted 4 lines · Kasper Pedersen
When a SMI or plain interrupt occurs during the delayed part of TSC calibration, and the SMI/irq handler is good and fast so that is does not exceed SMI_TRESHOLD, tsc_khz can be a bit off (10-30ppm).

Quoted 7 lines

I guess I'm curious how useful this is with the refined TSC calibration that was added not too long ago:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=08ec0c58fb8a05d3191d5cb6f5d6f81adb419798
Are you saying that you see the same 10-30ppm variance in the dmesg
line: "Refined TSC clocksource calibration: XXXX.XXX MHz" ?

Yes, I do.
With the delayed workqueue patch, I see a wonderful 0.4ppm offset _almost_ all of the time.

Very rarely though (about 1 in 3000) the value output by "Refined TSC .." jumps. It can jump no further than 50000/F_TSC/delayed_time, so on a modern machine it jumps no further than 20ppm.

This can happen when a short irq occurs between *p = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
and
t2 = get_cycles();

Without the delayed workqueue patch 30ppm is insignificant.

/Kasper Pederen

john stultz

2011-04-20 20:28 UTC #3 lore raw

Quoted 8 lines · Kasper Pedersen

With the delayed workqueue patch, I see a wonderful 0.4ppm offset _almost_ all of the time.
Very rarely though (about 1 in 3000) the value output by "Refined TSC .." jumps. It can jump no further than 50000/F_TSC/delayed_time, so on a modern machine it jumps no further than 20ppm.

Quoted 7 lines

This can happen when a short irq occurs between *p = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
and
t2 = get_cycles();
Without the delayed workqueue patch 30ppm is insignificant.

Thanks for the additional details!

In that case, the patch seems reasonable to me.

If the additional ~200us (I suspect 100us is a little low, as acpi pm can take ~3.5us access time * 5 * 8 = 140us) is an issue, you could probably add a "good vs best" flag, which would decide between returning the first value that is under the SMI_THRESHOLD vs your method of returning the best value with the lowest uncertainty. This would allow us to only use it in the refined calibration where the smaller noise reduction makes a difference.

Ingo, Thomas, any other complaints?

thanks
-john

Kasper Pedersen

author

2011-04-20 21:23 UTC #4 lore raw

Re: x86: tsc: v2 make TSC calibration more immune to interrupts

We should not depend on interrupts being longer than 50000 clocks, so, in the refined calibration, always do the 5 tries, and use the best sample we get.

This should work always for any four periodic or rate-limited interrupt sources. If we get 5 interrupts with 500ns gaps in a row, behaviour should be as without this patch.

It is safe to use the first value that passes SMI_TRESHOLD for the initial calibration: As long as tsc_khz is above 100MHz, SMI_TRESHOLD represents less than 1% of error.

The 8 additional samples costs us 28 microseconds in startup time.

v2: John Stulz suggested limiting best uncertainty to where it is needed, saving ~170usec startup time.

Signed-off-by Kasper Pedersen <[email protected]>

---
 arch/x86/kernel/tsc.c |   36 ++++++++++++++++++++++++------------
 1 files changed, 24 insertions(+), 12 deletions(-)

arch/x86/kernel/tsc.c +24 ~~-12~~

index ffe5755..8dc813b 100644
@@ -117,27 +117,39 @@ static int __init tsc_setup(char *str)
 
 __setup("tsc=", tsc_setup);
 
-#define MAX_RETRIES     5
+#define BESTOF_SAMPLES      5
 #define SMI_TRESHOLD    50000
 
 /*
  * Read TSC and the reference counters. Take care of SMI disturbance
  */
-static u64 tsc_read_refs(u64 *p, int hpet)
+static u64 tsc_read_refs(u64 *p, int hpet, int find_best)
 {
-	u64 t1, t2;
+	u64 t1, t2, tp, best_uncertainty, uncertainty, best_t2;
 	int i;
 
-	for (i = 0; i < MAX_RETRIES; i++) {
+	best_uncertainty = SMI_TRESHOLD;
+	best_t2 = 0;
+	for (i = 0; i < BESTOF_SAMPLES; i++) {
 		t1 = get_cycles();
 		if (hpet)
-			*p = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
+			tp = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
 		else
-			*p = acpi_pm_read_early();
+			tp = acpi_pm_read_early();
 		t2 = get_cycles();
-		if ((t2 - t1) < SMI_TRESHOLD)
-			return t2;
+		uncertainty = t2 - t1;
+		if (uncertainty < best_uncertainty) {
+			best_uncertainty = uncertainty;
+			best_t2 = t2;
+			*p = tp;
+			if (!find_best)
+				break;
+		}
 	}
+	if (best_uncertainty < SMI_TRESHOLD)
+		return best_t2;
+
+	*p = tp;
 	return ULLONG_MAX;
 }

@@ -455,9 +467,9 @@ unsigned long native_calibrate_tsc(void)
 		 * read the end value.
 		 */
 		local_irq_save(flags);
-		tsc1 = tsc_read_refs(&ref1, hpet);
+		tsc1 = tsc_read_refs(&ref1, hpet, 0);
 		tsc_pit_khz = pit_calibrate_tsc(latch, ms, loopmin);
-		tsc2 = tsc_read_refs(&ref2, hpet);
+		tsc2 = tsc_read_refs(&ref2, hpet, 0);
 		local_irq_restore(flags);
 
 		/* Pick the lowest PIT TSC calibration so far */

@@ -928,11 +940,11 @@ static void tsc_refine_calibration_work(struct work_struct *work)
 		 */
 		hpet = is_hpet_enabled();
 		schedule_delayed_work(&tsc_irqwork, HZ);
-		tsc_start = tsc_read_refs(&ref_start, hpet);
+		tsc_start = tsc_read_refs(&ref_start, hpet, 1);
 		return;
 	}
 
-	tsc_stop = tsc_read_refs(&ref_stop, hpet);
+	tsc_stop = tsc_read_refs(&ref_stop, hpet, 1);
 
 	/* hpet or pmtimer available ? */
 	if (ref_start == ref_stop)

Josh Triplett

2011-04-20 22:39 UTC #5 lore raw

Re: x86: tsc: v2 make TSC calibration more immune to interrupts

Quoted 27 lines · Kasper Pedersen

When a SMI or plain interrupt occurs during the delayed part of TSC calibration, and the SMI/irq handler is good and fast so that is does not exceed SMI_TRESHOLD, tsc_khz can be a bit off (10-30ppm).
We should not depend on interrupts being longer than 50000 clocks, so, in the refined calibration, always do the 5 tries, and use the best sample we get.
This should work always for any four periodic or rate-limited interrupt sources. If we get 5 interrupts with 500ns gaps in a row, behaviour should be as without this patch.
It is safe to use the first value that passes SMI_TRESHOLD for the initial calibration: As long as tsc_khz is above 100MHz, SMI_TRESHOLD represents less than 1% of error.
The 8 additional samples costs us 28 microseconds in startup time.
measurements:
On a 700MHz P3 I see t2-t1=~22000, and 31ppm error. A Core2 is similar: http://n1.taur.dk/tscdeviat.png
(while mostly t2-t1=~1000, in about 1 of 3000 tests I see t2-t1=~20000 for both machines.) vmware ESX4 has t2-t1=~8000 and up.

Quoted 2 lines

v2: John Stulz suggested limiting best uncertainty to where it is needed, saving ~170usec startup time.

Have you considered disabling interrupts while calibrating? That would ensure that you only have to care about SMIs, not arbitrary interrupts.

Also, on more recent x86 systems you could look at MSR_SMI_COUNT (MSR 0x34) to detect if any SMIs have occurred during the sample period. rdmsr, start sample period, stop sample period, rdmsr, if delta of 0 then no SMIs occurred. Exists on Nehalem and newer, at least.

- Josh Triplett

john stultz

2011-04-21 02:19 UTC #6 lore raw

Re: x86: tsc: v2 make TSC calibration more immune to interrupts

Quoted 28 lines · Josh Triplett

Quoted 27 lines · Kasper Pedersen
When a SMI or plain interrupt occurs during the delayed part of TSC calibration, and the SMI/irq handler is good and fast so that is does not exceed SMI_TRESHOLD, tsc_khz can be a bit off (10-30ppm).
We should not depend on interrupts being longer than 50000 clocks, so, in the refined calibration, always do the 5 tries, and use the best sample we get.
This should work always for any four periodic or rate-limited interrupt sources. If we get 5 interrupts with 500ns gaps in a row, behaviour should be as without this patch.
It is safe to use the first value that passes SMI_TRESHOLD for the initial calibration: As long as tsc_khz is above 100MHz, SMI_TRESHOLD represents less than 1% of error.
The 8 additional samples costs us 28 microseconds in startup time.
measurements:
On a 700MHz P3 I see t2-t1=~22000, and 31ppm error. A Core2 is similar: http://n1.taur.dk/tscdeviat.png
(while mostly t2-t1=~1000, in about 1 of 3000 tests I see t2-t1=~20000 for both machines.) vmware ESX4 has t2-t1=~8000 and up.

Quoted 5 lines

Quoted 2 lines
v2: John Stulz suggested limiting best uncertainty to where it is needed, saving ~170usec startup time.
Have you considered disabling interrupts while calibrating? That would ensure that you only have to care about SMIs, not arbitrary interrupts.

This calibration is actually timer based (and runs for 1 second, allowing the system to continue booting in the meantime), so disabling irqs wouldn't work. You could just disable irqs during the tsc_getref, but that still has the possibility to get hit by SMIs, which are the real issue.

Quoted 4 lines

Also, on more recent x86 systems you could look at MSR_SMI_COUNT (MSR 0x34) to detect if any SMIs have occurred during the sample period. rdmsr, start sample period, stop sample period, rdmsr, if delta of 0 then no SMIs occurred. Exists on Nehalem and newer, at least.

That's interesting... but probably still too machine specific to be generally useful.

thanks
-john

Josh Triplett

2011-04-21 04:32 UTC #7 lore raw

Re: x86: tsc: v2 make TSC calibration more immune to interrupts

Quoted 32 lines · john stultz

Quoted 31 lines · Josh Triplett
Quoted 29 lines · Kasper Pedersen
When a SMI or plain interrupt occurs during the delayed part of TSC calibration, and the SMI/irq handler is good and fast so that is does not exceed SMI_TRESHOLD, tsc_khz can be a bit off (10-30ppm).
We should not depend on interrupts being longer than 50000 clocks, so, in the refined calibration, always do the 5 tries, and use the best sample we get.
This should work always for any four periodic or rate-limited interrupt sources. If we get 5 interrupts with 500ns gaps in a row, behaviour should be as without this patch.
It is safe to use the first value that passes SMI_TRESHOLD for the initial calibration: As long as tsc_khz is above 100MHz, SMI_TRESHOLD represents less than 1% of error.
The 8 additional samples costs us 28 microseconds in startup time.
measurements:
On a 700MHz P3 I see t2-t1=~22000, and 31ppm error. A Core2 is similar: http://n1.taur.dk/tscdeviat.png
(while mostly t2-t1=~1000, in about 1 of 3000 tests I see t2-t1=~20000 for both machines.) vmware ESX4 has t2-t1=~8000 and up.
v2: John Stulz suggested limiting best uncertainty to where it is needed, saving ~170usec startup time.

Quoted 8 lines

Quoted 2 lines
Have you considered disabling interrupts while calibrating? That would ensure that you only have to care about SMIs, not arbitrary interrupts.
This calibration is actually timer based (and runs for 1 second, allowing the system to continue booting in the meantime), so disabling irqs wouldn't work. You could just disable irqs during the tsc_getref, but that still has the possibility to get hit by SMIs, which are the real issue.

Ah, I see. But it sounds like disabling IRQs during the critical region would at least control all the sources of jitter that the kernel has control over, and if tsc_getref only lasts for a few microseconds then it has a very good chance of avoiding SMIs, as evidenced by the rarity of the original problem reported in this thread ("about 1 in 3000").

Quoted 7 lines

Quoted 4 lines
Also, on more recent x86 systems you could look at MSR_SMI_COUNT (MSR 0x34) to detect if any SMIs have occurred during the sample period. rdmsr, start sample period, stop sample period, rdmsr, if delta of 0 then no SMIs occurred. Exists on Nehalem and newer, at least.
That's interesting... but probably still too machine specific to be generally useful.

It seems like something usable as an enhancement if available: if the MSR exists, use it to detect a lack of SMIs, and if no SMIs occur then you don't need to keep sampling. If the MSR doesn't exist, then go ahead and sample a few times.

- Josh Triplett

Kasper Pedersen

author

2011-04-21 19:48 UTC #8 lore raw

Re: x86: tsc: v2 make TSC calibration more immune to interrupts

In reply to Josh Triplett

Quoted 8 lines · Josh Triplett

Have you considered disabling interrupts while calibrating? That would ensure that you only have to care about SMIs, not arbitrary interrupts.
Also, on more recent x86 systems you could look at MSR_SMI_COUNT (MSR 0x34) to detect if any SMIs have occurred during the sample period. rdmsr, start sample period, stop sample period, rdmsr, if delta of 0 then no SMIs occurred. Exists on Nehalem and newer, at least.

I have now tested this, and it is worth doing.

+		local_irq_save(flags);
		t1 = get_cycles();
		if (hpet)
			tp = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
		else
			tp = acpi_pm_read_early();
		t2 = get_cycles();

+ local_irq_restore(flags);

On P3, when no SMI occur, the sample is now always perfect. On Core2 there is still very rare variation - up to 8000 clock, maybe bus contention?
Thinking it was cache I tried keeping irqs disabled for 5M iterations, and that makes it occur _more_ often.

Atom shows the same as Core2, but even more rarely.

So, even if we have a Nehalem with SMI counter, the 5 samples will still have benefit.

/Kasper Pedersen

Signature · 1 line

This is my second patch ever, tell me when I do something wrong.

john stultz

2011-04-23 01:38 UTC #9 lore raw

Re: x86: tsc: v2 make TSC calibration more immune to interrupts

Quoted 5 lines · Kasper Pedersen

Quoted 4 lines · Josh Triplett
Have you considered disabling interrupts while calibrating? That would ensure that you only have to care about SMIs, not arbitrary interrupts.

Quoted 7 lines

Quoted 4 lines
Also, on more recent x86 systems you could look at MSR_SMI_COUNT (MSR 0x34) to detect if any SMIs have occurred during the sample period. rdmsr, start sample period, stop sample period, rdmsr, if delta of 0 then no SMIs occurred. Exists on Nehalem and newer, at least.
I have now tested this, and it is worth doing.

Cool! When you have the chance, send out your latest patch and I'll review/ack and hopefully queue it for tglx.

thanks
-john

Kasper Pedersen

author

2011-04-21 19:54 UTC #10 lore raw

x86: tsc: v3 make TSC calibration more immune to interrupts

In reply to Josh Triplett

We should keep plain interrupts out of the reading of tsc/reference, so disable interrupts there.

We should not depend on SMIs being longer than 50000 clocks, so, in the refined calibration, always do the 5 tries, and use the best sample we get.

This should work always for any four periodic or rate-limited SMI sources. If we get 5 SMIs with 500ns gaps in a row, behaviour should be as without this patch.

It is safe to use the first value that passes SMI_TRESHOLD for the initial calibration: As long as tsc_khz is above 100MHz, SMI_TRESHOLD represents less than 1% of error.

The 8 additional samples costs us 28 microseconds in startup time.

v2: John Stultz suggested limiting best uncertainty to where it is needed, saving ~170usec startup time.

v3: Josh Triplett suggested disabling irqs. This does indeed help, and the 5-sample code now only needs to handle the SMI case.

Signed-off-by Kasper Pedersen <[email protected]>

---
 arch/x86/kernel/tsc.c |   39 +++++++++++++++++++++++++++------------
 1 files changed, 27 insertions(+), 12 deletions(-)

arch/x86/kernel/tsc.c +27 ~~-12~~

index ffe5755..9983c03 100644
@@ -117,27 +117,42 @@ static int __init tsc_setup(char *str)
 
 __setup("tsc=", tsc_setup);
 
-#define MAX_RETRIES     5
+#define BESTOF_SAMPLES      5
 #define SMI_TRESHOLD    50000
 
 /*
  * Read TSC and the reference counters. Take care of SMI disturbance
  */
-static u64 tsc_read_refs(u64 *p, int hpet)
+static u64 tsc_read_refs(u64 *p, int hpet, int find_best)
 {
-	u64 t1, t2;
+	u64 t1, t2, tp, best_uncertainty, uncertainty, best_t2;
 	int i;
+	unsigned long flags;
 
-	for (i = 0; i < MAX_RETRIES; i++) {
+	best_uncertainty = SMI_TRESHOLD;
+	best_t2 = 0;
+	for (i = 0; i < BESTOF_SAMPLES; i++) {
+		local_irq_save(flags);
 		t1 = get_cycles();
 		if (hpet)
-			*p = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
+			tp = hpet_readl(HPET_COUNTER) & 0xFFFFFFFF;
 		else
-			*p = acpi_pm_read_early();
+			tp = acpi_pm_read_early();
 		t2 = get_cycles();
-		if ((t2 - t1) < SMI_TRESHOLD)
-			return t2;
+		local_irq_restore(flags);
+		uncertainty = t2 - t1;
+		if (uncertainty < best_uncertainty) {
+			best_uncertainty = uncertainty;
+			best_t2 = t2;
+			*p = tp;
+			if (!find_best)
+				break;
+		}
 	}
+	if (best_uncertainty < SMI_TRESHOLD)
+		return best_t2;
+
+	*p = tp;
 	return ULLONG_MAX;
 }

@@ -455,9 +470,9 @@ unsigned long native_calibrate_tsc(void)
 		 * read the end value.
 		 */
 		local_irq_save(flags);
-		tsc1 = tsc_read_refs(&ref1, hpet);
+		tsc1 = tsc_read_refs(&ref1, hpet, 0);
 		tsc_pit_khz = pit_calibrate_tsc(latch, ms, loopmin);
-		tsc2 = tsc_read_refs(&ref2, hpet);
+		tsc2 = tsc_read_refs(&ref2, hpet, 0);
 		local_irq_restore(flags);
 
 		/* Pick the lowest PIT TSC calibration so far */

@@ -928,11 +943,11 @@ static void tsc_refine_calibration_work(struct work_struct *work)
 		 */
 		hpet = is_hpet_enabled();
 		schedule_delayed_work(&tsc_irqwork, HZ);
-		tsc_start = tsc_read_refs(&ref_start, hpet);
+		tsc_start = tsc_read_refs(&ref_start, hpet, 1);
 		return;
 	}
 
-	tsc_stop = tsc_read_refs(&ref_stop, hpet);
+	tsc_stop = tsc_read_refs(&ref_stop, hpet, 1);
 
 	/* hpet or pmtimer available ? */
 	if (ref_start == ref_stop)