* [PATCH v3] x86: tsc: make TSC calibration immune to interrupts
@ 2011-05-24 19:47 Kasper Pedersen
2011-05-25 10:51 ` Thomas Gleixner
0 siblings, 1 reply; 4+ messages in thread
From: Kasper Pedersen @ 2011-05-24 19:47 UTC (permalink / raw)
To: linux-kernel
Cc: Suresh Siddha, Peter Zijlstra, John Stultz, x86, H. Peter Anvin,
Ingo Molnar, Thomas Gleixner, Kasper Pedersen, Josh Triplett
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 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.
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: save ~170usec startup time (Stultz)
v3: also disabling irqs. (Triplett)
Signed-off-by: Kasper Pedersen <kkp2010@kasperkp.dk>
---
arch/x86/kernel/tsc.c | 39 +++++++++++++++++++++++++++------------
1 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index ffe5755..9983c03 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -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)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] x86: tsc: make TSC calibration immune to interrupts
2011-05-24 19:47 [PATCH v3] x86: tsc: make TSC calibration immune to interrupts Kasper Pedersen
@ 2011-05-25 10:51 ` Thomas Gleixner
2011-05-25 22:07 ` Kasper Pedersen
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Gleixner @ 2011-05-25 10:51 UTC (permalink / raw)
To: Kasper Pedersen
Cc: linux-kernel, Suresh Siddha, Peter Zijlstra, John Stultz, x86,
H. Peter Anvin, Ingo Molnar, Josh Triplett
> 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.
That's a good reason to avoid the whole conditional thing and just do
the best of 5 always.
> /*
> * 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;
The value is completely uninteresting when we return ULLONG_MAX.
Thanks,
tglx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] x86: tsc: make TSC calibration immune to interrupts
2011-05-25 10:51 ` Thomas Gleixner
@ 2011-05-25 22:07 ` Kasper Pedersen
2011-05-25 22:23 ` Thomas Gleixner
0 siblings, 1 reply; 4+ messages in thread
From: Kasper Pedersen @ 2011-05-25 22:07 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, Suresh Siddha, Peter Zijlstra, John Stultz, x86,
H. Peter Anvin, Ingo Molnar, Josh Triplett
On 05/25/2011 12:51 PM, Thomas Gleixner wrote:
>> The 8 additional samples costs us 28 microseconds in startup
>> time.
>
> That's a good reason to avoid the whole conditional thing and just do
> the best of 5 always.
I do not mind removing it again.
The additional 170us only happen when quick_pit_calibrate fails,
and then native_calibrate_tsc burns 30ms extra anyway.
>> +
>> + *p = tp;
>
> The value is completely uninteresting when we return ULLONG_MAX.
>
native_calibrate_tsc() depends on *p being written regardless of
whether we can get a SMI-free reading:
/* We don't have an alternative source, disable TSC */
if (!hpet && !ref1 && !ref2) {
printk("TSC: No reference (HPET/PMTIMER) available\n");
return 0;
}
this works since acpi_pm_read_early() returns 0 when pmtmr_ioport
is 0, or CONFIG_X86_PM_TIMER is not set.
Without it we would report "calibration failed" rather than
"No reference" or "Using PIT" when there is neither pmtimer nor
hpet.
/Kasper Pedersen
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] x86: tsc: make TSC calibration immune to interrupts
2011-05-25 22:07 ` Kasper Pedersen
@ 2011-05-25 22:23 ` Thomas Gleixner
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Gleixner @ 2011-05-25 22:23 UTC (permalink / raw)
To: Kasper Pedersen
Cc: linux-kernel, Suresh Siddha, Peter Zijlstra, John Stultz, x86,
H. Peter Anvin, Ingo Molnar, Josh Triplett
On Thu, 26 May 2011, Kasper Pedersen wrote:
> On 05/25/2011 12:51 PM, Thomas Gleixner wrote:
> >> The 8 additional samples costs us 28 microseconds in startup
> >> time.
> >
> > That's a good reason to avoid the whole conditional thing and just do
> > the best of 5 always.
>
> I do not mind removing it again.
> The additional 170us only happen when quick_pit_calibrate fails,
> and then native_calibrate_tsc burns 30ms extra anyway.
Yes, not worth the extra conditional.
> >> +
> >> + *p = tp;
> >
> > The value is completely uninteresting when we return ULLONG_MAX.
> >
>
> native_calibrate_tsc() depends on *p being written regardless of
> whether we can get a SMI-free reading:
>
> /* We don't have an alternative source, disable TSC */
> if (!hpet && !ref1 && !ref2) {
> printk("TSC: No reference (HPET/PMTIMER) available\n");
> return 0;
> }
>
> this works since acpi_pm_read_early() returns 0 when pmtmr_ioport
> is 0, or CONFIG_X86_PM_TIMER is not set.
>
> Without it we would report "calibration failed" rather than
> "No reference" or "Using PIT" when there is neither pmtimer nor
> hpet.
Fair enough. So a comment might be in order :) That code is fricking
complex just to figure out on which frequency that damn thing runs :(
Thanks,
tglx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-05-25 22:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-24 19:47 [PATCH v3] x86: tsc: make TSC calibration immune to interrupts Kasper Pedersen
2011-05-25 10:51 ` Thomas Gleixner
2011-05-25 22:07 ` Kasper Pedersen
2011-05-25 22:23 ` Thomas Gleixner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.