Re: [Lcd4linux-devel] Two new FIXMEs in timer code :)

[Michael R. <mi...@re...>]

Hi Martin,

sorry for being that late in answering your question, but I'm busy busy busy busy...

Thanks for your analysis, here are my comments:

> First, I think that CLOCK_SKEW_DETECT_TIME_IN_S should be set to a much
> higher value:
>
>     /* delay in seconds between timer events that is considered as being
>        induced by clock skew */
>     #define CLOCK_SKEW_DETECT_TIME_IN_S 1
>
> Imagine you only have timers that trigger every two seconds (think of a
> rather slow processor and/or LCD with data that rarely change).  Right
> now, the timers always trigger immediately instead of every two seconds
> because the code assumes the delay (two seconds) is being caused by
> clock skew instead of the timer.

clock skew detection was added in r1027 by michux, unfortunately I couldn'd find anything useful in my archives about 
the background. But I agree: 1 second may be too short, probably it never showed up because everybody is using timers 
shorter than one second.

A clean solution cold be to check if timestamp of next timer is later than "now + timer.delay"? (or probably "now + 
2*timer.delay") ?

I also think that the correction "set it to current" is wrong: It should be set to "now + timer+delay" or something similar.

> The other "thing" seems like a real bug to me (and is of course related
> to clock skew).  In case a clock skew has been detected, the next
> upcoming timer (and *only* that) is updated to the new time.
>
> Now imagine two timers of different intervals and a clock skew of one
> hour (think of the transitions between summer and winter time).  The
> next upcoming timer (let's say number 1) is updated and triggers on.
> But what about the second?  If I get that right, it will not trigger for
> at least a whole hour while the user awes in wonder!
>
> So I think we should update *all* timers:
>
>     for (timer = 0; timer<  nTimers; timer++)
>       Timers[timer].when = now;
>
> instead of only the next upcoming one:
>
>     Timers[next_timer].when = now;

I'm not sure about this: probably all the other timers would be corrected one by one as soon as they are processed 
(every timer should be the "next occuring timer" somethime, and then would be corrected)



regards, Michael

-- 
Michael Reinelt <mi...@re...>
http://home.pages.at/reinelt
GPG-Key 0xDF13BA50
ICQ #288386781