Re: kern/59339: heartbeat watchdog fires since 10.99.14

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

> Date: Fri, 9 May 2025 14:59:02 +0200
> From: Thomas Klausner <wiz%NetBSD.org@localhost>
> 
> Seems to have been the same problem both times:
> 
> (gdb) bt
> #4  0xffffffff80dff166 in itimer_arm_real (it=0xffff89c5007f1e80) at /usr/src/sys/kern/kern_time.c:837
> #5  0xffffffff80e007eb in itimer_settime (it=it@entry=0xffff89c5007f1e80) at /usr/src/sys/kern/kern_time.c:946

OK, that patch was a dud -- the assertion may not pass in that path,
but it should pass via itimer_callout.  Try the attached one instead?

# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1746796090 0
#      Fri May 09 13:08:10 2025 +0000
# Branch trunk
# Node ID 05a98ecf5eda0704c8b94356f22e97870c808378
# Parent  55a6ded9e19ed0908fbb94d13fa69cda6d0b4571
# EXP-Topic riastradh-pr59339-heartbeat
WIP: Rearming a timer should never go backwards.  Assert it.

Handle overflow case gracefully while here (not likely relevant to
the symptoms we've seen, though).

PR kern/59339: heartbeat watchdog fires since 10.99.14

diff -r 55a6ded9e19e -r 05a98ecf5eda sys/kern/kern_time.c
--- a/sys/kern/kern_time.c	Thu May 08 20:51:40 2025 +0000
+++ b/sys/kern/kern_time.c	Fri May 09 13:08:10 2025 +0000
@@ -837,6 +837,55 @@ itimer_arm_real(struct itimer * const it
 }
 
 /*
+ * itimer_rearm_real:
+ *
+ *	Re-arm a non-virtual timer, given the current clock readout.
+ */
+static void
+itimer_rearm_real(struct itimer * const it, const struct timespec * const now)
+{
+	const struct timespec * const interval = &it->it_time.it_interval;
+	const struct timespec * const next = &it->it_time.it_value;
+	struct timespec delta;
+	int ticks;
+
+	KASSERT(!it->it_dying);
+	KASSERT(!CLOCK_VIRTUAL_P(it->it_clockid));
+	KASSERT(!callout_pending(&it->it_ch));
+
+	if (__predict_true(timespecisset(next))) {
+		KASSERTMSG(timespeccmp(now, next, <),
+		    "[clock %u]"
+		    " now=%lld.%09ld interval=%lld.%09ld then=%lld.%09ld",
+		    it->it_clockid,
+		    (long long)now->tv_sec, (long)now->tv_nsec,
+		    (long long)interval->tv_sec, (long)interval->tv_nsec,
+		    (long long)next->tv_sec, (long)next->tv_nsec);
+		timespecsub(next, now, &delta);
+	} else {
+		/*
+		 * If the arithmetic overflowed, just take the interval
+		 * as the time to wait.  This is unlikely to happen
+		 * unless you are messing with your clock to set it
+		 * ahead by hundreds of years.
+		 */
+		delta = it->it_time.it_interval; /* overflow */
+	}
+	ticks = tstohz(&delta);
+	KASSERTMSG(ticks > 0,
+	    "[clock %u]"
+	    " now=%lld.%09ld interval=%lld.%09ld then=%lld.%09ld"
+	    " delta=%lld.%09ld ticks=%d",
+	    it->it_clockid,
+	    (long long)now->tv_sec, (long)now->tv_nsec,
+	    (long long)interval->tv_sec, (long)interval->tv_nsec,
+	    (long long)next->tv_sec, (long)next->tv_nsec,
+	    (long long)delta.tv_sec, (long)delta.tv_nsec,
+	    ticks);
+	callout_schedule(&it->it_ch, ticks);
+}
+
+/*
  * itimer_callout:
  *
  *	Callout to expire a non-virtual timer.  Queue it up for processing,
@@ -872,6 +921,18 @@ itimer_callout(void *arg)
 	 * now, compute the next itimer value and count overruns.
 	 */
 	itimer_transition(&it->it_time, &now, &next, &overruns);
+	KASSERTMSG(timespeccmp(&now, &next, <),
+	    "[clock %u]"
+	    " it->it_time.it_value=%lld.%09ld"
+	    " it->it_time.it_interval=%lld.%09ld"
+	    " now=%lld.%09ld next=%lld.%09ld",
+	    it->it_clockid,
+	    (long long)it->it_time.it_value.tv_sec,
+	    (long)it->it_time.it_value.tv_nsec,
+	    (long long)it->it_time.it_interval.tv_sec,
+	    (long)it->it_time.it_interval.tv_nsec,
+	    (long long)now.tv_sec, (long)now.tv_nsec,
+	    (long long)next.tv_sec, (long)next.tv_nsec);
 	it->it_time.it_value = next;
 	it->it_overruns += overruns;
 
@@ -879,7 +940,7 @@ itimer_callout(void *arg)
 	 * Reset the callout, if it's not going away.
 	 */
 	if (!it->it_dying)
-		itimer_arm_real(it);
+		itimer_rearm_real(it, &now);
 	itimer_unlock();
 }