x86, tsc: Skip refined tsc calibration on systems with reliable TSC.

PATCH In review Alok Kataria 2 participants 5 messages Feb 22, 2012 – Mar 7, 2012 1 message not in the archive x86

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Alok Kataria

author

2012-02-22 02:29 UTC #1 lore raw

PATCH 1/1 x86, tsc: Skip refined tsc calibration on systems with reliable TSC.

[Oops forgot to copy LKML, now it is, sorry for the duplicates]

While running the latest Linux as guest under VMware in highly over-committed situations, we have seen cases when the refined TSC algorithm fails to get a valid tsc_start value in tsc_refine_calibration_work from multiple attempts. As a result the kernel keeps on scheduling the tsc_irqwork task for later. Subsequently after several attempts when it gets a valid start value it goes through the refined calibration and either bails out or uses the new results. Given that the kernel originally read the TSC frequency from the platform, which is the best it can get, I don't think there is much value in refining it.

So IMO, for systems which get the TSC frequency from the platform we should skip the refined tsc algorithm.

We can use the TSC_RELIABLE cpu cap flag to detect this, right now it is set only on VMware and for Moorestown Penwell both of which have there own TSC calibration methods.

Thanks,
Alok

From: Alok N Kataria <[email protected]>

For systems which get the TSC frequency directly from the platform and don't go through the native TSC calibration algorithm, we should trust those values and not try to refine those.

This patch is applicable for kernel from v2.6.38 to current mainline.

Signed-off-by Alok N Kataria <[email protected]>

Cc John Stultz <[email protected]>

Cc Dirk Brandewie <[email protected]>

Cc Alan Cox <[email protected]>

Cc [email protected]

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

Index: linux-2.6/arch/x86/kernel/tsc.c
===================================================================
@@ -874,6 +874,13 @@ static void tsc_refine_calibration_work(
 		goto out;
 
 	/*
+	 * Trust the results of the earlier calibration on systems
+	 * exporting a reliable TSC.
+	 */
+	if (boot_cpu_has(X86_FEATURE_TSC_RELIABLE))
+		goto out;
+
+	/*
 	 * Since the work is started early in boot, we may be
 	 * delayed the first time we expire. So set the workqueue
 	 * again once we know timers are working.

Alok Kataria

author

2012-02-28 19:09 UTC #2 lore raw

Ping...any comments on this one.

Thanks,
Alok

Quoted 59 lines · Alok Kataria

[Oops forgot to copy LKML, now it is, sorry for the duplicates]
While running the latest Linux as guest under VMware in highly over-committed situations, we have seen cases when the refined TSC algorithm fails to get a valid tsc_start value in tsc_refine_calibration_work from multiple attempts. As a result the kernel keeps on scheduling the tsc_irqwork task for later. Subsequently after several attempts when it gets a valid start value it goes through the refined calibration and either bails out or uses the new results. Given that the kernel originally read the TSC frequency from the platform, which is the best it can get, I don't think there is much value in refining it.
So IMO, for systems which get the TSC frequency from the platform we should skip the refined tsc algorithm.
We can use the TSC_RELIABLE cpu cap flag to detect this, right now it is set only on VMware and for Moorestown Penwell both of which have there own TSC calibration methods.
Thanks,
Alok
--
From: Alok N Kataria <[email protected]>
For systems which get the TSC frequency directly from the platform and don't go through the native TSC calibration algorithm, we should trust those values and not try to refine those.
This patch is applicable for kernel from v2.6.38 to current mainline.
Signed-off-by Alok N Kataria <[email protected]>
Cc John Stultz <[email protected]>
Cc Dirk Brandewie <[email protected]>
Cc Alan Cox <[email protected]>
Cc [email protected]
Index: linux-2.6/arch/x86/kernel/tsc.c
===================================================================
arch/x86/kernel/tsc.c +7 -0
@@ -874,6 +874,13 @@ static void tsc_refine_calibration_work(
 		goto out;
 
 	/*
+	 * Trust the results of the earlier calibration on systems
+	 * exporting a reliable TSC.
+	 */
+	if (boot_cpu_has(X86_FEATURE_TSC_RELIABLE))
+		goto out;
+
+	/*
 	 * Since the work is started early in boot, we may be
 	 * delayed the first time we expire. So set the workqueue
 	 * again once we know timers are working.

john stultz

2012-03-07 01:32 UTC #3 lore raw

In reply to Alok Kataria

Quoted 13 lines · Alok Kataria

[Oops forgot to copy LKML, now it is, sorry for the duplicates]
While running the latest Linux as guest under VMware in highly over-committed situations, we have seen cases when the refined TSC algorithm fails to get a valid tsc_start value in tsc_refine_calibration_work from multiple attempts. As a result the kernel keeps on scheduling the tsc_irqwork task for later. Subsequently after several attempts when it gets a valid start value it goes through the refined calibration and either bails out or uses the new results. Given that the kernel originally read the TSC frequency from the platform, which is the best it can get, I don't think there is much value in refining it.

Quoted 6 lines

So IMO, for systems which get the TSC frequency from the platform we should skip the refined tsc algorithm.
We can use the TSC_RELIABLE cpu cap flag to detect this, right now it is set only on VMware and for Moorestown Penwell both of which have there own TSC calibration methods.

So this looks ok to me, only one nit below...

Quoted 6 lines

Index: linux-2.6/arch/x86/kernel/tsc.c
===================================================================

arch/x86/kernel/tsc.c static void tsc_refine_calibration_work( · line 883

Quoted 10 lines

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

@@ -874,6 +874,13 @@ static void tsc_refine_calibration_work(
 		goto out;
 
 	/*
+	 * Trust the results of the earlier calibration on systems
+	 * exporting a reliable TSC.
+	 */
+	if (boot_cpu_has(X86_FEATURE_TSC_RELIABLE))
+		goto out;
+
+	/*

Instead of dropping out in the function called by the work-queue, why not just avoid scheduling the work-queue to begin with?

The FEATURE_TSC_RELIABLE isn't something that is set late, and needs the delay, right?

Here's what I queued up, let me know if it looks ok to you and I'll push it on to Thomas.

thanks
-john

Quoted 1 line

From 50cd62f326fa3204763717c9808bdc29ba10512c Mon Sep 17 00:00:00 2001

From: Alok Kataria <[email protected]>
Date: Tue, 21 Feb 2012 18:19:55 -0800
Subject: [PATCH] x86, tsc: Skip refined tsc calibration on systems with reliable TSC.

So for systems which get the TSC frequency from the platform we should skip the refined tsc algorithm.

We can use the TSC_RELIABLE cpu cap flag to detect this, right now it is set only on VMware and for Moorestown Penwell both of which have there own TSC calibration methods.

Signed-off-by Alok N Kataria <[email protected]>

Cc John Stultz <[email protected]>

Cc Dirk Brandewie <[email protected]>

Cc Alan Cox <[email protected]>

Cc [email protected]

[jstultz: Reworked to simply not schedule the refining work, rather then scheduling the work and bombing out later]

Signed-off-by John Stultz <[email protected]>

---
 arch/x86/kernel/tsc.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

arch/x86/kernel/tsc.c +8 -0

index a62c201..b7d4d33 100644
@@ -932,6 +932,14 @@ static int __init init_tsc_clocksource(void)
 		clocksource_tsc.rating = 0;
 		clocksource_tsc.flags &= ~CLOCK_SOURCE_IS_CONTINUOUS;
 	}
+
+	/*
+	 * Trust the results of the earlier calibration on systems
+	 * exporting a reliable TSC.
+	 */
+	if (boot_cpu_has(X86_FEATURE_TSC_RELIABLE))
+		return 0;
+
 	schedule_delayed_work(&tsc_irqwork, 0);
 	return 0;
 }

Signature · 1 line

1.7.3.2.146.gca209

Alok Kataria

author

2012-03-07 02:05 UTC #4 lore raw

Hi John,

Quoted 43 lines · john stultz

Quoted 13 lines · Alok Kataria
[Oops forgot to copy LKML, now it is, sorry for the duplicates]
While running the latest Linux as guest under VMware in highly over-committed situations, we have seen cases when the refined TSC algorithm fails to get a valid tsc_start value in tsc_refine_calibration_work from multiple attempts. As a result the kernel keeps on scheduling the tsc_irqwork task for later. Subsequently after several attempts when it gets a valid start value it goes through the refined calibration and either bails out or uses the new results. Given that the kernel originally read the TSC frequency from the platform, which is the best it can get, I don't think there is much value in refining it.
Quoted 6 lines
So IMO, for systems which get the TSC frequency from the platform we should skip the refined tsc algorithm.
We can use the TSC_RELIABLE cpu cap flag to detect this, right now it is set only on VMware and for Moorestown Penwell both of which have there own TSC calibration methods.
So this looks ok to me, only one nit below...
Quoted 6 lines
Index: linux-2.6/arch/x86/kernel/tsc.c
===================================================================
arch/x86/kernel/tsc.c static void tsc_refine_calibration_work( · line 883
Quoted 10 lines
arch/x86/kernel/tsc.c +7 -0
@@ -874,6 +874,13 @@ static void tsc_refine_calibration_work(
 		goto out;
 
 	/*
+	 * Trust the results of the earlier calibration on systems
+	 * exporting a reliable TSC.
+	 */
+	if (boot_cpu_has(X86_FEATURE_TSC_RELIABLE))
+		goto out;
+
+	/*
Instead of dropping out in the function called by the work-queue, why not just avoid scheduling the work-queue to begin with?

Quoted 2 lines

The FEATURE_TSC_RELIABLE isn't something that is set late, and needs the delay, right?

Right, but the reason I did it as part of work-queue was because on the "out" path it still registered the tsc clocksource for us, the patch you suggested doesn't do that. Please find below a patch on similar lines, which registers the clocksource on RELIABLE_TSC systems, instead of relying on irq_work to do that.

Quoted 50 lines

Here's what I queued up, let me know if it looks ok to you and I'll push it on to Thomas.
thanks
-john
From 50cd62f326fa3204763717c9808bdc29ba10512c Mon Sep 17 00:00:00 2001
From: Alok Kataria <[email protected]>
Date: Tue, 21 Feb 2012 18:19:55 -0800
Subject: [PATCH] x86, tsc: Skip refined tsc calibration on systems with reliable TSC.
While running the latest Linux as guest under VMware in highly over-committed situations, we have seen cases when the refined TSC algorithm fails to get a valid tsc_start value in tsc_refine_calibration_work from multiple attempts. As a result the kernel keeps on scheduling the tsc_irqwork task for later. Subsequently after several attempts when it gets a valid start value it goes through the refined calibration and either bails out or uses the new results. Given that the kernel originally read the TSC frequency from the platform, which is the best it can get, I don't think there is much value in refining it.
So for systems which get the TSC frequency from the platform we should skip the refined tsc algorithm.
We can use the TSC_RELIABLE cpu cap flag to detect this, right now it is set only on VMware and for Moorestown Penwell both of which have there own TSC calibration methods.
Signed-off-by Alok N Kataria <[email protected]>
Cc John Stultz <[email protected]>
Cc Dirk Brandewie <[email protected]>
Cc Alan Cox <[email protected]>
Cc [email protected]
[jstultz: Reworked to simply not schedule the refining work, rather then scheduling the work and bombing out later]
Signed-off-by John Stultz <[email protected]>
---
 arch/x86/kernel/tsc.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
arch/x86/kernel/tsc.c +1 -0
index a62c201..b7d4d33 100644
@@ -932,6 +932,14 @@ static int __init init_tsc_clocksource(void)
 		clocksource_tsc.rating = 0;
 		clocksource_tsc.flags &= ~CLOCK_SOURCE_IS_CONTINUOUS;
 	}
+

arch/x86/kernel/tsc.c static int __init init_tsc_clocksource(void) · line 945

Quoted 10 lines

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

+	/*
+	 * Trust the results of the earlier calibration on systems
+	 * exporting a reliable TSC.
+	 */
+	if (boot_cpu_has(X86_FEATURE_TSC_RELIABLE))
+		return 0;
+
 	schedule_delayed_work(&tsc_irqwork, 0);
 	return 0;
 }

arch/x86/kernel/tsc.c +10 -0

Index: linux-2.6/arch/x86/kernel/tsc.c
===================================================================
@@ -938,6 +938,16 @@ static int __init init_tsc_clocksource(v
 		clocksource_tsc.rating = 0;
 		clocksource_tsc.flags &= ~CLOCK_SOURCE_IS_CONTINUOUS;
 	}
+
+	/*
+	 * Trust the results of the earlier calibration on systems
+	 * exporting a reliable TSC.
+	 */
+	if (boot_cpu_has(X86_FEATURE_TSC_RELIABLE)) {
+		clocksource_register_khz(&clocksource_tsc, tsc_khz);
+		return 0;
+	}
+
 	schedule_delayed_work(&tsc_irqwork, 0);
 	return 0;
 }

john stultz

2012-03-07 03:26 UTC #5 lore raw

Quoted 46 lines · Alok Kataria

Hi John,
Quoted 43 lines · john stultz
Quoted 13 lines · Alok Kataria
[Oops forgot to copy LKML, now it is, sorry for the duplicates]
While running the latest Linux as guest under VMware in highly over-committed situations, we have seen cases when the refined TSC algorithm fails to get a valid tsc_start value in tsc_refine_calibration_work from multiple attempts. As a result the kernel keeps on scheduling the tsc_irqwork task for later. Subsequently after several attempts when it gets a valid start value it goes through the refined calibration and either bails out or uses the new results. Given that the kernel originally read the TSC frequency from the platform, which is the best it can get, I don't think there is much value in refining it.
Quoted 6 lines
So IMO, for systems which get the TSC frequency from the platform we should skip the refined tsc algorithm.
We can use the TSC_RELIABLE cpu cap flag to detect this, right now it is set only on VMware and for Moorestown Penwell both of which have there own TSC calibration methods.
So this looks ok to me, only one nit below...
Quoted 6 lines
Index: linux-2.6/arch/x86/kernel/tsc.c
===================================================================
arch/x86/kernel/tsc.c static void tsc_refine_calibration_work( · line 883
Quoted 10 lines
arch/x86/kernel/tsc.c +7 -0
@@ -874,6 +874,13 @@ static void tsc_refine_calibration_work(
 		goto out;
 
 	/*
+	 * Trust the results of the earlier calibration on systems
+	 * exporting a reliable TSC.
+	 */
+	if (boot_cpu_has(X86_FEATURE_TSC_RELIABLE))
+		goto out;
+
+	/*
Instead of dropping out in the function called by the work-queue, why not just avoid scheduling the work-queue to begin with?

Quoted 8 lines

Quoted 2 lines
The FEATURE_TSC_RELIABLE isn't something that is set late, and needs the delay, right?
Right, but the reason I did it as part of work-queue was because on the "out" path it still registered the tsc clocksource for us, the patch you suggested doesn't do that. Please find below a patch on similar lines, which registers the clocksource on RELIABLE_TSC systems, instead of relying on irq_work to do that.

Ah, yes. Thanks for pointing that out!

I'll go ahead and queue your revision.

thanks
-john