Created attachment 177580 [details] test case When a kqueue timer is created and updated (before the original timer fires), the update has no effect. See the attached sample program, which sets a oneshot timer for 1s in the future, then changes the period to 1ms, then waits. The final kevent call waits for (approximately) 1s, which would happen if the update doesn't take effect. freebsd-11-x86-64-0:/u0/home/bwelling: ./timer kevent time: 1000124 On macOS Sierra, for comparison: wavelet:~: ./timer kevent time: 1217 It's possible that reusing timers is not allowed, but I can't find anything in the documentation that indicates that.
Empirical observation: Despite EV_ADD being used for both kevents in your sample code, the same knote is reused. Despite filt_timerattach() being called twice without error, the shorter callout doesn't wake the note. Digging more...
Just kidding. filt_timerattach() is only invoked once.
I see. You are reusing the timer with ident=1. Hm. With: @@ -25,7 +25,7 @@ int n = ::kevent(kq, &event, 1, nullptr, 0, nullptr); assert(n == 0); - EV_SET(&event, 1, EVFILT_TIMER, flags, fflags, 1000, nullptr); + EV_SET(&event, 2, EVFILT_TIMER, flags, fflags, 1000, nullptr); n = ::kevent(kq, &event, 1, nullptr, 0, nullptr); assert(n == 0); Two timers are attached, and the program indeed wakes up quickly: CPU ID FUNCTION:NAME 1 64686 kevent:entry 2017 Jan 11 20:06:00.530 1 15060 filt_timerattach:entry 2017 Jan 11 20:06:00.530 kn:fffff800286c4e00 1 15061 filt_timerattach:return 2017 Jan 11 20:06:00.530 kn:fffff800286c4e00 kc:fffff80005cb4200 1 15061 filt_timerattach:return 2017 Jan 11 20:06:00.530 return:0 1 15058 filt_timer:entry 2017 Jan 11 20:06:00.530 kn:fffff800286c4e00 hint:0 1 15059 filt_timer:return 2017 Jan 11 20:06:00.530 return:0 1 64686 kevent:entry 2017 Jan 11 20:06:00.530 1 15060 filt_timerattach:entry 2017 Jan 11 20:06:00.530 kn:fffff800286c4e80 1 15061 filt_timerattach:return 2017 Jan 11 20:06:00.530 kn:fffff800286c4e80 kc:fffff80005cb4180 1 15061 filt_timerattach:return 2017 Jan 11 20:06:00.530 return:0 1 15058 filt_timer:entry 2017 Jan 11 20:06:00.530 kn:fffff800286c4e80 hint:0 1 15059 filt_timer:return 2017 Jan 11 20:06:00.530 return:0 1 64686 kevent:entry 2017 Jan 11 20:06:00.530 1 15064 filt_timerexpire:entry 2017 Jan 11 20:06:00.531 kn:fffff800286c4e80 1 15065 filt_timerexpire:return 2017 Jan 11 20:06:00.531 return:fffff80004224a00 3 15062 filt_timerdetach:entry 2017 Jan 11 20:06:00.531 kn:fffff800286c4e80 3 15063 filt_timerdetach:return 2017 Jan 11 20:06:00.531 return:2 3 15062 filt_timerdetach:entry 2017 Jan 11 20:06:00.532 kn:fffff800286c4e00 3 15063 filt_timerdetach:return 2017 Jan 11 20:06:00.532 return:1
It appears that fd-based filters will have the same behavior -- duplicate adds (same ident and filter) are just ignored. Given that, perhaps this is just a documentation bug?
In fact: EV_ADD Adds the event to the kqueue. Re-adding an existing event ^^^^^^^^^^^^^^^^^^^^^^^^^^^ will modify the parameters of the original event, and not result in a duplicate entry. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ As far as I can tell, this isn't actually true.
(In reply to Conrad Meyer from comment #5) Well, ok, things like the user-supplied udata, fflags, and data are updated. Hmmm. We could hook f_touch or just handle it in f_event.
I have a patch to modify existing timers: $ ./test kevent time: 1017 It is probably racy. f_event calls should be protected by the knote list lock, but nothing protects the first timer callout racing with the 2nd registration's polling.
Created attachment 178772 [details] patch to update timer kevents
Created attachment 178773 [details] Simple dtrace script for tracing timer filter calls
Conrad, do you want to take your patch forward? If not, I'd be interested in taking on this bug. (In reply to Conrad Meyer from comment #8)
(In reply to David Bright from comment #10) Nope, I don't have any interest in it. Please, go ahead.
A commit references this bug: Author: dab Date: Fri Jul 27 13:49:18 UTC 2018 New revision: 336761 URL: https://svnweb.freebsd.org/changeset/base/336761 Log: Allow a EVFILT_TIMER kevent to be updated. If a timer is updated (re-added) with a different time period (specified in the .data field of the kevent), the new time period has no effect; the timer will not expire until the original time has elapsed. This violates the documented behavior as the kqueue(2) man page says (in part) "Re-adding an existing event will modify the parameters of the original event, and not result in a duplicate entry." This modification, adapted from a patch submitted by cem@ to PR214987, fixes the kqueue system to allow updating a timer entry. The kevent timer behavior is changed to: * When a timer is re-added, update the timer parameters to and re-start the timer using the new parameters. * Allow updating both active and already expired timers. * When the timer has already expired, dequeue any undelivered events and clear the count of expirations. All of these changes address the original PR and also bring the FreeBSD and macOS kevent timer behaviors into agreement. A few other changes were made along the way: * Update the kqueue(2) man page to reflect the new timer behavior. * Fix man page style issues in kqueue(2) diagnosed by igor. * Update the timer libkqueue system test to test for the updated timer behavior. * Fix the (test) libkqueue common.h file so that it includes config.h which defines various HAVE_* feature defines, before the #if tests for such variables in common.h. This enables the use of the actual err(3) family of functions. * Fix the usages of the err(3) functions in the tests for incorrect type of variables. Those were formerly undiagnosed due to the disablement of the err(3) functions (see previous bullet point). PR: 214987 Reported by: Brian Wellington <bwelling@xbill.org> Reviewed by: kib MFC after: 1 week Relnotes: yes Sponsored by: Dell EMC Differential Revision: https://reviews.freebsd.org/D15778 Changes: head/lib/libc/sys/kqueue.2 head/sys/kern/kern_event.c head/tests/sys/kqueue/libkqueue/common.h head/tests/sys/kqueue/libkqueue/main.c head/tests/sys/kqueue/libkqueue/timer.c
Awesome! Thanks for following through on this, David.
A commit references this bug: Author: dab Date: Tue Aug 7 14:39:01 UTC 2018 New revision: 337418 URL: https://svnweb.freebsd.org/changeset/base/337418 Log: MFC r336761 & r336781: Allow a EVFILT_TIMER kevent to be updated. If a timer is updated (re-added) with a different time period (specified in the .data field of the kevent), the new time period has no effect; the timer will not expire until the original time has elapsed. This violates the documented behavior as the kqueue(2) man page says (in part) "Re-adding an existing event will modify the parameters of the original event, and not result in a duplicate entry." This modification, adapted from a patch submitted by cem@ to PR214987, fixes the kqueue system to allow updating a timer entry. The kevent timer behavior is changed to: * When a timer is re-added, update the timer parameters to and re-start the timer using the new parameters. * Allow updating both active and already expired timers. * When the timer has already expired, dequeue any undelivered events and clear the count of expirations. All of these changes address the original PR and also bring the FreeBSD and macOS kevent timer behaviors into agreement. A few other changes were made along the way: * Update the kqueue(2) man page to reflect the new timer behavior. * Fix man page style issues in kqueue(2) diagnosed by igor. * Update the timer libkqueue system test to test for the updated timer behavior. * Fix the (test) libkqueue common.h file so that it includes config.h which defines various HAVE_* feature defines, before the #if tests for such variables in common.h. This enables the use of the actual err(3) family of functions. * Fix the usages of the err(3) functions in the tests for incorrect type of variables. Those were formerly undiagnosed due to the disablement of the err(3) functions (see previous bullet point). PR: 214987 Relnotes: yes Sponsored by: Dell EMC Changes: _U stable/11/ stable/11/lib/libc/sys/kqueue.2 stable/11/sys/kern/kern_event.c stable/11/tests/sys/kqueue/libkqueue/common.h stable/11/tests/sys/kqueue/libkqueue/main.c stable/11/tests/sys/kqueue/libkqueue/timer.c
Committed to -12 when it was -CURRENT (i.e., it is in 12.0-RELEASE), so no MFC to stable12. MFC'd to stable11. No intention to MFC to stable10. Closing.