LIU Hao wrote: > I think this spinlock implementation is over-complicated. Since > `pthread_spinlock_t` is just an integer, > you can store the ID of the owner thread in there.
I attached updated version of 0004.txt (applies on top of 0003.txt in original message) which stores thread ID directly in `pthread_spinlock_t`. Other patches that follow need to be adjusted, so this is by no means final version. I just want to know what you think about this version. > And old code has > > while (unlikely(InterlockedExchangePointer((PVOID*)lk, 0) == 0)) > YieldProcessor(); > > which effects a huge amount of cache pressure. This can be much better: > > while (unlikely(InterlockedExchangePointer((thatD*)lk, 0) == 0)) > for (int i = 0; i != 100; ++i) > YieldProcessor(); > > The magic number 100 is, of course, an arbitrary choice. I believe you can > find a better one basing on > benchmark results on your hardware. The problem with new (attached) implementation is that we have to use InterlockedCompareExchange*. We cannot simply use InterlockedExchange* since it would store current thread's ID in `pthread_spinlock_t` when it should not. It didn't matter for previous implementation since it simply had two states "locked" and "unlocked", without keeping track of lock ownership. Leaving aside "spinning" part of `pthread_spin_lock`, I think my original implementation has one important advantage: it uses `pthread_spinlock_t` (which is `intptr_t`) as a handle. Currently, winpthreads does not support `PTHREAD_PROCESS_SHARED` for interprocess synchronization, but moving towards using handles for `pthread_*_t` objects is a direction to make it possible in the long run. Also note that current spinlock implementation was added in 249898d9ae (11 years ago), and this is probably what broke winpthreads' spinlocks (that they no longer keep track of lock ownership). Before that winpthreads had fairly complex spinlock implementation. - Kirill Makurin
From 8401e0fa14fd760483bb188d8252dd1d9a2c9c7a Mon Sep 17 00:00:00 2001 From: Kirill Makurin <[email protected]> Date: Thu, 14 May 2026 22:42:59 +0900 Subject: [PATCH 4/4] winpthreads: update spinlock implementation Signed-off-by: Kirill Makurin <[email protected]> --- .../winpthreads/src/spinlock.c | 100 ++++++++++++++---- .../winpthreads/tests/Makefile.am | 3 +- 2 files changed, 82 insertions(+), 21 deletions(-) diff --git a/mingw-w64-libraries/winpthreads/src/spinlock.c b/mingw-w64-libraries/winpthreads/src/spinlock.c index 0fd8d256d..e87d145e4 100644 --- a/mingw-w64-libraries/winpthreads/src/spinlock.c +++ b/mingw-w64-libraries/winpthreads/src/spinlock.c @@ -1,5 +1,5 @@ /* - Copyright (c) 2013 mingw-w64 project + Copyright (c) 2013,2026 mingw-w64 project Copyright (c) 2015 Intel Corporation Permission is hereby granted, free of charge, to any person obtaining a @@ -33,46 +33,108 @@ /* internal header files */ #include "misc.h" -/* We use the pthread_spinlock_t itself as a lock: - -1 is free, 0 is locked. - (This is dictated by PTHREAD_SPINLOCK_INITIALIZER, which we can't change - without breaking binary compatibility.) */ -typedef intptr_t spinlock_word_t; +/** + * We use the `pthread_spinlock_t` itself as a lock. + * + * We store `THREAD_ID_NO_OWNER` when lock is unlocked. Otherwise we store + * ID of the thread which owns the lock. + */ +#define THREAD_ID_NO_OWNER ((void *) PTHREAD_SPINLOCK_INITIALIZER) int pthread_spin_init (pthread_spinlock_t *lock, int pshared) { - spinlock_word_t *lk = (spinlock_word_t *)lock; - *lk = -1; + if (lock == NULL) { + return EINVAL; + } + + if (unlikely (pshared != PTHREAD_PROCESS_PRIVATE && pshared != PTHREAD_PROCESS_SHARED)) { + return EINVAL; + } + + if (unlikely (pshared == PTHREAD_PROCESS_SHARED)) { + return ENOSYS; + } + + *lock = PTHREAD_SPINLOCK_INITIALIZER; + return 0; } int pthread_spin_destroy (pthread_spinlock_t *lock) { + if (lock == NULL) { + return EINVAL; + } + + if (unlikely (*(void *volatile *) lock != THREAD_ID_NO_OWNER)) { + return EBUSY; + } + return 0; } int pthread_spin_lock (pthread_spinlock_t *lock) { - spinlock_word_t *lk = (spinlock_word_t *)lock; - while (unlikely(InterlockedExchangePointer((PVOID*)lk, 0) == 0)) { - YieldProcessor(); + if (lock == NULL) { + return EINVAL; + } + + void *threadId = (void *) (uintptr_t) GetCurrentThreadId (); + void *ownerId = InterlockedCompareExchangePointer ( + (void **) lock, threadId, THREAD_ID_NO_OWNER + ); + + if (unlikely (ownerId == threadId)) { + return EDEADLK; + } + + while (unlikely (ownerId != THREAD_ID_NO_OWNER)) { + YieldProcessor (); + + ownerId = InterlockedCompareExchangePointer ( + (void **) lock, threadId, THREAD_ID_NO_OWNER + ); } + return 0; } int pthread_spin_trylock (pthread_spinlock_t *lock) { - spinlock_word_t *lk = (spinlock_word_t *)lock; - return InterlockedExchangePointer((PVOID*)lk, 0) == 0 ? EBUSY : 0; + if (lock == NULL) { + return EINVAL; + } + + void *threadId = (void *) (uintptr_t) GetCurrentThreadId (); + void *ownerId = InterlockedCompareExchangePointer( + (void **) lock, threadId, THREAD_ID_NO_OWNER + ); + + if (unlikely(ownerId != THREAD_ID_NO_OWNER)) { + return EBUSY; + } + + return 0; } int pthread_spin_unlock (pthread_spinlock_t *lock) { - spinlock_word_t *lk = (spinlock_word_t *)lock; -#if defined(__GNUC__) || defined(__clang__) - __atomic_store_n(lk, -1, __ATOMIC_RELEASE); -#else - InterlockedExchangePointer((PVOID*)lk, (void*)-1); -#endif + if (lock == NULL) { + return EINVAL; + } + + void *threadId = (void *) (uintptr_t) GetCurrentThreadId (); + void *ownerId = InterlockedCompareExchangePointer ( + (void **) lock, THREAD_ID_NO_OWNER, threadId + ); + + if (unlikely (ownerId == THREAD_ID_NO_OWNER)) { + return 0; + } + + if (unlikely (ownerId != threadId)) { + return EPERM; + } + return 0; } diff --git a/mingw-w64-libraries/winpthreads/tests/Makefile.am b/mingw-w64-libraries/winpthreads/tests/Makefile.am index 711b7ff8e..2edc9a6f4 100644 --- a/mingw-w64-libraries/winpthreads/tests/Makefile.am +++ b/mingw-w64-libraries/winpthreads/tests/Makefile.am @@ -174,5 +174,4 @@ TESTS = $(check_PROGRAMS) # TODO: investigate why these tests fail XFAIL_TESTS = \ - pthread_spinlock/spin1 \ - pthread_spinlock/spin3 + pthread_spinlock/spin1 -- 2.51.0.windows.1
_______________________________________________ Mingw-w64-public mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
