Re: [PATCH] make tsc=reliable override boot time stability checks

[john stultz <johnstul@us.ibm.com>]

On Mon, 2009-08-17 at 18:39 -0700, Alok Kataria wrote:
> Hi John,
> 
> On Mon, 2009-08-17 at 16:40 -0700, john stultz wrote:
> > On Mon, 2009-08-17 at 16:27 -0700, Stephen Hemminger wrote:
> > > On Mon, 17 Aug 2009 16:17:54 -0700
> > > john stultz <johnstul@us.ibm.com> wrote:
> > > 
> > > > On Mon, 2009-08-17 at 16:02 -0700, Stephen Hemminger wrote:
> > > > > This adds tsc, but makes it first?  it is reliable, but do I want
> > > > > to make it most important?
> > > > > 
> > > > > $ cat /sys/devices/system/clocksource/clocksource0/available_clocksource
> > > > > tsc acpi_pm
> > > > 
> > > > 
> > > > Well, if you're overriding the system saying that its safe, then sure,
> > > > its better then anything else, why wouldn't we?
> > > > 
> > > 
> > > That's acceptable, maybe add change to Documentation/kernel-parameters.txt
> > > 
> > > 	tsc=		Disable clocksource-must-verify flag for TSC.
> > > 			Format: <string>
> > > 			[x86] reliable: mark tsc clocksource as reliable and
> > >                         makes tsc the default clocksource; this
> > > 			disables clocksource verification at runtime.
> > > 			Used to enable high-resolution timer mode on older
> > > 			hardware, and in virtualized environment.
> > > 
> > 
> > Sounds good. Thanks so much for the bug report and testing!
> > 
> > 
> > This patch makes the tsc=reliable option disable the boot time stability
> > checks. Currently the option only disables the runtime watchdog checks.
> > This change allows folks who want to override the boot time TSC
> > stability checks and use the TSC when the system would otherwise
> > disqualify it.
> > 
> > There still are some situations that the TSC will be disqualified, such
> > as cpufreq scaling. But these are situations where the box will hang if
> > allowed.
> > 
> 
> I had purposefully kept the tsc=reliable separate from the TSC
> synchronous checks. 
> With this patch TSC is marked as usable though the hardware below
> doesn't export a CONSTANT_TSC, it might not be a problem generally, but
> since TSC has the highest rating, don't you think that timekeeping might
> be wayward on such systems ? 

Oh yea, there's a risk of that, but we are telling the kernel to
override its runtime checking of the clocksource, so it seems reasonable
to also include the boot time checks. I worry otherwise the option
becomes too subtle to be really useful to users. 

> Having said that, I don't think I have a particular problem with the
> patch as far as we are explicitly mentioning the fact that TSC=reliable
> means TSC is blindly trusted on this system, and time might be little
> off on some systems.

I think the explicit boot option, along with the kernel-parameters text
makes it clear enough, but if you have a specific wording in mind that
works better, please send a patch and I'll ack it.

thanks
-john