lkml.org 
[lkml]   [2012]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH] x86, tsc: Skip refined tsc calibration on systems with reliable TSC.
From
Date
Hi John,

On Tue, 2012-03-06 at 17:32 -0800, john stultz wrote:
> On Tue, 2012-02-21 at 18:19 -0800, Alok Kataria wrote:
> > [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.
>
> So this looks ok to me, only one nit below...
>
> >
> > Index: linux-2.6/arch/x86/kernel/tsc.c
> > ===================================================================
> > --- linux-2.6.orig/arch/x86/kernel/tsc.c 2012-02-21 17:31:01.000000000 -0800
> > +++ linux-2.6/arch/x86/kernel/tsc.c 2012-02-21 17:39:05.000000000 -0800
> > @@ -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?

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.


> 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 <akataria@vmware.com>
> 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 <akataria@vmware.com>
> Cc: John Stultz <johnstul@us.ibm.com>
> Cc: Dirk Brandewie <dirk.brandewie@gmail.com>
> Cc: Alan Cox <alan@linux.intel.com>
> Cc: stable@kernel.org
> [jstultz: Reworked to simply not schedule the refining work,
> rather then scheduling the work and bombing out later]
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> arch/x86/kernel/tsc.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index a62c201..b7d4d33 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -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;
> }

Index: linux-2.6/arch/x86/kernel/tsc.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/tsc.c 2012-02-21 17:25:35.000000000 -0800
+++ linux-2.6/arch/x86/kernel/tsc.c 2012-03-06 17:56:00.000000000 -0800
@@ -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;
}



\
 
 \ /
  Last update: 2012-03-07 03:07    [W:0.057 / U:48.612 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site