[flashrom] [PATCH] Fix the delay loop

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Thu Apr 1 01:55:35 CEST 2010


On 31.03.2010 18:58, Maciej Pijanka wrote:
> On Mon, 29 Mar 2010, Carl-Daniel Hailfinger wrote:
>
>   
>> The current delay loop calculation is still from revision 1 of flashrom,
>> and since then it had a logic bug which caused all delays to be twice as
>> long as intended. Fix the delay duration.
>>
>> Protect against delay loop overflows.
>>
>> Detect a non-working delay loop.
>>
>> Change the delay loop itself to ensure clever compiler optimizers won't
>> eliminate it (as happens with clang/llvm in the current code). Some
>> people suggested machine-specific asm, but the empty asm statement with
>> the loop counter as register/memory input has the benefit of being
>> perfectly cross-platform and working in gcc and clang.
>>
>> If time goes backwards (catastrophical NTP time difference, manual time
>> change), timing measurements were shot because the new-old time
>> subtraction yielded negative numbers which weren't handled correctly
>> because the variable is unsigned. Simply return 1 microsecond timing in
>> that case.
>>
>> If time goes forward too fast, pick the biggest possible timing
>> measurement with a guaranteed overflow avoidance for all timing
>> calculations.
>>     
>
> Maybe in case of leap seconds or time changing simply detect, and either fail
> and restart whole timing calculation, from scratch (possibly noting attempt
> number). If after some fixed number of retries you can't get reliable result.
> simply fail?
>   

The code does exactly what you suggested (unless I'm mistaken). The
changelog needs improvement to actually tell people about this.


>> Check four times if the calculated timing is at most 10% too fast. This
>> addresses OS scheduler interactions, e.g. being scheduled out during
>> measurement which inflates measurements.
>>     
>
> this makes sense but if we want that good results, maybe odd number of repeats,
> leave off minimum and maximum ones, then calculate over remaining (and minimum
> sane number of remaining atempts is imo 5, thus seven is needed to make sufficient
> number of iterations).
>   

Hmmm... could be done, but I'd rather tell the user that timing is
garbage than to add an elaborate algorithm to convert garbage to gold. ;-)

> Anyway, imo code is good and readable and gives more acurate results on my machines.
>
> Acked-by: Maciej Pijanka <maciej.pijanka at gmail.com>
>   

Thanks, committed in r990.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the flashrom mailing list