Date:	Wed, 10 Jun 2015 11:25:54 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	George Spelvin <linux@...izon.com>
Cc:	a.p.zijlstra@...llo.nl, adrian.hunter@...el.com,
	ak@...ux.intel.com, akpm@...ux-foundation.org, arjan@...radead.org,
	bp@...en8.de, hpa@...or.com, linux-kernel@...r.kernel.org,
	luto@...capital.net, penberg@....fi, tglx@...utronix.de,
	torvalds@...ux-foundation.org
Subject: Re: Discussion: quick_pit_calibrate is slow


* George Spelvin <linux@...izon.com> wrote:

> > Could you please structure it the following way:
> >
> > - first a patch that fixes bogus comments about the current code. It has 
> >   bitrotten and if we change it significantly we better have a well
> >   documented starting point that is easier to compare against.

Btw., we should make the patch fixing Adrian's system patch 0, as it looks simple 
and obvious enough, agreed?

> > - then a patch that introduces your more accurate calibration method and
> >   uses it as the first method to calibrate. If it fails (and it should have a 
> >   notion of failing) then it should fall back to the other two methods.
> >
> > - possibly add a boot option to skip your new calibration method -
> >   i.e. to make the kernel behave in the old way. This would be useful
> >   for tracking down any regressions in this.
> >
> >  - then maybe add a patch for the RTC method, but as a .config driven opt-in 
> >    initially.
> 
> Sonds good, but when do we get to the decruftification?  I'd prefer to prepare 
> the final patch (if nothing else, so Linus will be reassured by the diffstat), 
> although I can see holding it back for a few releases.

what do you call 'decruftification'? So I'd suggest besides obvious bug (and 
comment) fixes we should not change the current calibration code but add your new 
logic as the first step, and preserve those other methods, because we know that it 
works reasonably well on a wide range of hardware.

Because it all has to be kept in perspective: the benefits of any changes here are 
a small boot speedup plus more stable calibration at best (and a warm fuzzy 
feeling from having nicely structured, well working code), while the drawbacks are 
the potential for totally miscalibrated systems that were working fine before.

Once your bits work fine will there be any need for any of the two other PIT based 
calibration methods? We can simply remove them at a suitable point in time and 
have a single (and by definition non-crufty) piece of PIT calibration code.

(and RTC if that can be managed.)

> > Please also add calibration tracing code (.config driven and default-off), so 
> > that the statistical properties of calibration can be debugged and validated 
> > without patching the kernel.
> 
> Definitely desired, but I have to be careful here.  Obviously I can't print 
> during the timing loop, so it will take either a lot of memory, or add 
> significant computation to the loop.

So in theory you can use a tracepoint and enable boot tracing. Not sure it's 
initialized that early in the bootup, has to be explored ...

> I also don't want to flood the kernel log before syslog is started.

By default it should not print any trace.

So I don't think the details really matter: this is a boot and .config option for 
debugging, not for production kernels. You can do a big memory array and printk 
the result or use the generic tracing facilities if they are available. People can 
increase the kmsg buffer if it does not fit.

> >> Any suggestions for a reasonable time/quality tradeoff?  500 ppm ASAP?
> >> Best I can do in 10 ms?  Wait until the PIT is 500 ppm and then use
> >> the better result from a higher-resolution timer if available?
> 
> > So I'd suggest a minimum polling interval (at least 1 msecs?) plus a
> > ppm target.  Would 100ppm be too aggressive?
> 
> How about 122 ppm (1/8192) because I'm lazy? :-)

;-)

> What I imagine is this:
> 
> - The code will loop until it reaches 122 ppm or 55 ms, whichever comes
>   first.  (There's also a minimum, before which 122 ppm isn't checked.)
> - Initially, failure to reach 122 ppm will print a message and fall back.
> - In the final cleanup patch, I'll accept anything up to 500 ppm
>   and only fail (and disable TSC) if I can't reach that.

Sounds good to me in principle.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists