[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openbsd-misc
Subject:    Re: [OpenBSD -current] Change event timer in main loop with kqueue
From:       Visa Hankala <visa () hankala ! org>
Date:       2021-02-27 13:36:29
Message-ID: 20210227125632.2mG9k3LMQDx9u26U () hankala ! org
[Download RAW message or body]

Moving to tech@.

On Fri, Feb 26, 2021 at 09:42:07PM +0100, martin mag wrote:
> I've been trying to use kqueue for the last couple of day but I keep
> having an issue with EVFILT_TIMER filter. (I'm running Openbsd
> -current)
> 
> Right now, I'm trying to do the following:
> 1) Initilialize a timer event @ 200ms, periodically.
> 2) Inside the main event loop => If this event is retrieved, print
> elapsed time since last one
> 3) After 2 iterations, MODIFY the timer event to 1000ms and continue the loop
> 4) Code stops after 4 iterations as pb arise after the first timer
> change @ iteration 2.
> 
> Reading the manpages kqueue(2), one sees that:
>     ** ) An event is uniquely defined by the pair (ident, filter) ==>
> in the example below (TIMER1, EVFILT_TIMER)
>     **)  "" Re-adding an existing event will modify the parameters of
> the original event, and not result in a duplicate entry. "" => So
> re-adding the event (TIMER1, EVFILT_TIMER) with a modified field
> 'data' should update the timer from 200ms to 1000ms.
> 
> => Apparently, timer is updated, but not in the way I expected.

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.

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

[prev in list] [next in list] [prev in thread] [next in thread]