Support OpenBSD. #559
Open
Support OpenBSD. #559
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kithrup
reviewed
May 18, 2021
src/event/event_internal.h
Outdated
Comment on lines
128
to
129
| #elif defined(__OpenBSD__) | ||
| typedef uint32_t dispatch_unote_ident_t; |
There was a problem hiding this comment.
Unless the typedef is going to change, that is an unnecessary ifdef, no?
There was a problem hiding this comment.
I was experimenting a little here and forgot to change this back. This needs to match struct kevent, so should be a uintptr_t, and necessitates some further changes.
kithrup
reviewed
May 18, 2021
src/event/event_kevent.c
Outdated
| @@ -727,6 +745,10 @@ _dispatch_kq_poll(dispatch_wlh_t wlh, dispatch_kevent_t ke, int n, | |||
| (void)avail; | |||
| const struct timespec timeout_immediately = {}, *timeout = NULL; | |||
| if (flags & KEVENT_FLAG_IMMEDIATE) timeout = &timeout_immediately; | |||
| #if EVFILT_USER | |||
This was referenced Jul 7, 2021
|
Closed smaller PRs in favor of this omnibus, so it is easier to track and will centralize any future review. |
This was referenced Jul 9, 2021
3405691582
force-pushed
the
build
branch
3 times, most recently
from
b5d8e1a to
71bb0ec
Compare
|
@etcwilde, mind reviewing? |
kithrup
reviewed
Dec 10, 2021
src/queue.c
Outdated
| @@ -6592,6 +6606,11 @@ _dispatch_runloop_queue_handle_dispose(dispatch_lane_t dq) | |||
| #elif defined(__linux__) | |||
| int rc = close(handle); | |||
| (void)dispatch_assume_zero(rc); | |||
| #elif defined(__unix__) | |||
| int rc = close((int)(handle & 0xffffffff)); | |||
There was a problem hiding this comment.
The handle packs both fds of the pipe into a single 64-bit integer -- this plucks out the lower 32-bits.
I'll define some macros to make this a little more obvious.
kithrup
approved these changes
Dec 10, 2021
3405691582
added a commit
to 3405691582/swift
that referenced
this pull request
Mar 26, 2022
Make the following documentation changes: * update last tested version number and emphasize that the process may work for later platform versions, * update required packages to emphasize py3 instead of py2.7, * update the suggested checkout config file to the stable branch used in the "upstream" checkout config file, and * mention some extra flags are now required to properly switch off Dispatch (since without pr swiftlang/swift-corelibs-libdispatch#559 Dispatch won't compile). * reinforce building release by default, since the flags likely needed for debug builds aren't provided in the command snippet.
3405691582
added a commit
to 3405691582/swift
that referenced
this pull request
Jun 11, 2022
Make the following documentation changes: * update last tested version number and emphasize that the process may work for later platform versions, * update required packages to emphasize py3 instead of py2.7, * python3 link is not strictly required to need root. * update the suggested checkout config file to the stable branch used in the "upstream" checkout config file, and * mention some extra flags are now required to properly switch off Dispatch (since without pr swiftlang/swift-corelibs-libdispatch#559 Dispatch won't compile). * reinforce building release by default, since the flags likely needed for debug builds aren't provided in the command snippet.
|
@drexin, would you review? |
3405691582
force-pushed
the
build
branch
3 times, most recently
from
b617c6e to
4a3942e
Compare
|
While waiting for additional reviews, I made the following changes in this pr from cross-compiling and building this change on Linux:
Additionally, I ran the tests on Linux forcing the queue runloop to use the pipe2 implementation proposed in this pr rather than epoll; it seemed to work fine. |
3405691582
requested review from
rokhinip and
compnerd
and removed request for
rokhinip and
compnerd
|
@swift-ci please test |
x86_64 is spelled "amd64" on this platform. Return "amd64", for consistency with Swift and other projects. For Windows, return "x86_64".
The kevent backend requires _dispatch_bug_kevent_client unconditionally. We don't need to require this entire function require HAVE_MACH, just the inner few that is already present behind this conditional.
|
@swift-ci please test |
It is just a hint, after all, so it is not an error if it is unavailable.
Building dispatch from swift sets -Werror -Wimplicit-fallthrough. It is not enough to just comment the fallthrough; we have elsewhere defined DISPATCH_FALLTHROUGH, so we might as well use it.
Prior to this change, on non-Linux, the test harness will already attempt to use dispatch functionality when testing dispatch itself. This does not seem intentional, and instead appears to be intended as the Mac OS case where the platform already has Dispatch and is using that when testing the just-built version. Instead, make the Linux case the general `__unix__` case.
OpenBSD has futex, but not futex_pi. Segregate futex functionality from pi-futex functionality with a new preprocessor symbol.
The Linux implementation uses an eventfd to impement the runloop contract for CF. However, ordinary POSIX pipes are a widely available API and can be used to achieve the same result as eventfd on other systems that support it. Since POSIX pipes require a pair of file descriptors, we force file descriptors to 32-bit integers and pack these two into a single 64-bit integer. This keeps the management of the runloop handle simple rather than having to manage the handle storage externally, for the cost of limiting the range of the file descriptor type.
Some functionality requires particular preprocessor feature flags to be
set but errors are raised if they aren't, so the remedy is to put these
behind relevant feature flags.
* dispatch_kevent_t has a qos field only when DISPATCH_USE_KEVENT_QOS
* _dispatch_kq_unote_update requires DISPATCH_HAVE_DIRECT_KNOTES as it
is only called from functions behind this
* _dispatch_workloop_actions and the associated enum are only used when
DISPATCH_USE_KEVENT_WORKLOOP.
Some unused variable void casts are removed (guard_ptr and pp), which do
not refer to variables -- these may be upstream merge artefacts.
A type cast is made to match types for _dispatch_bug_kevent_client. This
function is incorrectly placed behind HAVE_MACH in src/init.c, but to
keep the scope and impact of this commit narrow, we will defer that
change.
Since EVFILT_FS is not supported on all kevent implementations, put these
behind an #ifdef. EVFILT_USER also requires similar handling, but since
EVFILT_USER kevents are used intrinsically as part of the functionality,
these will be handled in a separate commit.
Some kqueue timer implementations do not support absolute timers; these also only have millisecond resolution.
Some kqueue implementations do not have EVFILT_USER. We can work around this by creating a timer implementation with a very small timeout.[1] [1] Thanks to this post, which provided the idea for the workaround: https://lists.macosforge.org/pipermail/libdispatch-dev/2009-September/000010.html
Different platforms may define dispatch_unote_ident_t differently and subsequently this type may have different bit widths. Therefore, when printing du_ident, cast explicitly to deal with format mismatch errors. The alternative is to specify explicit format string macros, but this is a little complex and casting is already being used in swiftlang#584. One special case for consideration is `_dispatch_timer_unote_disarm`; this gets the `du_ident` and converts that into a `uint32_t` timer index. When `dispatch_unote_ident` is a `uint32_t` this is of course fine, but needs consideration for when it isn't. Here, we just cast down. This is somewhat reasonable since this is initialized from `_dispatch_timer_unote_idx` which returns an `unsigned int`. (Of course, we are not actually bounds checking that index, but that's outside the scope of this commit).
Add the right `os(OpenBSD)` directives. Ensure `DISPATCH_COCOA_COMPAT` is defined and expose the runloop innards for `CFRunLoop` to use in Foundation down the line.
Simply speaking, the global concurrent queue has a thread per cpu and schedules functions or blocks on one of these threads. If these threads are all busy, without workqueue monitoring, we cannot schedule a new function or block until one of those threads returns. If thread(s) have not returned however because they are asleep (say), then we could overcommit the global concurrent queue by scheduling more work. The Linux implementation checks each registered thread ID to see if the corresponding thread is in a runnable state according to procfs every second. On some platforms without procfs, this same information is obtainable essentially via the relevant sysctls. Libraries may exist to facilitate issuing the correct sysctls (c.f. `kvm_getprocs`) but these were not used here so to avoid complications with the right invocations when linking. A unit test is also added to test the overcommit functionality. In this test, we tie up all cpus to prevent the scheduled threads from returning and then attempt to run one more task. If the overcommit monitoring is working as intended, then Dispatch will detect all the threads are simply asleep and will attempt to overcommit. If not, the test will notice the function we attempted to overcommit did not run and fail the test.
Using a 512K buffer in this test may mean that the test process may hit the stack size rlimit quickly and segfault. Instead of trying to reduce the buffer sizes and then subsequently try and unwind other weird behavior in this test, just use heap-allocated buffers. We don't actually seem to care about the contents in this test, so we can just throw them away immediately after we read into them.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior separated PRs were closed in favor of this omnibus.
This PR implements functionality to properly support Dispatch on OpenBSD. This includes fixing some errors, portability cleanup, as well as a portable implementation of the Dispatch runloop and making sure that the kevent backend works properly and portably on kernels that may not support all kevent features (like user-driven kevents).
This PR influences the design of swiftlang/swift-corelibs-foundation#3004 because CFRunLoop in Foundation peeks under the curtain of how the Dispatch runloop handles are implemented.