Re: Bug fix for PR 26470 for review

[Dennis Ferguson <dennis.c.ferguson%gmail.com@localhost>]

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

        timeradd(&pt->pt_time.it_value,
            &pt->pt_time.it_interval, &pt->pt_time.it_value);
        getmicrotime(&now);
        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);
        mutex_spin_exit(&timer_lock);

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