tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Bug fix for PR 26470 for review

On 24 May 2008, at 15:15 , Joerg Sonnenberger wrote:
On Sat, May 24, 2008 at 03:10:38PM -0700, Dennis Ferguson wrote:
I'd note, however, that in all normal cases the code which
is there now will only be doing a single add, and that your patch changes this into a couple of divides in all cases. It might be better to save the divides just for the case where the single add fails to advance the
time past `now' as this should seldom happen.

settimeofday and other time skipping things are not normal events.
They generally happen once after boot. There's a reason why this is not
exposed far more often :-) Just using the div/mod arithmetic is much
simpler and this is not a critical code path

I think that's what I'm pointing out.  You are adding the div/mod to
fix a case that seldom happens, but the div/mod always happens.  If
you make the code look like

            &pt->pt_time.it_interval, &pt->pt_time.it_value);
        if (timercmp(&pt->pt_time.it_value, &now, <)) {
                /* make mod/div correction here */
        callout_reset(&pt->pt_ch, hzto(&pt->pt_time.it_value),
            realtimerexpire, pt);

then you only do the mod/div in the case that seldom happens.  It is
like eating your cake and having it too.

Of course the code above also retains the behaviour of the existing
code in the case where the time-of-day is set backwards, whereas your
patch changes that too.  If you think the current behaviour when the
time is set backwards is a bug, however, then I think what you are
doing is rather poorly fixing what is fixed extremely well by making
the timers use uptime instead, since then the time the timers use won't
ever go backwards and the code above works in all cases.

Dennis Ferguson

Home | Main Index | Thread Index | Old Index