On Sat, Feb 27, 2021 at 01:36:29PM +0000, Visa Hankala wrote: > The kernel does not reschedule the timer when the user changes the > timeout period. The new period will take effect only after the current > period has expired. This is not explained in the manual page, though. > > With the recent kqueue changes, it is straightforward to make the kernel > modify an existing timer. I think the clearest behaviour is to reset the > timer completely when it is modified. If there are pending events, they > should be cancelled because they do not necessarily correspond to the > new settings. > > When f_modify and f_process are present in kqread_filtops, filt_timer > is not used. filt_timerexpire() activates timer knotes directly using > knote_activate() instead of KNOTE(). > > However, the current behaviour has been around so long that one can > argue that it is an actual feature. BSDs are not consistent with this, > though. FreeBSD resets the timer immediately, whereas NetBSD and > DragonFly BSD apply the new period after expiry. > > I guess the resetting is harmless in most cases but might wreak havoc > at least with software that keeps poking its timers before expiry.
I have received too little feedback to commit this. The most important question is, should the timer behaviour be changed? > Index: lib/libc/sys/kqueue.2 > =================================================================== > RCS file: src/lib/libc/sys/kqueue.2,v > retrieving revision 1.43 > diff -u -p -r1.43 kqueue.2 > --- lib/libc/sys/kqueue.2 14 Nov 2020 10:16:15 -0000 1.43 > +++ lib/libc/sys/kqueue.2 27 Feb 2021 12:54:27 -0000 > @@ -468,6 +468,11 @@ contains the number of times the timeout > This filter automatically sets the > .Dv EV_CLEAR > flag internally. > +.Pp > +If an existing timer is re-added, the existing timer and related pending > events > +will be cancelled. > +The timer will be re-started using the timeout period > +.Fa data . > .It Dv EVFILT_DEVICE > Takes a descriptor as the identifier and the events to watch for in > .Fa fflags , > Index: sys/kern/kern_event.c > =================================================================== > RCS file: src/sys/kern/kern_event.c,v > retrieving revision 1.161 > diff -u -p -r1.161 kern_event.c > --- sys/kern/kern_event.c 24 Feb 2021 14:59:52 -0000 1.161 > +++ sys/kern/kern_event.c 27 Feb 2021 12:54:27 -0000 > @@ -135,7 +135,8 @@ int filt_fileattach(struct knote *kn); > void filt_timerexpire(void *knx); > int filt_timerattach(struct knote *kn); > void filt_timerdetach(struct knote *kn); > -int filt_timer(struct knote *kn, long hint); > +int filt_timermodify(struct kevent *kev, struct knote *kn); > +int filt_timerprocess(struct knote *kn, struct kevent *kev); > void filt_seltruedetach(struct knote *kn); > > const struct filterops kqread_filtops = { > @@ -163,7 +164,9 @@ const struct filterops timer_filtops = { > .f_flags = 0, > .f_attach = filt_timerattach, > .f_detach = filt_timerdetach, > - .f_event = filt_timer, > + .f_event = NULL, > + .f_modify = filt_timermodify, > + .f_process = filt_timerprocess, > }; > > struct pool knote_pool; > @@ -444,15 +447,48 @@ filt_timerdetach(struct knote *kn) > struct timeout *to; > > to = (struct timeout *)kn->kn_hook; > - timeout_del(to); > + timeout_del_barrier(to); > free(to, M_KEVENT, sizeof(*to)); > kq_ntimeouts--; > } > > int > -filt_timer(struct knote *kn, long hint) > +filt_timermodify(struct kevent *kev, struct knote *kn) > +{ > + struct timeout *to = kn->kn_hook; > + int s; > + > + /* Reset the timer. Any pending events are discarded. */ > + > + timeout_del_barrier(to); > + > + s = splhigh(); > + if (kn->kn_status & KN_QUEUED) > + knote_dequeue(kn); > + kn->kn_status &= ~KN_ACTIVE; > + splx(s); > + > + kn->kn_data = 0; > + knote_modify(kev, kn); > + /* Reinit timeout to invoke tick adjustment again. */ > + timeout_set(to, filt_timerexpire, kn); > + filt_timer_timeout_add(kn); > + > + return (0); > +} > + > +int > +filt_timerprocess(struct knote *kn, struct kevent *kev) > { > - return (kn->kn_data != 0); > + int active, s; > + > + s = splsoftclock(); > + active = (kn->kn_data != 0); > + if (active) > + knote_submit(kn, kev); > + splx(s); > + > + return (active); > } > >