[PATCH v3 0/4] MR5237: Use futexes on macOS for thread-ID alerts.
This also bumps the minimum supported version of macOS to 10.12 to unconditionally assume the ulock syscalls are present. As discussed with @Gcenx this shouldn't be an issue, as the current minimum support version of 10.8 does not work anyways at the moment with only 10.10+ being useable and even then virtually no-one using wine on versions < 10.12 (and also being unsupported by the prebuilt binaries). Bumping to 10.12 would also have the side-benefit of allowing some cleanup of deprecated functions or legacy codepaths (like `mach_continuous_time` vs `mach_absolute_time` and probably much more). In my testing ulock futexes are about 2x faster for synchronization than kevents. Using the syscall wrappers here should provide protection against changing syscall numbers, and given that ulock is also directly used by libc++, the probability of an incompatible API change here is basically zero imo. By default ulock is also process-private, similar to the current linux implementation, cross-process synchronization would require darwin 19+ (macOS 10.15+) with `UL_COMPARE_AND_WAIT_SHARED`. The diff is a bit bigger than it actually is in effect, since https://gitlab.winehq.org/mzent/wine/-/commit/28a8a991c165f0d37dbf78ca3cdee4... is only moving some code around and not actually changing anything. -- v3: ntdll: Implement futex_wait() and futex_wake_one() on macOS. ntdll: Simplify futex interface from futex_wake() to futex_wake_one(). https://gitlab.winehq.org/wine/wine/-/merge_requests/5237
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/ntdll/unix/sync.c | 122 ++++++++++++++++++----------------------- 1 file changed, 54 insertions(+), 68 deletions(-) diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index cde6a0f8483..917c281b6a5 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -2334,10 +2334,10 @@ NTSTATUS WINAPI NtQueryInformationAtom( RTL_ATOM atom, ATOM_INFORMATION_CLASS cl union tid_alert_entry { -#ifdef HAVE_KQUEUE - int kq; -#elif defined(__linux__) +#ifdef __linux__ LONG futex; +#elif defined(HAVE_KQUEUE) + int kq; #else HANDLE event; #endif @@ -2375,7 +2375,9 @@ static union tid_alert_entry *get_tid_alert_entry( HANDLE tid ) entry = &tid_alert_blocks[block_idx][idx % TID_ALERT_BLOCK_SIZE]; -#ifdef HAVE_KQUEUE +#ifdef __linux__ + return entry; +#elif defined(HAVE_KQUEUE) if (!entry->kq) { int kq = kqueue(); @@ -2405,8 +2407,6 @@ static union tid_alert_entry *get_tid_alert_entry( HANDLE tid ) if (InterlockedCompareExchange( (LONG *)&entry->kq, kq, 0 )) close( kq ); } -#elif defined(__linux__) - return entry; #else if (!entry->event) { @@ -2434,7 +2434,14 @@ NTSTATUS WINAPI NtAlertThreadByThreadId( HANDLE tid ) if (!entry) return STATUS_INVALID_CID; -#ifdef HAVE_KQUEUE +#ifdef __linux__ + { + LONG *futex = &entry->futex; + if (!InterlockedExchange( futex, 1 )) + futex_wake( futex, 1 ); + return STATUS_SUCCESS; + } +#elif defined(HAVE_KQUEUE) { static const struct kevent signal_event = { @@ -2449,13 +2456,6 @@ NTSTATUS WINAPI NtAlertThreadByThreadId( HANDLE tid ) kevent( entry->kq, &signal_event, 1, NULL, 0, NULL ); return STATUS_SUCCESS; } -#elif defined(__linux__) - { - LONG *futex = &entry->futex; - if (!InterlockedExchange( futex, 1 )) - futex_wake( futex, 1 ); - return STATUS_SUCCESS; - } #else return NtSetEvent( entry->event, NULL ); #endif @@ -2485,59 +2485,6 @@ static LONGLONG update_timeout( ULONGLONG end ) #endif -#ifdef HAVE_KQUEUE - -/*********************************************************************** - * NtWaitForAlertByThreadId (NTDLL.@) - */ -NTSTATUS WINAPI NtWaitForAlertByThreadId( const void *address, const LARGE_INTEGER *timeout ) -{ - union tid_alert_entry *entry = get_tid_alert_entry( NtCurrentTeb()->ClientId.UniqueThread ); - ULONGLONG end; - int ret; - struct timespec timespec; - struct kevent wait_event; - - TRACE( "%p %s\n", address, debugstr_timeout( timeout ) ); - - if (!entry) return STATUS_INVALID_CID; - - if (timeout) - { - if (timeout->QuadPart == TIMEOUT_INFINITE) - timeout = NULL; - else - end = get_absolute_timeout( timeout ); - } - - do - { - if (timeout) - { - LONGLONG timeleft = update_timeout( end ); - - timespec.tv_sec = timeleft / (ULONGLONG)TICKSPERSEC; - timespec.tv_nsec = (timeleft % TICKSPERSEC) * 100; - if (timespec.tv_sec > 0x7FFFFFFF) timeout = NULL; - } - - ret = kevent( entry->kq, NULL, 0, &wait_event, 1, timeout ? ×pec : NULL ); - } while (ret == -1 && errno == EINTR); - - switch (ret) - { - case 1: - return STATUS_ALERTED; - case 0: - return STATUS_TIMEOUT; - default: - ERR( "kevent failed with error: %d (%s)\n", errno, strerror( errno ) ); - return STATUS_INVALID_HANDLE; - } -} - -#else - /*********************************************************************** * NtWaitForAlertByThreadId (NTDLL.@) */ @@ -2581,6 +2528,46 @@ NTSTATUS WINAPI NtWaitForAlertByThreadId( const void *address, const LARGE_INTEG } return STATUS_ALERTED; } +#elif defined(HAVE_KQUEUE) + { + ULONGLONG end; + int ret; + struct timespec timespec; + struct kevent wait_event; + + if (timeout) + { + if (timeout->QuadPart == TIMEOUT_INFINITE) + timeout = NULL; + else + end = get_absolute_timeout( timeout ); + } + + do + { + if (timeout) + { + LONGLONG timeleft = update_timeout( end ); + + timespec.tv_sec = timeleft / (ULONGLONG)TICKSPERSEC; + timespec.tv_nsec = (timeleft % TICKSPERSEC) * 100; + if (timespec.tv_sec > 0x7FFFFFFF) timeout = NULL; + } + + ret = kevent( entry->kq, NULL, 0, &wait_event, 1, timeout ? ×pec : NULL ); + } while (ret == -1 && errno == EINTR); + + switch (ret) + { + case 1: + return STATUS_ALERTED; + case 0: + return STATUS_TIMEOUT; + default: + ERR( "kevent failed with error: %d (%s)\n", errno, strerror( errno ) ); + return STATUS_INVALID_HANDLE; + } + } #else { NTSTATUS status = NtWaitForSingleObject( entry->event, FALSE, timeout ); @@ -2590,7 +2577,6 @@ NTSTATUS WINAPI NtWaitForAlertByThreadId( const void *address, const LARGE_INTEG #endif } -#endif /* Notify direct completion of async and close the wait handle if it is no longer needed. */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5237
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/ntdll/unix/sync.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index 917c281b6a5..476f0bb44c0 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -100,9 +100,10 @@ static inline ULONGLONG monotonic_counter(void) return ticks_from_time_t( now.tv_sec ) + now.tv_usec * 10 - server_start_time; } - #ifdef __linux__ +#define USE_FUTEX + #include <linux/futex.h> static inline int futex_wait( const LONG *addr, int val, struct timespec *timeout ) @@ -126,8 +127,7 @@ static inline int futex_wake( const LONG *addr, int val ) return syscall( __NR_futex, addr, FUTEX_WAKE_PRIVATE, val, NULL, 0, 0 ); } -#endif - +#endif /* __linux__ */ /* create a struct security_descriptor and contained information in one contiguous piece of memory */ unsigned int alloc_object_attributes( const OBJECT_ATTRIBUTES *attr, struct object_attributes **ret, @@ -2334,7 +2334,7 @@ NTSTATUS WINAPI NtQueryInformationAtom( RTL_ATOM atom, ATOM_INFORMATION_CLASS cl union tid_alert_entry { -#ifdef __linux__ +#ifdef USE_FUTEX LONG futex; #elif defined(HAVE_KQUEUE) int kq; @@ -2375,7 +2375,7 @@ static union tid_alert_entry *get_tid_alert_entry( HANDLE tid ) entry = &tid_alert_blocks[block_idx][idx % TID_ALERT_BLOCK_SIZE]; -#ifdef __linux__ +#ifdef USE_FUTEX return entry; #elif defined(HAVE_KQUEUE) if (!entry->kq) @@ -2434,7 +2434,7 @@ NTSTATUS WINAPI NtAlertThreadByThreadId( HANDLE tid ) if (!entry) return STATUS_INVALID_CID; -#ifdef __linux__ +#ifdef USE_FUTEX { LONG *futex = &entry->futex; if (!InterlockedExchange( futex, 1 )) @@ -2462,7 +2462,7 @@ NTSTATUS WINAPI NtAlertThreadByThreadId( HANDLE tid ) } -#if defined(__linux__) || defined(HAVE_KQUEUE) +#if defined(USE_FUTEX) || defined(HAVE_KQUEUE) static LONGLONG get_absolute_timeout( const LARGE_INTEGER *timeout ) { LARGE_INTEGER now; @@ -2496,7 +2496,7 @@ NTSTATUS WINAPI NtWaitForAlertByThreadId( const void *address, const LARGE_INTEG if (!entry) return STATUS_INVALID_CID; -#ifdef __linux__ +#ifdef USE_FUTEX { LONG *futex = &entry->futex; ULONGLONG end; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5237
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/ntdll/unix/sync.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index 476f0bb44c0..465e98c0d1d 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -122,9 +122,9 @@ static inline int futex_wait( const LONG *addr, int val, struct timespec *timeou return syscall( __NR_futex, addr, FUTEX_WAIT_PRIVATE, val, timeout, 0, 0 ); } -static inline int futex_wake( const LONG *addr, int val ) +static inline int futex_wake_one( const LONG *addr ) { - return syscall( __NR_futex, addr, FUTEX_WAKE_PRIVATE, val, NULL, 0, 0 ); + return syscall( __NR_futex, addr, FUTEX_WAKE_PRIVATE, 1, NULL, 0, 0 ); } #endif /* __linux__ */ @@ -2438,7 +2438,7 @@ NTSTATUS WINAPI NtAlertThreadByThreadId( HANDLE tid ) { LONG *futex = &entry->futex; if (!InterlockedExchange( futex, 1 )) - futex_wake( futex, 1 ); + futex_wake_one( futex ); return STATUS_SUCCESS; } #elif defined(HAVE_KQUEUE) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5237
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- README.md | 2 +- dlls/ntdll/unix/sync.c | 66 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 8a804b30dfc..28ab8fe7299 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ To compile and run Wine, you must have one of the following: - FreeBSD 12.4 or later - Solaris x86 9 or later - NetBSD-current -- Mac OS X 10.8 or later +- Mac OS X 10.12 or later As Wine requires kernel-level thread support to run, only the operating systems mentioned above are supported. Other operating systems which diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index 465e98c0d1d..8554e0747a6 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -127,7 +127,71 @@ static inline int futex_wake_one( const LONG *addr ) return syscall( __NR_futex, addr, FUTEX_WAKE_PRIVATE, 1, NULL, 0, 0 ); } -#endif /* __linux__ */ +#elif defined(__APPLE__) + +#define USE_FUTEX + +#include <AvailabilityMacros.h> + +#ifdef MAC_OS_VERSION_14_4 +#include <os/os_sync_wait_on_address.h> +#endif + +#define UL_COMPARE_AND_WAIT 1 + +extern int __ulock_wait( uint32_t operation, void *addr, uint64_t value, uint32_t timeout ); + +extern int __ulock_wake( uint32_t operation, void *addr, uint64_t wake_value ); + +static inline int futex_wait( const LONG *addr, int val, struct timespec *timeout ) +{ +#ifdef MAC_OS_VERSION_14_4 + if (__builtin_available( macOS 14.4, * )) + { + /* 18446744073 seconds could overflow a uint64_t in nanoseconds */ + if (timeout && timeout->tv_sec < 18446744073) + { + uint64_t ns_timeout = (timeout->tv_sec * 1000000000) + timeout->tv_nsec; + + if (!ns_timeout) + { + errno = ETIMEDOUT; + return -1; + } + return os_sync_wait_on_address_with_timeout( (void *)addr, (uint64_t)val, 4, OS_SYNC_WAIT_ON_ADDRESS_NONE, + OS_CLOCK_MACH_ABSOLUTE_TIME, ns_timeout ); + } + + return os_sync_wait_on_address( (void *)addr, (uint64_t)val, 4, OS_SYNC_WAIT_ON_ADDRESS_NONE ); + } +#endif + + /* 4294 seconds could overflow a uint32_t in microseconds */ + if (timeout && timeout->tv_sec < 4294) + { + uint32_t us_timeout = ((uint32_t)timeout->tv_sec * 1000000) + ((uint32_t)timeout->tv_nsec / 1000); + + if (!us_timeout) + { + errno = ETIMEDOUT; + return -1; + } + return __ulock_wait( UL_COMPARE_AND_WAIT, (void *)addr, (uint64_t)val, us_timeout ); + } + + return __ulock_wait( UL_COMPARE_AND_WAIT, (void *)addr, (uint64_t)val, 0 ); +} + +static inline int futex_wake_one( const LONG *addr ) +{ +#ifdef MAC_OS_VERSION_14_4 + if (__builtin_available( macOS 14.4, * )) + return os_sync_wake_by_address_any( (void *)addr, 4, OS_SYNC_WAKE_BY_ADDRESS_NONE ); +#endif + return __ulock_wake( UL_COMPARE_AND_WAIT, (void *)addr, 0 ); +} + +#endif /* __APPLE__ */ /* create a struct security_descriptor and contained information in one contiguous piece of memory */ unsigned int alloc_object_attributes( const OBJECT_ATTRIBUTES *attr, struct object_attributes **ret, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5237
Since wine does not use wake all or wake N functionality I simplified the interface a bit in v3, to make the implementation a bit cleaner and not implement unused functionality for `ulock` or `os_sync_wait_on_address`. This should make it also a bit easier to extend this implementation to other operating systems in the future. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5237#note_63654
On Wed Mar 6 08:03:28 2024 +0000, Dean M Greer wrote:
As discussed with @Gcenx this shouldn't be an issue, as the current minimum support version of 10.8 does not work anyways at the moment with only 10.10+ being useable and even then virtually no-one using wine on versions \< 10.12 (and also being unsupported by the prebuilt binaries). The Winehq macOS packages (installed via brew cask) require macOS Catalina (10.15.4) as of wine-stable-9.0. The lowest version of macOS we should really care to support going forward should be macOS High Sierra as Xcode9.4.1 was the last version of officially support 32Bit, that's ignoring Xcode10 Command Line Tools `macOS_SDK_headers_for_macOS_10.14.pkg` I wouldn't mind bumping the minimum version to 10.13 as well, if that is the consensus of what is reasonable (although maybe in a separate MR, since it is not strictly necessary for this one).
It definitely wouldn't hurt when trying to clean up some of the deprecated warnings. Incidentally 10.13 is also the lowest possible SDK used by nix when targeting Darwin, so that would line up nicely too. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5237#note_63655
participants (2)
-
Marc-Aurel Zent -
Marc-Aurel Zent (＠mzent)