[PATCH v11 0/2] MR4372: kernel32: impl sync barrier
This is my first time contributing wine and there are a lot of things I'm not sure about, so any comments are welcome. :) -- v11: kernel32: add simple sync barrier test kernel32: impl sync barrier https://gitlab.winehq.org/wine/wine/-/merge_requests/4372
From: quininer <quininer(a)live.com> --- dlls/kernel32/kernel32.spec | 3 ++ dlls/kernelbase/kernelbase.spec | 6 +-- dlls/kernelbase/sync.c | 25 +++++++++++++ dlls/ntdll/ntdll.spec | 3 ++ dlls/ntdll/sync.c | 66 +++++++++++++++++++++++++++++++++ include/synchapi.h | 14 +++++++ include/winnt.h | 8 ++++ 7 files changed, 122 insertions(+), 3 deletions(-) diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec index 1deeb2a2ca5..081147d827d 100644 --- a/dlls/kernel32/kernel32.spec +++ b/dlls/kernel32/kernel32.spec @@ -375,6 +375,7 @@ @ stdcall DisassociateCurrentThreadFromCallback(ptr) NTDLL.TpDisassociateCallback @ stdcall DiscardVirtualMemory(ptr long) kernelbase.DiscardVirtualMemory @ stdcall DeleteTimerQueue(long) +@ stdcall -import DeleteSynchronizationBarrier(ptr) @ stdcall -import DeleteTimerQueueEx(long long) @ stdcall -import DeleteTimerQueueTimer(long long long) @ stdcall -arch=win64 DeleteUmsCompletionList(ptr) @@ -398,6 +399,7 @@ @ stdcall EndUpdateResourceA(long long) @ stdcall EndUpdateResourceW(long long) @ stdcall EnterCriticalSection(ptr) NTDLL.RtlEnterCriticalSection +@ stdcall -import EnterSynchronizationBarrier(ptr long) @ stdcall EnumCalendarInfoA(ptr long long long) @ stdcall EnumCalendarInfoExA(ptr long long long) @ stdcall -import EnumCalendarInfoExEx(ptr wstr long wstr long long) @@ -970,6 +972,7 @@ @ stdcall -import InitializeProcThreadAttributeList(ptr long long ptr) @ stdcall InitializeSListHead(ptr) NTDLL.RtlInitializeSListHead @ stdcall InitializeSRWLock(ptr) NTDLL.RtlInitializeSRWLock +@ stdcall -import InitializeSynchronizationBarrier(ptr long long) @ stdcall -arch=i386 InterlockedCompareExchange (ptr long long) @ stdcall -arch=i386 -ret64 InterlockedCompareExchange64(ptr int64 int64) NTDLL.RtlInterlockedCompareExchange64 @ stdcall -arch=i386 InterlockedDecrement(ptr) diff --git a/dlls/kernelbase/kernelbase.spec b/dlls/kernelbase/kernelbase.spec index e85f2b67d74..c3c6d8ed0dd 100644 --- a/dlls/kernelbase/kernelbase.spec +++ b/dlls/kernelbase/kernelbase.spec @@ -260,7 +260,7 @@ # @ stub DeleteStateAtomValue # @ stub DeleteStateContainer # @ stub DeleteStateContainerValue -# @ stub DeleteSynchronizationBarrier +@ stdcall DeleteSynchronizationBarrier(ptr) @ stdcall DeleteTimerQueueEx(long long) @ stdcall DeleteTimerQueueTimer(long long long) @ stdcall DeleteVolumeMountPointW(wstr) @@ -293,7 +293,7 @@ @ stdcall EncodeSystemPointer(ptr) ntdll.RtlEncodeSystemPointer # @ stub EnterCriticalPolicySectionInternal @ stdcall EnterCriticalSection(ptr) ntdll.RtlEnterCriticalSection -# @ stub EnterSynchronizationBarrier +@ stdcall EnterSynchronizationBarrier(ptr long) @ stdcall EnumCalendarInfoExEx(ptr wstr long wstr long long) @ stdcall EnumCalendarInfoExW(ptr long long long) @ stdcall EnumCalendarInfoW(ptr long long long) @@ -843,7 +843,7 @@ @ stdcall InitializeSRWLock(ptr) ntdll.RtlInitializeSRWLock @ stdcall InitializeSecurityDescriptor(ptr long) @ stdcall InitializeSid(ptr ptr long) -# @ stub InitializeSynchronizationBarrier +@ stdcall InitializeSynchronizationBarrier(ptr long long) # @ stub InstallELAMCertificateInfo @ stdcall -arch=i386 InterlockedCompareExchange(ptr long long) @ stdcall -arch=i386 -ret64 InterlockedCompareExchange64(ptr int64 int64) ntdll.RtlInterlockedCompareExchange64 diff --git a/dlls/kernelbase/sync.c b/dlls/kernelbase/sync.c index b2086207caa..a7943778891 100644 --- a/dlls/kernelbase/sync.c +++ b/dlls/kernelbase/sync.c @@ -1684,6 +1684,31 @@ BOOL WINAPI DECLSPEC_HOTPATCH InitOnceExecuteOnce( INIT_ONCE *once, PINIT_ONCE_F return !RtlRunOnceExecuteOnce( once, (PRTL_RUN_ONCE_INIT_FN)func, param, context ); } +/*********************************************************************** + * InitializeSynchronizationBarrier (kernelbase.@) + */ +BOOL WINAPI DECLSPEC_HOTPATCH InitializeSynchronizationBarrier( LPSYNCHRONIZATION_BARRIER barrier, LONG total_threads, + LONG spin_count ) +{ + return set_ntstatus( RtlInitBarrier( barrier, total_threads, spin_count )); +} + +/*********************************************************************** + * EnterSynchronizationBarrier (kernelbase.@) + */ +BOOL WINAPI DECLSPEC_HOTPATCH EnterSynchronizationBarrier( LPSYNCHRONIZATION_BARRIER barrier, DWORD flags ) +{ + return RtlBarrier(barrier, flags); +} + +/*********************************************************************** + * DeleteSynchronizationBarrier (kernelbase.@) + */ +BOOL WINAPI DECLSPEC_HOTPATCH DeleteSynchronizationBarrier( LPSYNCHRONIZATION_BARRIER barrier ) +{ + return set_ntstatus( RtlDeleteBarrier( barrier )); +} + #ifdef __i386__ /*********************************************************************** diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index c09157a369b..f9eac0744ef 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -514,6 +514,7 @@ @ stdcall RtlAreBitsSet(ptr long long) # @ stub RtlAssert2 @ stdcall RtlAssert(ptr ptr long str) +@ stdcall RtlBarrier(ptr long) # @ stub RtlCancelTimer @ stdcall -norelay RtlCaptureContext(ptr) @ stdcall RtlCaptureStackBackTrace(long long ptr ptr) @@ -598,6 +599,7 @@ @ stub RtlDelete @ stdcall RtlDeleteAce(ptr long) @ stdcall RtlDeleteAtomFromAtomTable(ptr long) +@ stdcall RtlDeleteBarrier(ptr) @ stdcall RtlDeleteCriticalSection(ptr) @ stdcall -arch=!i386 RtlDeleteGrowableFunctionTable(ptr) @ stub RtlDeleteElementGenericTable @@ -777,6 +779,7 @@ @ stdcall RtlImpersonateSelf(long) @ stdcall RtlInitAnsiString(ptr str) @ stdcall RtlInitAnsiStringEx(ptr str) +@ stdcall RtlInitBarrier(ptr long long) @ stdcall RtlInitCodePageTable(ptr ptr) # @ stub RtlInitMemoryStream @ stdcall RtlInitNlsTables(ptr ptr ptr ptr) diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c index bb5dcbb66e6..8ef5e1ef56e 100644 --- a/dlls/ntdll/sync.c +++ b/dlls/ntdll/sync.c @@ -1000,3 +1000,69 @@ void WINAPI RtlWakeAddressSingle( const void *addr ) if (tid) NtAlertThreadByThreadId( (HANDLE)(DWORD_PTR)tid ); } + + +typedef struct { + RTL_SRWLOCK lock; + RTL_CONDITION_VARIABLE cond; + ULONG count; +} barrier_entry; +C_ASSERT( sizeof(barrier_entry) <= sizeof(RTL_BARRIER) ); + +/*********************************************************************** + * RtlInitBarrier (NTDLL.@) + */ +NTSTATUS WINAPI RtlInitBarrier( PRTL_BARRIER Barrier, ULONG TotalThreads, ULONG SpinCount ) +{ + if(!Barrier) + return STATUS_INVALID_PARAMETER; + + barrier_entry *barrier = (barrier_entry*)Barrier; + + RtlInitializeSRWLock(&barrier->lock); + RtlInitializeConditionVariable(&barrier->cond); + barrier->count = TotalThreads; + + return STATUS_SUCCESS; +} + +/*********************************************************************** + * RtlBarrier (NTDLL.@) + */ +BOOLEAN WINAPI RtlBarrier( PRTL_BARRIER Barrier, ULONG Flags ) +{ + barrier_entry *barrier = (barrier_entry*)Barrier; + BOOL wait = TRUE; + BOOL hint = FALSE; + + RtlAcquireSRWLockExclusive(&barrier->lock); + + if (barrier->count > 0) { + barrier->count -= 1; + } + + if (barrier->count == 0) { + RtlWakeAllConditionVariable(&barrier->cond); + wait = FALSE; + hint = TRUE; + } + + while (wait) { + RtlSleepConditionVariableSRW(&barrier->cond, &barrier->lock, NULL, 0); + wait = barrier->count > 0; + } + + RtlReleaseSRWLockExclusive(&barrier->lock); + + return hint; +} + +/*********************************************************************** + * RtlDeleteBarrier (NTDLL.@) + */ +NTSTATUS WINAPI RtlDeleteBarrier( PRTL_BARRIER Barrier ) +{ + if(!Barrier) + return STATUS_INVALID_PARAMETER; + return STATUS_SUCCESS; +} diff --git a/include/synchapi.h b/include/synchapi.h index 0b3dcb8ff74..2c33907da6b 100644 --- a/include/synchapi.h +++ b/include/synchapi.h @@ -19,6 +19,8 @@ #ifndef _SYNCHAPI_H #define _SYNCHAPI_H +#include <winbase.h> + #ifdef __cplusplus extern "C" { #endif @@ -27,6 +29,18 @@ BOOL WINAPI WaitOnAddress(volatile void*, void*, SIZE_T, DWORD); void WINAPI WakeByAddressAll(void*); void WINAPI WakeByAddressSingle(void*); +typedef RTL_BARRIER SYNCHRONIZATION_BARRIER; +typedef PRTL_BARRIER PSYNCHRONIZATION_BARRIER; +typedef PRTL_BARRIER LPSYNCHRONIZATION_BARRIER; + +#define SYNCHRONIZATION_BARRIER_FLAGS_SPIN_ONLY 0x01 +#define SYNCHRONIZATION_BARRIER_FLAGS_BLOCK_ONLY 0x02 +#define SYNCHRONIZATION_BARRIER_FLAGS_NO_DELETE 0x04 + +BOOL WINAPI InitializeSynchronizationBarrier(LPSYNCHRONIZATION_BARRIER lpBarrier, LONG lTotalThreads, LONG lSpinCount); +BOOL WINAPI EnterSynchronizationBarrier(LPSYNCHRONIZATION_BARRIER lpBarrier, DWORD dwFlags); +BOOL WINAPI DeleteSynchronizationBarrier(LPSYNCHRONIZATION_BARRIER lpBarrier); + #ifdef __cplusplus } #endif diff --git a/include/winnt.h b/include/winnt.h index d1c82084f9f..7983b1eea4d 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -6097,6 +6097,14 @@ NTSYSAPI DWORD WINAPI RtlRunOnceExecuteOnce(PRTL_RUN_ONCE,PRTL_RUN_ONCE_INIT_FN, NTSYSAPI DWORD WINAPI RtlRunOnceBeginInitialize(PRTL_RUN_ONCE, DWORD, PVOID*); NTSYSAPI DWORD WINAPI RtlRunOnceComplete(PRTL_RUN_ONCE, DWORD, PVOID); +typedef struct _RTL_BARRIER { + DWORD Reserved1; + DWORD Reserved2; + ULONG_PTR Reserved3[2]; + DWORD Reserved4; + DWORD Reserved5; +} RTL_BARRIER, *PRTL_BARRIER; + #include <pshpack8.h> typedef struct _IO_COUNTERS { ULONGLONG DECLSPEC_ALIGN(8) ReadOperationCount; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4372
From: quininer <quininer(a)live.com> --- dlls/kernel32/tests/sync.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/dlls/kernel32/tests/sync.c b/dlls/kernel32/tests/sync.c index 56a9d6e4859..c15cfd14c0b 100644 --- a/dlls/kernel32/tests/sync.c +++ b/dlls/kernel32/tests/sync.c @@ -2841,6 +2841,42 @@ static void test_QueueUserAPC(void) ok(apc_count == 1, "APC count %u\n", apc_count); } +static DWORD WINAPI barrier_worker(LPVOID b) { + SYNCHRONIZATION_BARRIER *barrier = (SYNCHRONIZATION_BARRIER*)b; + EnterSynchronizationBarrier(barrier, 0); + return 0; +} + +static void test_barrier(void) +{ + SYNCHRONIZATION_BARRIER barrier; + DWORD dummy; + DWORD r; + + r = InitializeSynchronizationBarrier(&barrier, 3, 0); + ok( r == TRUE, "init sync barrier failed\n"); + + CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy); + CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy); + + EnterSynchronizationBarrier(&barrier, 0); + r = DeleteSynchronizationBarrier(&barrier); + ok( r == TRUE, "delete sync barrier failed\n"); + + // reuse barrier object + r = InitializeSynchronizationBarrier(&barrier, 5, 0); + ok( r == TRUE, "init sync barrier failed\n"); + + CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy); + CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy); + CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy); + CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy); + + EnterSynchronizationBarrier(&barrier, 0); + r = DeleteSynchronizationBarrier(&barrier); + ok( r == TRUE, "delete sync barrier failed\n"); +} + START_TEST(sync) { char **argv; @@ -2912,4 +2948,5 @@ START_TEST(sync) test_alertable_wait(); test_apc_deadlock(); test_crit_section(); + test_barrier(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4372
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=139901 Your paranoid android. === w8 (32 bit report) === kernel32: sync.c:2864: Test failed: delete sync barrier failed sync.c:2877: Test failed: delete sync barrier failed === w8adm (32 bit report) === kernel32: sync.c:2864: Test failed: delete sync barrier failed sync.c:2877: Test failed: delete sync barrier failed === w864 (32 bit report) === kernel32: sync.c:2864: Test failed: delete sync barrier failed sync.c:2877: Test failed: delete sync barrier failed === w1064v1507 (32 bit report) === kernel32: sync.c:2864: Test failed: delete sync barrier failed sync.c:2877: Test failed: delete sync barrier failed === w864 (64 bit report) === kernel32: sync.c:2864: Test failed: delete sync barrier failed sync.c:2877: Test failed: delete sync barrier failed === w1064v1507 (64 bit report) === kernel32: sync.c:2864: Test failed: delete sync barrier failed sync.c:2877: Test failed: delete sync barrier failed === debian11 (build log) === /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/kernelbase/sync.c:1693: undefined reference to `RtlInitBarrier' /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/kernelbase/sync.c:1701: undefined reference to `RtlBarrier' /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/kernelbase/sync.c:1709: undefined reference to `RtlDeleteBarrier' collect2: error: ld returned 1 exit status Task: The win32 Wine build failed === debian11b (build log) === /home/winetest/tools/testbot/var/wine-wow32/../wine/dlls/kernelbase/sync.c:1693: undefined reference to `RtlInitBarrier' /home/winetest/tools/testbot/var/wine-wow32/../wine/dlls/kernelbase/sync.c:1701: undefined reference to `RtlBarrier' /home/winetest/tools/testbot/var/wine-wow32/../wine/dlls/kernelbase/sync.c:1709: undefined reference to `RtlDeleteBarrier' collect2: error: ld returned 1 exit status Task: The wow32 Wine build failed
Etaash Mathamsetty (@etaash.mathamsetty) commented about dlls/kernel32/tests/sync.c:
+ SYNCHRONIZATION_BARRIER barrier; + DWORD dummy; + DWORD r; + + r = InitializeSynchronizationBarrier(&barrier, 3, 0); + ok( r == TRUE, "init sync barrier failed\n"); + + CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy); + CreateThread(NULL, 0, barrier_worker, &barrier, 0, &dummy); + + EnterSynchronizationBarrier(&barrier, 0); + r = DeleteSynchronizationBarrier(&barrier); + ok( r == TRUE, "delete sync barrier failed\n"); + + // reuse barrier object + r = InitializeSynchronizationBarrier(&barrier, 5, 0); By reusing I meant reusing without reinitializing btw you have to make comments like /* */ rather than // when contributing to wine
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4372#note_52441
On Wed Nov 15 22:29:00 2023 +0000, Etaash Mathamsetty wrote:
By reusing I meant reusing without reinitializing btw you have to make comments like /* */ rather than // when contributing to wine Ah, I subconsciously never thought of it as "reuse". I'll try to write a more useful test.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4372#note_52525
participants (4)
-
Etaash Mathamsetty (＠etaash.mathamsetty) -
Marvin -
quininer
-
quininer (＠quininer)