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);
> }
>
>
