Re: CVS commit: src

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

> Module Name:    src
> Committed By:   yamt
> Date:           Tue Mar 17 08:12:54 UTC 2026
> 
> Modified Files:
>         src/sys/kern: subr_time_arith.c
>         src/tests/kernel: t_time_arith.c
> 
> Log Message:
> itimer_transition: do not keep it_value unchanged after firing the event

This appears to have broken the tests, cases 47 through 56:

https://releng.netbsd.org/b5reports/i386/2026/2026.03.17.08.20.52/test.html#kernel_t_time_arith_itimer_transitions

The change also makes several tests inconsistent with the explanation
of the reasoning justifying them:

>  	/*
>  	 * Fired more than one interval early -- treat clock as wound
>  	 * backwards, not counting overruns.  Advance to the next
>  	 * integral multiple of it_interval starting from it_value.
>  	 */
>  	[0] = {{.it_value = {3,0}, .it_interval = {1,0}},
> -	       {0,1}, {1,0}, 0,
> +	       {0,1}, {4,0}, 0,
>  	       NULL},

OK, the comment was not clear here, but what it meant was to advance
to the earliest value of it_value + k*it_interval, for some k, after
the current time {0,1} = 0.0000000001sec.  And that is {1,0} = 1sec
(with k = -2).

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.

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.)

>  	/*
>  	 * Fired exactly one interval early.  Treat this too as clock
>  	 * wound backwards.
>  	 */
>  	[7] = {{.it_value = {3,0}, .it_interval = {1,0}},
> -	       {2,0}, {3,0}, 0,
> +	       {2,0}, {4,0}, 0,
>  	       NULL},

Ditto.  This is an edge case between above and below, but the comment
should match the test case, and I think it is better for the actual
delays to aim for <2*it_interval rather than <=2*it_interval (but it's
not that big a deal either way).

>  	/*
>  	 * 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.

> -	 *            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.