RE: [Discuss][i386] Platform SMIs and their interferance with tsc based delay calibration

[Pallipadi, Venkatesh]

>-----Original Message-----
>From: Andrew Morton [mailto:akpm@xxxxxxxx] 
>Subject: Re: [Discuss][i386] Platform SMIs and their 
>interferance with tsc based delay calibration
>
>
>Please don't send emails which contain 500-column lines?

Sorry. Something got messed up during cut and paste onto my mailer.

>>  Solution:
>>  The patch below makes the calibration routine aware of 
>asynchronous events
>> like SMIs. We increase the delay calibration time and also 
>identify any
>> significant errors (greater than 12.5%) in the calibration 
>and notify it
>> to user. Like to know your comments on this.
>
>I find calibrate_delay_tsc() quite confusing.  Are you sure that the
>variable names are correct?
>
> +	tsc_rate_max = (post_end - pre_start) / DELAY_CALIBRATION_TICKS;
> +	tsc_rate_min = (pre_end - post_start) / DELAY_CALIBRATION_TICKS;
>
>that looks strange.  I'm sure it all makes sense if one understands the
>algorithm, but it shouldn't be this hard.  Please reissue the 
>patch with
>adequate comments which describe what the code is doing.
>

I will resend the patch soon with more comments. I think the variable 
names here are bit confusing.

>Shouldn't calibrate_delay_tsc() be __devinit?  (That may 
>generate warnings
>from reference_discarded.pl, but they're false positives)
>
>
>From a maintainability POV it's not good that x86 is no longer 
>using the
>generic calibrate_delay() code.  Can you rework the code so that all
>architectures must implement arch_calibrate_delay(), then 
>provide stubs for
>all except x86?  After all, other architectures/platforms may 
>have the same
>problem.
>

Agreed. I will add a stub in other architectures. That way we don't 
have to duplicate the current delay_calibration under i386.

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