filt_timerexpire_l(): re-insert restarted timer into head instead of tail

[kib]

Summary

of the resumed timers list, so that kqtimer_proc_continue() does not
iterated into the same timer again.

PR:     293141


kqtimer_proc_continue(): correct calculation of 'now'

It must be sbinuptime(), this is how kc->next is set up.

PR:     293141
Noted by:       markj

[markj]

sys/kern/kern_event.c

828

now is derived from getboottimebin(), isn't this bogus for non-NOTE_ABSTIME knotes?

852

... and isn't this wrong for NOTE_ABSTIME knotes?

[kib]

sys/kern/kern_event.c

828

Sorry, I do not follow.

For any kind of timer (ABSTIME or not), kc->next records the sbintime (relative to the boottime) of the moment when the timer must be fired. At least this is my understanding, please see filt_timerstart().

[markj]

sys/kern/kern_event.c

828

filt_timerstart() records the sbintime relative to the uptime, the amount of time elapsed since the kernel booted. The boottime is the time that the kernel booted relative to the unix epoch.

So here, the comparison kc->next <= now is always true in practice, for non-ABSTIME knotes. That is why the infinite loop happens: filt_timerexpire_l() should be setting kc->next to some time in the future, so kqtimer_proc_continue() should not visit the knote again, but it does anyway.

I agree with your patch in any case, but this loop also looks wrong to me.

[kib]

sys/kern/kern_event.c

828

Do you mean that now is incorrectly calculated there? Like this?

diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index 85b7b6c953af..a1cd409c53d9 100644
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -814,14 +814,11 @@ void
 kqtimer_proc_continue(struct proc *p)
 {
 	struct kq_timer_cb_data *kc, *kc1;
-	struct bintime bt;
 	sbintime_t now;
 
 	PROC_LOCK_ASSERT(p, MA_OWNED);
 
-	getboottimebin(&bt);
-	now = bttosbt(bt);
-
+	now = sbinuptime();
 	TAILQ_FOREACH_SAFE(kc, &p->p_kqtim_stop, link, kc1) {
 		TAILQ_REMOVE(&p->p_kqtim_stop, kc, link);
 		kc->flags &= ~KQ_TIMER_CB_ENQUEUED;

[markj]

sys/kern/kern_event.c

828

I think so, yes.

852

My comment here is wrong, I misunderstood the code. kc->next is always relative to the kernel's boot time.

[kib]

Correct the 'now' calculation for kqtimer_proc_continue()

[markj]

Thank you.

sys/kern/kern_event.c

828

I verified that this change on its own fixes the loop triggered by the test program in PR 293141.

[kib]

sys/kern/kern_event.c

828

In typical situation yes, but if the thread was descheduled for some time larger than the timer timeout, it might be not.