Source-Changes-D archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: CVS commit: src
> 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.
> 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
one interval early, does not appear in the cases you quoted in the
commit message, does it? Isn't that the following case below?
> > > /*
> > > * 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