Re: [OpenBSD -current] Change event timer in main loop with kqueue
On 21/03/21(Sun) 11:27, Visa Hankala wrote:
> On Sat, Feb 27, 2021 at 01:36:29PM +, 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?
I don't know if it exist code depending on this specific behavior but I
believe that, when it comes to BSD APIs exported to userland, being
aligned with FreeBSD is helpful. That's what I learned when working on
kqueue(2) backends when porting OSS.
> > 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 - 1.43
> > +++ lib/libc/sys/kqueue.2 27 Feb 2021 12:54:27 -
> > @@ -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 - 1.161
> > +++ sys/kern/kern_event.c 27 Feb 2021 12:54:27 -
> > @@ -135,7 +135,8 @@ int filt_fileattach(struct knote *kn);
> > void filt_timerexpire(void *knx);
> > intfilt_timerattach(struct knote *kn);
> > void filt_timerdetach(struct knote *kn);
> > -intfilt_timer(struct knote *kn, long hint);
> > +intfilt_timermodify(struct kevent *kev, struct knote *kn);
> > +intfilt_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);
> > }
> >
Re: [OpenBSD -current] Change event timer in main loop with kqueue
On Sat, Feb 27, 2021 at 01:36:29PM +, 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 - 1.43
> +++ lib/libc/sys/kqueue.2 27 Feb 2021 12:54:27 -
> @@ -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 - 1.161
> +++ sys/kern/kern_event.c 27 Feb 2021 12:54:27 -
> @@ -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);
> }
>
>
Re: [OpenBSD -current] Change event timer in main loop with kqueue
On Sun, Feb 28, 2021 at 03:36:59PM +0100, martin mag wrote: > Visa Hankala wrote (patch truncated): > > 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. > > ... PATCH TRUNCATED ... > > Thank you very much for your response. I won't be of much help on this > tech mailing list as I've just started using OpenBSD as my daily OS. > Still, I tried to apply your patch yesterday but my issue is still there. > Maybe I did not apply it correctly? Or maybe I'm not understanding very > well how kqueue should work and I should revert to using EV_ONESHOT > when an EVFILT_TIMER is to be modified inside loop? With the patch, if there is an existing timer with ident, EV_ADD will first cancel it and then start it anew using the new timer period. With an unpatched kernel, you can apply the new timeout period immediately by cancelling the existing timer manually with EV_DELETE and then adding a new timer with EV_ADD. > One other question (related) with modifying events inside main loop: If only > ONE > monitored event is to be changed, it is NOT possible to ONLY register THAT ONE > event to the queue right? Events can be added and removed from a kqueue separately without affecting other events. This is explained in the manual page.
Re: [OpenBSD -current] Change event timer in main loop with kqueue
Visa Hankala wrote (patch truncated):
> 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.
> ... PATCH TRUNCATED ...
Thank you very much for your response. I won't be of much help on this
tech mailing list as I've just started using OpenBSD as my daily OS.
Still, I tried to apply your patch yesterday but my issue is still there.
Maybe I did not apply it correctly? Or maybe I'm not understanding very
well how kqueue should work and I should revert to using EV_ONESHOT
when an EVFILT_TIMER is to be modified inside loop?
One other question (related) with modifying events inside main loop: If only ONE
monitored event is to be changed, it is NOT possible to ONLY register THAT ONE
event to the queue right?
For example:
.
count=0;
TIMER1=201;
BATT1 = open("/dev/apm", O_RDONLY);
EV_SET(&ch[0], TIMER1, EVFILT_TIMER, EV_ADD | EV_ENABLE, 0, 5000, NULL);
EV_SET(&ch[1], BATT1, EVFILT_READ, EV_ADD | EV_ENABLE | EV_CLEAR, 0, 0, NULL);
kevent(kq, ch, 2, NULL, 0, NULL);
for (;;){
nev = kevent(kq, NULL, 0, ev, 2, NULL);
for (int i=0; i you always get for the same very first event retrieved:
APM_EVENT_TYPE(ev[i].data) = 0
which according to source, is a NOT EVENT equivalent.
--> After retrieving that first event, you should receive other events such as
POWER_CHANGE (and no more that very first one).
However, if using the example above, you will ONLY receive events of type
NO_EVENT as the monitored events seem to be reset with the last
example line (executed at every iteration), disregarding it it was modified or
not.
Any idea of how to solve both those issues (apart from using multiple kqueues
with threads)? Is there a way to only register changed events, leaving
the others
untouched? (I guess I would still have the same issues if using
libev / libevent / libuv, as the backend is kqueue right?)
Thank you very much
Re: [OpenBSD -current] Change event timer in main loop with kqueue
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 - 1.43
+++ lib/libc/sys/kqueue.2 27 Feb 2021 12:54:27 -
@@ -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 - 1.161
+++ sys/kern/kern_event.c 27 Feb 2021 12:54:27 -
@@ -135,7 +135,8 @@ int filt_fileattach(struct knote *kn);
void filt_timerexpire(void *knx);
intfilt_timerattach(struct knote *kn);
void filt_timerdetach(struct knote *kn);
-intfilt_timer(struct knote *kn, long hint);
+intfilt_timermodify(struct kevent *kev, struct knote *kn);
+intfilt_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,
