Summary: | kevent EVFILT_TIMER, EV_DISPATCH lost if event is re-enabled | ||
---|---|---|---|
Product: | Base System | Reporter: | Arran Cudbard-Bell <a.cudbardb> |
Component: | kern | Assignee: | freebsd-bugs (Nobody) <bugs> |
Status: | Open --- | ||
Severity: | Affects Some People | CC: | markj |
Priority: | --- | ||
Version: | 13.0-RELEASE | ||
Hardware: | Any | ||
OS: | Any |
Description
Arran Cudbard-Bell
2021-09-10 18:56:04 UTC
After investigating further macOS seems to be behaving correctly, as the event is only disabled when it is retrieved which is after sleeping for 1s. It would make sense then for the data field to contain 5. The FreeBSD behaviour still seems erroneous however. Thanks for the report and the pointer to the updated libkqueue tests. I'm sorry this didn't get any attention sooner. I ported three new timer tests to our in-tree copy of the libkqueue test suite, periodic_modify, periodic_to_oneshot, and dispatch. They found a couple of kernel bugs unrelated to the one that you described. Looking at our documentation, it's not obvious to me that FreeBSD's behaviour in the dispatch test is incorrect. With a periodic timer, EV_DISPATCH masks the knote after its state is retrieved by kqueue(), but the timer continues to fire and we continue to count events. So when the timer filter is unmasked with EV_ENABLE | EV_DISPATCH, the knote is immediately activated. That seems consistent with my reading of the documentation. > The above step fails on FreeBSD 13, we get one event with the data field set to 1. I would expect this to be 2 = 500ms / 200ms. That itself is a kernel bug which I have a patch for. > Adding EV_ADD to the call re-enabling the timer does nothing, as does only passing EV_ENABLE. In fact, adding EV_ADD fixes the test for me on FreeBSD. That also seems to make sense, since EV_ADD causes the timer state to be reset, throwing away timer firings that occurred while the knote was masked. So, I'm not sure what to do here. I spent some time looking at the XNU sources to better understand what macos is doing but don't quite understand their timer state machine yet. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=d6d4f9b45e0be306bdaf53b2133b2cd0f7642167 commit d6d4f9b45e0be306bdaf53b2133b2cd0f7642167 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2022-05-25 00:16:32 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2022-05-25 00:16:32 +0000 kqueue tests: Add new EVFILT_TIMER regression tests from upstream One of the tests exposes the regression reported in PR 264131. One test is disabled because FreeBSD does not support setting EV_ONESHOT on an already-added periodic timer. Though, in this case the flag is simply ignored, which isn't ideal. One test is slightly modified to set EV_ADD when reconfiguring a disabled timer per some commentary in PR 258412. Ideally we would re-import the test suite from libkqueue but there is a fair bit of divergence so this will require some effort. This just gets us one small step closer while increasing test coverage. PR: 258412 MFC after: 2 weeks Sponsored by: The FreeBSD Foundation tests/sys/kqueue/libkqueue/config.h | 1 + tests/sys/kqueue/libkqueue/timer.c | 129 +++++++++++++++++++++++++++++++++++- 2 files changed, 128 insertions(+), 2 deletions(-) A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=f2ce33a640fddeb0763d4114bc7f06aa521effd0 commit f2ce33a640fddeb0763d4114bc7f06aa521effd0 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2022-05-25 00:16:32 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2022-06-08 00:42:25 +0000 kqueue tests: Add new EVFILT_TIMER regression tests from upstream One of the tests exposes the regression reported in PR 264131. One test is disabled because FreeBSD does not support setting EV_ONESHOT on an already-added periodic timer. Though, in this case the flag is simply ignored, which isn't ideal. One test is slightly modified to set EV_ADD when reconfiguring a disabled timer per some commentary in PR 258412. Ideally we would re-import the test suite from libkqueue but there is a fair bit of divergence so this will require some effort. This just gets us one small step closer while increasing test coverage. PR: 258412 Sponsored by: The FreeBSD Foundation (cherry picked from commit d6d4f9b45e0be306bdaf53b2133b2cd0f7642167) tests/sys/kqueue/libkqueue/config.h | 1 + tests/sys/kqueue/libkqueue/timer.c | 129 +++++++++++++++++++++++++++++++++++- 2 files changed, 128 insertions(+), 2 deletions(-) |