Skip to content

Commit ad2737f

Browse files
yamtyamt
committed
itimer_transition: do not keep it_value unchanged after firing the event
this mostly fixes the very quick output from "netstat -w1" i've seen on netbsd on qemu/nvmm. the following logs are the output of https://github.com/yamt/garbage/blob/cf7b893415080b177b5104ff6e8c75be5b57dc94/c/itimer/itimer.c on the vm. w/o this change: ``` # ./a.out 1: 490375420ns frm start, 490375420ns frm prev, ov=0, int=1 2: 750174060ns frm start, 259798640ns frm prev, ov=0, int=2 3: 875501930ns frm start, 125327870ns frm prev, ov=0, int=3 4: 933287110ns frm start, 57785180ns frm prev, ov=0, int=4 5: 970379010ns frm start, 37091900ns frm prev, ov=0, int=5 6: 972449040ns frm start, 2070030ns frm prev, ov=0, int=6 7: 990374320ns frm start, 17925280ns frm prev, ov=0, int=7 8: 991798300ns frm start, 1423980ns frm prev, ov=0, int=8 9: 992333220ns frm start, 534920ns frm prev, ov=0, int=9 10: 993545880ns frm start, 1212660ns frm prev, ov=0, int=10 11: 994030150ns frm start, 484270ns frm prev, ov=0, int=11 12: 994496010ns frm start, 465860ns frm prev, ov=0, int=12 13: 996376070ns frm start, 1880060ns frm prev, ov=0, int=13 14: 996848870ns frm start, 472800ns frm prev, ov=0, int=14 15: 998169890ns frm start, 1321020ns frm prev, ov=0, int=15 16: 998632820ns frm start, 462930ns frm prev, ov=0, int=16 17: 999045990ns frm start, 413170ns frm prev, ov=0, int=17 18: 2298018910ns frm start, 1298972920ns frm prev, ov=0, int=18 19: 3377882220ns frm start, 1079863310ns frm prev, ov=0, int=19 20: 4195221330ns frm start, 817339110ns frm prev, ov=0, int=20 21: 5275385900ns frm start, 1080164570ns frm prev, ov=0, int=21 22: 6565223200ns frm start, 1289837300ns frm prev, ov=0, int=22 23: 7375210250ns frm start, 809987050ns frm prev, ov=0, int=23 24: 8555230580ns frm start, 1180020330ns frm prev, ov=0, int=24 25: 9300371900ns frm start, 745141320ns frm prev, ov=0, int=25 26: 10600372960ns frm start, 1300001060ns frm prev, ov=0, int=26 27: 11340376020ns frm start, 740003060ns frm prev, ov=0, int=27 ^C # ``` w/ this change: ``` # ./a.out 1: 605541180ns frm start, 605541180ns frm prev, ov=0, int=1 2: 1407955990ns frm start, 802414810ns frm prev, ov=0, int=2 3: 2285627110ns frm start, 877671120ns frm prev, ov=0, int=3 4: 2602194370ns frm start, 316567260ns frm prev, ov=0, int=4 5: 3932994350ns frm start, 1330799980ns frm prev, ov=0, int=5 6: 6022993810ns frm start, 2089999460ns frm prev, ov=0, int=6 7: 6942991930ns frm start, 919998120ns frm prev, ov=0, int=7 8: 8103008190ns frm start, 1160016260ns frm prev, ov=0, int=8 9: 9149924480ns frm start, 1046916290ns frm prev, ov=0, int=9 10: 9218383340ns frm start, 68458860ns frm prev, ov=0, int=10 11: 12002313290ns frm start, 2783929950ns frm prev, ov=1, int=12 12: 13682980240ns frm start, 1680666950ns frm prev, ov=1, int=13 13: 14282982100ns frm start, 600001860ns frm prev, ov=1, int=14 14: 15642962120ns frm start, 1359980020ns frm prev, ov=1, int=15 15: 16302952020ns frm start, 659989900ns frm prev, ov=1, int=16 16: 17667002460ns frm start, 1364050440ns frm prev, ov=1, int=17 17: 18276997850ns frm start, 609995390ns frm prev, ov=1, int=18 18: 19606989090ns frm start, 1329991240ns frm prev, ov=1, int=19 19: 20396986930ns frm start, 789997840ns frm prev, ov=1, int=20 20: 21596995570ns frm start, 1200008640ns frm prev, ov=1, int=21 21: 22396987640ns frm start, 799992070ns frm prev, ov=1, int=22 22: 23556982430ns frm start, 1159994790ns frm prev, ov=1, int=23 23: 24416981240ns frm start, 859998810ns frm prev, ov=1, int=24 24: 25576994000ns frm start, 1160012760ns frm prev, ov=1, int=25 25: 26356993150ns frm start, 779999150ns frm prev, ov=1, int=26 26: 27082519580ns frm start, 725526430ns frm prev, ov=1, int=27 ^C # ```
1 parent 9352934 commit ad2737f

2 files changed

Lines changed: 63 additions & 110 deletions

File tree

‎sys/kern/subr_time_arith.c‎

Lines changed: 46 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $NetBSD: subr_time_arith.c,v 1.7 2026/01/04 03:21:19 riastradh Exp $ */
1+
/* $NetBSD: subr_time_arith.c,v 1.8 2026/03/17 08:12:53 yamt Exp $ */
22

33
/*-
44
* Copyright (c) 2000, 2004, 2005, 2007, 2008, 2009, 2020
@@ -63,7 +63,7 @@
6363
*/
6464

6565
#include <sys/cdefs.h>
66-
__KERNEL_RCSID(0, "$NetBSD: subr_time_arith.c,v 1.7 2026/01/04 03:21:19 riastradh Exp $");
66+
__KERNEL_RCSID(0, "$NetBSD: subr_time_arith.c,v 1.8 2026/03/17 08:12:53 yamt Exp $");
6767

6868
#include <sys/types.h>
6969

@@ -482,8 +482,7 @@ itimer_transition(const struct itimerspec *restrict it,
482482
struct timespec *restrict next,
483483
int *restrict overrunsp)
484484
{
485-
int64_t last_val, next_val, interval, remainder, now_ns;
486-
int backwards;
485+
int64_t last_val, interval, now_ns, overruns, last_tick;
487486

488487
/*
489488
* Zero the outputs so we can test assertions in userland
@@ -500,9 +499,6 @@ itimer_transition(const struct itimerspec *restrict it,
500499
return;
501500
}
502501

503-
/* Did the clock wind backwards? */
504-
backwards = (timespeccmp(&it->it_value, now, >));
505-
506502
/* Valid value and interval guaranteed by itimerfix. */
507503
KASSERT(it->it_value.tv_sec >= 0);
508504
KASSERT(it->it_value.tv_nsec < 1000000000);
@@ -513,20 +509,31 @@ itimer_transition(const struct itimerspec *restrict it,
513509
KASSERT(it->it_interval.tv_sec >= 0);
514510
KASSERT(it->it_interval.tv_nsec >= 0);
515511

516-
/* Handle the easy case of non-overflown timers first. */
517-
if (__predict_true(!backwards)) {
518-
if (__predict_false(!timespecaddok(&it->it_value,
519-
&it->it_interval)))
520-
goto overflow;
521-
timespecadd(&it->it_value, &it->it_interval, next);
522-
if (__predict_true(timespeccmp(now, next, <)))
523-
return;
512+
/*
513+
* Handle the easy case of non-overflown timers first.
514+
*
515+
* Note: 'it_value' can be after 'now' here. maybe the callout fired
516+
* a bit earlier than we expected. or maybe the system REALTIME clock
517+
* went backward. in that case, as the caller may have already queued
518+
* the event (eg. SIGALRM) corresponding to the current value of
519+
* it_value, the best thing we can do here is to advance it by
520+
* it_interval as usual.
521+
*/
522+
if (__predict_false(!timespecaddok(&it->it_value, &it->it_interval)))
523+
goto overflow;
524+
timespecadd(&it->it_value, &it->it_interval, next);
525+
if (__predict_true(timespeccmp(now, next, <))) {
526+
/* no overruns. */
527+
return;
524528
}
525529

526530
/*
527531
* If we can't represent the input as a number of nanoseconds,
528532
* bail. This is good up to the year 2262, if we start
529533
* counting from 1970 (2^63 nanoseconds ~ 292 years).
534+
*
535+
* TODO: we can implement timespecdiv etc to overcome this
536+
* limitation. maybe it's more straightforward with bintime.
530537
*/
531538
if (__predict_false(!timespec2nsok(now)) ||
532539
__predict_false(!timespec2nsok(&it->it_value)) ||
@@ -539,83 +546,34 @@ itimer_transition(const struct itimerspec *restrict it,
539546

540547
KASSERT(now_ns >= 0);
541548
KASSERT(last_val >= 0);
542-
KASSERT(interval >= 0);
549+
KASSERT(interval > 0);
543550

544551
/*
545-
* now [backwards] overruns now [forwards]
546-
* | v v v |
547-
* |--+----+-*--x----+----+----|----+----+----+--*-x----+-->
548-
* \/ | \/
549-
* remainder last_val remainder
550-
* (zero or negative) (zero or positive)
551-
*
552-
* Set next_val to last_value + k*interval for some k.
553-
*
554-
* The interval is always positive, and division in C
555-
* truncates, so dividing a positive duration by the interval
556-
* always gives zero or a positive remainder, and dividing a
557-
* negative duration by the interval always gives zero or a
558-
* negative remainder. Hence:
559-
*
560-
* - If now_ns < last_val -- which happens iff backwards, i.e.,
561-
* the clock was wound backwards -- then remainder is zero or
562-
* negative, so subtracting it stays in place or moves
563-
* forward in time, and thus this finds the _earliest_ value
564-
* that is not earlier than now_ns. We will advance this by
565-
* one more interval if we are already firing exactly on the
566-
* interval to find the earliest value _after_ now_ns.
552+
* at this point, we know 'last_val' < 'next' <= 'now'.
553+
* we are about to compute 'overruns' and new 'next'.
567554
*
568-
* - If now_ns > last_val -- which happens iff !backwards,
569-
* i.e., the clock ran fast -- then remainder is zero or
570-
* positive positive, so this finds the _latest_ value not
571-
* later than now_ns. We will always advance this by one
572-
* more interval to find the earliest value _after_ now_ns.
573-
* We will also count overflows.
574-
*
575-
* (now_ns == last_val is not possible at this point because it
576-
* only happens if the addition of struct timespec would
577-
* overflow, and that is only possible when timespec2ns would
578-
* also overflow for at least one of the inputs.)
555+
* overruns now
556+
* v v v |
557+
* |----+----+----+--*-x----+-->
558+
* | | | |
559+
* last_val | | next (to be computed)
560+
* | |
561+
* | last_tick
562+
* |
563+
* next (at this point)
579564
*/
580-
KASSERT(last_val != now_ns);
581-
remainder = (now_ns - last_val) % interval;
582-
next_val = now_ns - remainder;
583-
KASSERT((last_val - next_val) % interval == 0);
584-
if (backwards) {
585-
/*
586-
* If the clock was wound back to an exact multiple of
587-
* the interval, so next_val = now_ns, don't demand to
588-
* fire again in the same instant -- advance to the
589-
* next interval. Overflow is not possible; proof is
590-
* asserted.
591-
*/
592-
if (remainder == 0) {
593-
KASSERT(now_ns < last_val);
594-
KASSERT(next_val == now_ns);
595-
KASSERT(last_val - next_val >= interval);
596-
KASSERT(interval <= last_val - next_val);
597-
KASSERT(next_val <= last_val - interval);
598-
KASSERT(next_val <= INT64_MAX - interval);
599-
next_val += interval;
600-
}
601-
} else {
602-
/*
603-
* next_val is the largest integer multiple of interval
604-
* not later than now_ns. Count the number of full
605-
* intervals that were skipped (division should be
606-
* exact here), not counting any partial interval
607-
* between next_val and now_ns, as the number of
608-
* overruns. Advance by one interval -- unless that
609-
* would overflow.
610-
*/
611-
*overrunsp = MIN(INT_MAX, (next_val - last_val) / interval);
612-
if (__predict_false(next_val > INT64_MAX - interval))
613-
goto overflow;
614-
next_val += interval;
615-
}
616-
617-
next->tv_sec = next_val / 1000000000;
618-
next->tv_nsec = next_val % 1000000000;
565+
KASSERT(last_val < now_ns);
566+
overruns = (now_ns - last_val) / interval;
567+
KASSERT(overruns > 0);
568+
last_tick = last_val + interval * overruns;
569+
next->tv_sec = last_tick / 1000000000;
570+
next->tv_nsec = last_tick % 1000000000;
571+
KASSERT(timespeccmp(next, now, <=));
572+
if (__predict_false(!timespecaddok(next, &it->it_interval)))
573+
goto overflow;
574+
timespecadd(next, &it->it_interval, next);
575+
KASSERT(timespeccmp(now, next, <));
576+
*overrunsp = MIN(INT_MAX, overruns);
619577
return;
620578

621579
overflow:

‎tests/kernel/t_time_arith.c‎

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $NetBSD: t_time_arith.c,v 1.8 2026/03/17 08:10:58 yamt Exp $ */
1+
/* $NetBSD: t_time_arith.c,v 1.9 2026/03/17 08:12:53 yamt Exp $ */
22

33
/*-
44
* Copyright (c) 2024-2025 The NetBSD Foundation, Inc.
@@ -27,7 +27,7 @@
2727
*/
2828

2929
#include <sys/cdefs.h>
30-
__RCSID("$NetBSD: t_time_arith.c,v 1.8 2026/03/17 08:10:58 yamt Exp $");
30+
__RCSID("$NetBSD: t_time_arith.c,v 1.9 2026/03/17 08:12:53 yamt Exp $");
3131

3232
#include <sys/timearith.h>
3333

@@ -89,33 +89,33 @@ const struct itimer_transition {
8989
* integral multiple of it_interval starting from it_value.
9090
*/
9191
[0] = {{.it_value = {3,0}, .it_interval = {1,0}},
92-
{0,1}, {1,0}, 0,
92+
{0,1}, {4,0}, 0,
9393
NULL},
9494
[1] = {{.it_value = {3,0}, .it_interval = {1,0}},
95-
{0,500000000}, {1,0}, 0,
95+
{0,500000000}, {4,0}, 0,
9696
NULL},
9797
[2] = {{.it_value = {3,0}, .it_interval = {1,0}},
98-
{0,999999999}, {1,0}, 0,
98+
{0,999999999}, {4,0}, 0,
9999
NULL},
100100
[3] = {{.it_value = {3,0}, .it_interval = {1,0}},
101-
{1,0}, {2,0}, 0,
101+
{1,0}, {4,0}, 0,
102102
NULL},
103103
[4] = {{.it_value = {3,0}, .it_interval = {1,0}},
104-
{1,1}, {2,0}, 0,
104+
{1,1}, {4,0}, 0,
105105
NULL},
106106
[5] = {{.it_value = {3,0}, .it_interval = {1,0}},
107-
{1,500000000}, {2,0}, 0,
107+
{1,500000000}, {4,0}, 0,
108108
NULL},
109109
[6] = {{.it_value = {3,0}, .it_interval = {1,0}},
110-
{1,999999999}, {2,0}, 0,
110+
{1,999999999}, {4,0}, 0,
111111
NULL},
112112

113113
/*
114114
* Fired exactly one interval early. Treat this too as clock
115115
* wound backwards.
116116
*/
117117
[7] = {{.it_value = {3,0}, .it_interval = {1,0}},
118-
{2,0}, {3,0}, 0,
118+
{2,0}, {4,0}, 0,
119119
NULL},
120120

121121
/*
@@ -124,13 +124,13 @@ const struct itimer_transition {
124124
* overruns. Advance by one interval from the scheduled time.
125125
*/
126126
[8] = {{.it_value = {3,0}, .it_interval = {1,0}},
127-
{2,1}, {3,0}, 0,
127+
{2,1}, {4,0}, 0,
128128
NULL},
129129
[9] = {{.it_value = {3,0}, .it_interval = {1,0}},
130-
{2,500000000}, {3,0}, 0,
130+
{2,500000000}, {4,0}, 0,
131131
NULL},
132132
[10] = {{.it_value = {3,0}, .it_interval = {1,0}},
133-
{2,999999999}, {3,0}, 0,
133+
{2,999999999}, {4,0}, 0,
134134
NULL},
135135

136136
/*
@@ -222,24 +222,19 @@ const struct itimer_transition {
222222
[28] = {{.it_value = {3,0}, .it_interval = {9223372036,854775809}},
223223
{3,1}, {9223372039,854775809}, 0, NULL},
224224

225-
/*
226-
* Overflows -- we should (XXX but currently don't) reject
227-
* intervals of at least 2^64 nanoseconds up front, since this
228-
* is more time than it is reasonable to wait (more than 584
229-
* years).
230-
*/
225+
/* Large intervals */
231226

232227
/* (2^64 - 1) ns */
233228
[29] = {{.it_value = {3,0}, .it_interval = {18446744073,709551615}},
234-
{2,999999999}, {0,0}, 0,
229+
{2,999999999}, {18446744076,709551615}, 0,
235230
NULL},
236231
/* 2^64 ns */
237232
[30] = {{.it_value = {3,0}, .it_interval = {18446744073,709551616}},
238-
{2,999999999}, {0,0}, 0,
233+
{2,999999999}, {18446744076,709551616}, 0,
239234
NULL},
240235
/* (2^64 + 1) ns */
241236
[31] = {{.it_value = {3,0}, .it_interval = {18446744073,709551617}},
242-
{2,999999999}, {0,0}, 0,
237+
{2,999999999}, {18446744076,709551617}, 0,
243238
NULL},
244239

245240
/* invalid intervals */

0 commit comments

Comments
 (0)