Re: [PATCH v2] x86: Reduce clock calibration time during slave cpu startup

[Ingo Molnar <mingo@elte.hu>]

* Jack Steiner <steiner@sgi.com> wrote:

> On Fri, Aug 05, 2011 at 12:46:35PM +0200, Ingo Molnar wrote:
> > 
> > * Jack Steiner <steiner@sgi.com> wrote:
> > 
> > > +/*
> > > + * Check if another cpu is in the same socket and has already been calibrated.
> > > + * If found, use the previous value. This assumes all cores in the same physical
> > > + * socket have the same core frequency.
> > > +
> > > +unsigned long __cpuinit calibrate_delay_is_known(void)
> > > +{
> > > +	int i, cpu = smp_processor_id();
> > > +
> > > +	if (!tsc_disabled && !cpu_has(&cpu_data(cpu), X86_FEATURE_CONSTANT_TSC))
> > > +		return 0;
> > > +
> > > +	for_each_online_cpu(i)
> > > +		if (cpu_data(i).phys_proc_id == cpu_data(cpu).phys_proc_id)
> > > +			return cpu_data(i).loops_per_jiffy;
> > 
> > Hm, why do we have to make such an assumption? Cannot we query the 
> > core frequency?
> 
> >> See V2 of the patch. The above assumption was removed & replaced by a check for
> >> X86_FEATURE_CONSTANT_TSC & using the TSC for __delay(). If all cores see a
> >> constant TSC frequency, then core frequency should not matter.
> >> 
> >> Does this make sense....
> 
> 
> Ahhh..  I see what you mean. I failed to update the comment in the code.
> Aside from the bogus comment (I'll send a V3 with a fixed comment), does the patch
> look ok.

Well, it still uses heuristics: it assumes frequency is the same when 
the cpuid data tells us that two CPUs are on the same socket, right?

Cannot we directly see the frequency(ies) of the CPU, and decide 
based on that whether the calibration result can be cached?

Thanks,

	Ingo