Source-Changes-D archive

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

Re: CVS commit: src



On Sun, Mar 22, 2026 at 5:45 AM Taylor R Campbell <riastradh%netbsd.org@localhost> wrote:
>
> > Date: Fri, 20 Mar 2026 13:18:20 +0900
> > From: Takashi YAMAMOTO <yamt9999%gmail.com@localhost>
> >
> > On Wed, Mar 18, 2026 at 9:18 PM Taylor R Campbell <riastradh%netbsd.org@localhost> wrote:
> > >
> > > Consider what happens if we set a periodic timer that runs once per
> > > second starting from {0,0}.  The timer last fired at {59,1}, and is
> > > scheduled to fire at {60,0} next; at {59,999999999}, we wound the
> > > clock back by a minute to {0,0}, and then at {0,1} the timer fires
> > > again.
> >
> > my logic is that, because the events for {0, 0} ... {59, 0} have been fired,
> > the next one should be for {60, 0}.
> >
> > > Should there be a delay of another _minute_ before the next time this
> > > _once-per-second_ timer is scheduled to fire, at {61,0}, or should
> > > there be a delay of around a _second_ before the then, at {1,0}?
> > >
> > > I think {1,0} is better-justified (or maybe {2,0}), and {61,0} is
> > > surely wrong.
> > >
> > > (See PR kern/58925: `itimer(9) responds erratically to clock wound
> > > back' <https://gnats.NetBSD.org/58925> for what prompted the current
> > > logic.)
> >
> > IMO, the correct answer for the question in the PR is (e).
> > it doesn't make sense to fire "an interval timer to fire every minute
> > starting at 03:00:00 on the real-time clock" at 02:01:00.
>
> OK, I guess, we need to distinguish relative periodic timers and
> absolute periodic timers (TIMER_ABSTIME).
>
> Internally, the setitimer and timer_settime syscalls convert relative
> periodic timers into absolute periodic timers by adding the current
> time to it_value.  But they are supposed to behave differently, under
> POSIX semantics:
>
>         If the value of the CLOCK_REALTIME clock is set via
>         clock_settime(), the new value of the clock shall be used to
>         determine the time of expiration for absolute time services
>         based upon the CLOCK_REALTIME clock.  This applies to the time
>         at which armed absolute timers expire.  If the absolute time
>         requested at the invocation of such a time service is before
>         the new value of the clock, the time service shall expire
>         immediately as if the clock had reached the requested time
>         normally.
>
>         Setting the value of the CLOCK_REALTIME clock via
>         clock_settime() shall have no effect on threads that are
>         blocked waiting for a relative time service based upon this
>         clock, including the nanosleep() and thrd_sleep() functions;
>         nor on the expiration of relative timers based upon this
>         clock.  Consequently, these time services shall expire when
>         the requested relative interval elapses, independently of the
>         new or old value of the clock.
>
> https://pubs.opengroup.org/onlinepubs/9799919799.2024edition/functions/clock_settime.html
>
> Suppose the real-time clock now reads 02:59:00 (1970-01-01), and
> shortly after the clock reads 03:00:00 (and any timers scheduled for
> then have fired), it is wound back to 02:00:00.
>
> - If we schedule a _relative_ timer with it_value={60,0} and
>   it_interval={1,0}, then it should first fire when the clock reads
>   03:00:00, and it should next fire when the clock -- having been
>   wound back an hour -- reads 02:01:00 a minute later.
>
> - If we schedule an _absolute_ timer with it_value={180,0} and
>   it_interval={1,0}, then it should first fire when the clock reads
>   03:00:00, and it should next fire when the clock -- having been
>   wound back an hour -- reads 03:01:01, an hour and a minute later.
>
> Perhaps handling this properly requires an ito_realtime_changed
> callback to correct the current deadlines of the relative timers to
> DTRT; in that case, your change may be fine.

are you suggesting to change the behavior on TIMER_ABSTIME?
i feel it's dangerous to read too much from the standard text.
do you know any systems which implement posix timers that way?

>
> > i suppose that it's somehow expected for realtime timers to behave a
> > bit strange on a large time adjustment.
> > although there might be a way to make it nicer, your trick doesn't
> > seem so nice as it doesn't work well for the cases i mentioned in the
> > commit message.
>
> But the case you're responding to, where the timer fires _more_ than

sorry, which case?

> one interval early, does not appear in the cases you quoted in the
> commit message, does it?  Isn't that the following case below?

well, itimer_transition doesn't have much context.
it_value can be the first callout, not for the interval.
in that case, "more than one interval early" doesn't have much meaning.

>
> > > >       /*
> > > >        * Fired less than one interval early -- callouts and real-time
> > > >        * clock might not be perfectly synced, counted as zero
> > > >        * overruns.  Advance by one interval from the scheduled time.
> > > >        */
> > > >       [8] = {{.it_value = {3,0}, .it_interval = {1,0}},
> > > > -            {2,1}, {3,0}, 0,
> > > > +            {2,1}, {4,0}, 0,
> > > >              NULL},
> > >
> > > This looks like a reasonable correction of an off-by-one error on my
> > > part.
>
> Isn't this what the commit message with your test program was about?
>
> > > > -      *            now [backwards]         overruns    now [forwards]
> > > > -      *           |                      v    v    v  |
> > > > -      * |--+----+-*--x----+----+----|----+----+----+--*-x----+-->
> > > > -      *            \/               |               \/
> > > > -      *         remainder        last_val        remainder
> > > > -      *     (zero or negative)                (zero or positive)
> > > > -      *
> > > > -      * Set next_val to last_value + k*interval for some k.
> > > > [...]
> > > > +      *            overruns   now
> > > > +      *          v    v    v  |
> > > > +      *     |----+----+----+--*-x----+-->
> > > > +      *     |    |         |    |
> > > > +      * last_val |         |    next (to be computed)
> > > > +      *          |         |
> > > > +      *          |       last_tick
> > > > +      *          |
> > > > +      *          next (at this point)
> > >
> > > It is unfortunate that the diagram no longer represents the
> > > clock-wound-backwards case.
> >
> > i removed it because backwards case doesn't actually need special treatment.
>
> It may not need special treatment in the form of extra branches in the
> code (I'm not convinced one way or another at the moment) but it does
> need to be explained because this stuff is extremely not obvious and
> needs to be very carefully spelled out.
>
> I spent weeks thinking about and testing this, and while it may have
> taken me that much thinking because I'm not very smart, I would like
> to ensure the next person doesn't have to spend the same effort to
> figure it out.


Home | Main Index | Thread Index | Old Index