[PATCH] ntdll: Read the condition variable and manipulate the lock on the PE side.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49712 Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/ntdll/sync.c | 21 +++------ dlls/ntdll/unix/loader.c | 3 +- dlls/ntdll/unix/sync.c | 84 ++++++---------------------------- dlls/ntdll/unix/unix_private.h | 7 +-- dlls/ntdll/unixlib.h | 8 +--- 5 files changed, 26 insertions(+), 97 deletions(-) diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c index edbdd91c1b9..8df7015df9f 100644 --- a/dlls/ntdll/sync.c +++ b/dlls/ntdll/sync.c @@ -481,16 +481,12 @@ void WINAPI RtlWakeAllConditionVariable( RTL_CONDITION_VARIABLE *variable ) NTSTATUS WINAPI RtlSleepConditionVariableCS( RTL_CONDITION_VARIABLE *variable, RTL_CRITICAL_SECTION *crit, const LARGE_INTEGER *timeout ) { + const void *value = variable->Ptr; NTSTATUS status; - int val; - if ((status = unix_funcs->fast_RtlSleepConditionVariableCS( variable, crit, - timeout )) != STATUS_NOT_IMPLEMENTED) - return status; - - val = *(int *)&variable->Ptr; RtlLeaveCriticalSection( crit ); - status = RtlWaitOnAddress( &variable->Ptr, &val, sizeof(int), timeout ); + if ((status = unix_funcs->fast_wait_cv( variable, value, timeout )) == STATUS_NOT_IMPLEMENTED) + status = RtlWaitOnAddress( &variable->Ptr, &value, sizeof(value), timeout ); RtlEnterCriticalSection( crit ); return status; } @@ -517,21 +513,16 @@ NTSTATUS WINAPI RtlSleepConditionVariableCS( RTL_CONDITION_VARIABLE *variable, R NTSTATUS WINAPI RtlSleepConditionVariableSRW( RTL_CONDITION_VARIABLE *variable, RTL_SRWLOCK *lock, const LARGE_INTEGER *timeout, ULONG flags ) { + const void *value = variable->Ptr; NTSTATUS status; - int val; - - if ((status = unix_funcs->fast_RtlSleepConditionVariableSRW( variable, lock, timeout, - flags )) != STATUS_NOT_IMPLEMENTED) - return status; - - val = *(int *)&variable->Ptr; if (flags & RTL_CONDITION_VARIABLE_LOCKMODE_SHARED) RtlReleaseSRWLockShared( lock ); else RtlReleaseSRWLockExclusive( lock ); - status = RtlWaitOnAddress( &variable->Ptr, &val, sizeof(int), timeout ); + if ((status = unix_funcs->fast_wait_cv( variable, value, timeout )) == STATUS_NOT_IMPLEMENTED) + status = RtlWaitOnAddress( variable, &value, sizeof(value), timeout ); if (flags & RTL_CONDITION_VARIABLE_LOCKMODE_SHARED) RtlAcquireSRWLockShared( lock ); diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index e7cc050ba9d..1424bf3cc6a 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -1383,9 +1383,8 @@ static struct unix_funcs unix_funcs = fast_RtlAcquireSRWLockShared, fast_RtlReleaseSRWLockExclusive, fast_RtlReleaseSRWLockShared, - fast_RtlSleepConditionVariableSRW, - fast_RtlSleepConditionVariableCS, fast_RtlWakeConditionVariable, + fast_wait_cv, ntdll_atan, ntdll_ceil, ntdll_cos, diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index 23dca9c61b3..18e64b02627 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -2490,81 +2490,34 @@ NTSTATUS CDECL fast_RtlReleaseSRWLockShared( RTL_SRWLOCK *lock ) return STATUS_SUCCESS; } -static NTSTATUS wait_cv( int *futex, int val, const LARGE_INTEGER *timeout ) +NTSTATUS CDECL fast_wait_cv( RTL_CONDITION_VARIABLE *variable, const void *value, const LARGE_INTEGER *timeout ) { + const char *value_ptr; + int aligned_value, *futex; struct timespec timespec; int ret; - if (timeout && timeout->QuadPart != TIMEOUT_INFINITE) - { - timespec_from_timeout( ×pec, timeout ); - ret = futex_wait( futex, val, ×pec ); - } - else - ret = futex_wait( futex, val, NULL ); - - if (ret == -1 && errno == ETIMEDOUT) - return STATUS_TIMEOUT; - return STATUS_WAIT_0; -} - -NTSTATUS CDECL fast_RtlSleepConditionVariableCS( RTL_CONDITION_VARIABLE *variable, - RTL_CRITICAL_SECTION *cs, const LARGE_INTEGER *timeout ) -{ - NTSTATUS status; - int val, *futex; - if (!use_futexes()) return STATUS_NOT_IMPLEMENTED; if (!(futex = get_futex( &variable->Ptr ))) return STATUS_NOT_IMPLEMENTED; - val = *futex; + value_ptr = (const char *)&value; + value_ptr += ((ULONG_PTR)futex) - ((ULONG_PTR)&variable->Ptr); + aligned_value = *(int *)value_ptr; - if (cs->RecursionCount == 1) + if (timeout && timeout->QuadPart != TIMEOUT_INFINITE) { - /* FIXME: simplified version of RtlLeaveCriticalSection/RtlEnterCriticalSection to avoid imports */ - cs->RecursionCount = 0; - cs->OwningThread = 0; - if (InterlockedDecrement( &cs->LockCount ) >= 0) fast_RtlpUnWaitCriticalSection( cs ); - - status = wait_cv( futex, val, timeout ); - - if (InterlockedIncrement( &cs->LockCount )) fast_RtlpWaitForCriticalSection( cs, INT_MAX ); - cs->OwningThread = ULongToHandle( GetCurrentThreadId() ); - cs->RecursionCount = 1; + timespec_from_timeout( ×pec, timeout ); + ret = futex_wait( futex, aligned_value, ×pec ); } - else status = wait_cv( futex, val, timeout ); - return status; -} - -NTSTATUS CDECL fast_RtlSleepConditionVariableSRW( RTL_CONDITION_VARIABLE *variable, RTL_SRWLOCK *lock, - const LARGE_INTEGER *timeout, ULONG flags ) -{ - NTSTATUS status; - int val, *futex; - - if (!use_futexes()) - return STATUS_NOT_IMPLEMENTED; - - if (!(futex = get_futex( &variable->Ptr )) || !get_futex( &lock->Ptr )) - return STATUS_NOT_IMPLEMENTED; - - val = *futex; - - if (flags & RTL_CONDITION_VARIABLE_LOCKMODE_SHARED) - fast_RtlReleaseSRWLockShared( lock ); else - fast_RtlReleaseSRWLockExclusive( lock ); - - status = wait_cv( futex, val, timeout ); + ret = futex_wait( futex, aligned_value, NULL ); - if (flags & RTL_CONDITION_VARIABLE_LOCKMODE_SHARED) - fast_RtlAcquireSRWLockShared( lock ); - else - fast_RtlAcquireSRWLockExclusive( lock ); - return status; + if (ret == -1 && errno == ETIMEDOUT) + return STATUS_TIMEOUT; + return STATUS_WAIT_0; } NTSTATUS CDECL fast_RtlWakeConditionVariable( RTL_CONDITION_VARIABLE *variable, int count ) @@ -2678,19 +2631,12 @@ NTSTATUS CDECL fast_RtlReleaseSRWLockShared( RTL_SRWLOCK *lock ) return STATUS_NOT_IMPLEMENTED; } -NTSTATUS CDECL fast_RtlSleepConditionVariableSRW( RTL_CONDITION_VARIABLE *variable, RTL_SRWLOCK *lock, - const LARGE_INTEGER *timeout, ULONG flags ) -{ - return STATUS_NOT_IMPLEMENTED; -} - -NTSTATUS CDECL fast_RtlSleepConditionVariableCS( RTL_CONDITION_VARIABLE *variable, - RTL_CRITICAL_SECTION *cs, const LARGE_INTEGER *timeout ) +NTSTATUS CDECL fast_RtlWakeConditionVariable( RTL_CONDITION_VARIABLE *variable, int count ) { return STATUS_NOT_IMPLEMENTED; } -NTSTATUS CDECL fast_RtlWakeConditionVariable( RTL_CONDITION_VARIABLE *variable, int count ) +NTSTATUS CDECL fast_wait_cv( RTL_CONDITION_VARIABLE *variable, const void *value, const LARGE_INTEGER *timeout ) { return STATUS_NOT_IMPLEMENTED; } diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 397211957bd..1d3d58b2533 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -98,13 +98,10 @@ extern NTSTATUS CDECL fast_RtlTryAcquireSRWLockShared( RTL_SRWLOCK *lock ) DECLS extern NTSTATUS CDECL fast_RtlAcquireSRWLockShared( RTL_SRWLOCK *lock ) DECLSPEC_HIDDEN; extern NTSTATUS CDECL fast_RtlReleaseSRWLockExclusive( RTL_SRWLOCK *lock ) DECLSPEC_HIDDEN; extern NTSTATUS CDECL fast_RtlReleaseSRWLockShared( RTL_SRWLOCK *lock ) DECLSPEC_HIDDEN; -extern NTSTATUS CDECL fast_RtlSleepConditionVariableSRW( RTL_CONDITION_VARIABLE *variable, RTL_SRWLOCK *lock, - const LARGE_INTEGER *timeout, ULONG flags ) DECLSPEC_HIDDEN; -extern NTSTATUS CDECL fast_RtlSleepConditionVariableCS( RTL_CONDITION_VARIABLE *variable, - RTL_CRITICAL_SECTION *cs, - const LARGE_INTEGER *timeout ) DECLSPEC_HIDDEN; extern NTSTATUS CDECL fast_RtlWakeConditionVariable( RTL_CONDITION_VARIABLE *variable, int count ) DECLSPEC_HIDDEN; extern LONGLONG CDECL fast_RtlGetSystemTimePrecise(void) DECLSPEC_HIDDEN; +extern NTSTATUS CDECL fast_wait_cv( RTL_CONDITION_VARIABLE *variable, const void *value, + const LARGE_INTEGER *timeout ) DECLSPEC_HIDDEN; void CDECL mmap_add_reserved_area( void *addr, SIZE_T size ) DECLSPEC_HIDDEN; void CDECL mmap_remove_reserved_area( void *addr, SIZE_T size ) DECLSPEC_HIDDEN; diff --git a/dlls/ntdll/unixlib.h b/dlls/ntdll/unixlib.h index 2f57873671c..2af1bc6ac98 100644 --- a/dlls/ntdll/unixlib.h +++ b/dlls/ntdll/unixlib.h @@ -53,13 +53,9 @@ struct unix_funcs NTSTATUS (CDECL *fast_RtlAcquireSRWLockShared)( RTL_SRWLOCK *lock ); NTSTATUS (CDECL *fast_RtlReleaseSRWLockExclusive)( RTL_SRWLOCK *lock ); NTSTATUS (CDECL *fast_RtlReleaseSRWLockShared)( RTL_SRWLOCK *lock ); - NTSTATUS (CDECL *fast_RtlSleepConditionVariableSRW)( RTL_CONDITION_VARIABLE *variable, - RTL_SRWLOCK *lock, - const LARGE_INTEGER *timeout, ULONG flags ); - NTSTATUS (CDECL *fast_RtlSleepConditionVariableCS)( RTL_CONDITION_VARIABLE *variable, - RTL_CRITICAL_SECTION *cs, - const LARGE_INTEGER *timeout ); NTSTATUS (CDECL *fast_RtlWakeConditionVariable)( RTL_CONDITION_VARIABLE *variable, int count ); + NTSTATUS (CDECL *fast_wait_cv)( RTL_CONDITION_VARIABLE *variable, const void *value, + const LARGE_INTEGER *timeout ); /* math functions */ double (CDECL *atan)( double d ); -- 2.28.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=77388 Your paranoid android. === debiant (64 bit WoW report) === ntdll: threadpool.c:1904: Test failed: WaitForSingleObject returned 258
Zebediah Figura <z.figura12(a)gmail.com> writes:
RtlLeaveCriticalSection( crit ); - status = RtlWaitOnAddress( &variable->Ptr, &val, sizeof(int), timeout ); + if ((status = unix_funcs->fast_wait_cv( variable, value, timeout )) == STATUS_NOT_IMPLEMENTED) + status = RtlWaitOnAddress( &variable->Ptr, &value, sizeof(value), timeout );
Couldn't you always use RtlWaitOnAddress, since it has a fast path already? I'd like to reduce the number of non-syscall entry points into the Unix library. -- Alexandre Julliard julliard(a)winehq.org
On 8/21/20 7:59 AM, Alexandre Julliard wrote:
Zebediah Figura <z.figura12(a)gmail.com> writes:
RtlLeaveCriticalSection( crit ); - status = RtlWaitOnAddress( &variable->Ptr, &val, sizeof(int), timeout ); + if ((status = unix_funcs->fast_wait_cv( variable, value, timeout )) == STATUS_NOT_IMPLEMENTED) + status = RtlWaitOnAddress( &variable->Ptr, &value, sizeof(value), timeout );
Couldn't you always use RtlWaitOnAddress, since it has a fast path already? I'd like to reduce the number of non-syscall entry points into the Unix library.
Unfortunately it's not as easy as that. Pseudo-futexes can't map onto futexes directly, because they can have a size of 1/2/4/8 *and* RtlWakeAddress* doesn't know the size. As a result we have to map them onto a smaller, fixed-size array of futexes. That's worse performance than we would like. Implementing condition variables on top of that loses performance relative to the current implementation, not only in theory but also in practice—the game Path of Exile suffered from this (though I don't remember how much).
participants (3)
-
Alexandre Julliard -
Marvin -
Zebediah Figura