Re: [PATCH v3 3/3] x86/tsc_msr: Make MSR derived TSC frequency more accurate

[Hans de Goede]

Hi,

On 2/8/20 1:05 AM, Thomas Gleixner wrote:
> Hans,
>
> Hans de Goede <hdegoede@xxxxxxxxxx> writes:
> > @@ -120,11 +180,23 @@ unsigned long cpu_khz_from_msr(void)
> >   	rdmsr(MSR_FSB_FREQ, lo, hi);
> >   	index = lo & freq_desc->mask;
> >   
> > -	/* Map CPU reference clock freq ID(0-7) to CPU reference clock freq(KHz) */
> > -	freq = freq_desc->freqs[index];
> > -
> > -	/* TSC frequency = maximum resolved freq * maximum resolved bus ratio */
> > -	res = freq * ratio;
> > +	/*
> > +	 * Note this also catches cases where the index points to an unpopulated
> > +	 * part of muldiv, in that case the else will set freq and res to 0.
> > +	 */
> > +	if (freq_desc->muldiv[index].divider) {
> > +		freq = DIV_ROUND_CLOSEST(TSC_REFERENCE_KHZ *
> > +					   freq_desc->muldiv[index].multiplier,
> > +					 freq_desc->muldiv[index].divider);
> > +		/* Multiply by ratio before the divide for better accuracy */
> > +		res = DIV_ROUND_CLOSEST(TSC_REFERENCE_KHZ *
> > +					   freq_desc->muldiv[index].multiplier *
> > +					   ratio,
> > +					freq_desc->muldiv[index].divider);
>
> What about:
>
>          struct muldiv *md = &freq_desc->muldiv[index];
>
>          if (md->divider) {
> 		tscref = TSC_REFERENCE_KHZ * md->multiplier;
>          	freq = DIV_ROUND_CLOSEST(tscref, md->divider);
> 		/*
>                   * Multiplying by ratio before the division has better
>                   * accuracy than just calculating freq * ratio
>                   */
>                  res = DIV_ROUND_CLOSEST(tscref * ratio, md->divider);
>
> Hmm?

That indeed looks nicer, I've prepared (and tested) a v4 with the
suggested change, I'll send out v4 right after this email.

Regards,

Hans