From d1b499c43e225971af92387e3ff85d42cbb22461 Mon Sep 17 00:00:00 2001 From: Matheus Date: Sat, 18 Jul 2026 17:35:00 -0300 Subject: [PATCH] pushlock: fix possible sleep on unlocked pushlock --- kernel-src/lib/pushlock.c | 46 +++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/kernel-src/lib/pushlock.c b/kernel-src/lib/pushlock.c index 07472150..08bb48a3 100644 --- a/kernel-src/lib/pushlock.c +++ b/kernel-src/lib/pushlock.c @@ -2,10 +2,10 @@ #include #include -#define CAS_ACQUIRE__RELAXED(ptr, saved, new) \ +#define CAS_ACQUIRE(ptr, saved, new) \ __atomic_compare_exchange_n(ptr, &saved, new, false, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED) -#define CAS_RELEASE__RELAXED(ptr, saved, new) \ +#define CAS_RELEASE(ptr, saved, new) \ __atomic_compare_exchange_n(ptr, &saved, new, false, __ATOMIC_RELEASE, __ATOMIC_RELAXED) typedef struct pushlock_wait_block_t { @@ -28,13 +28,13 @@ typedef struct pushlock_wait_block_t { bool pushlock_try_acquire_exclusive(pushlock_t *pushlock) { retry_lock: pushlock_t saved_value = 0; - if (CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) + if (CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) return true; if (saved_value & PUSHLOCK_FLAGS_ACQUIRED) return false; - if (!CAS_ACQUIRE__RELAXED(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) + if (!CAS_ACQUIRE(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) goto retry_lock; return true; @@ -43,11 +43,15 @@ retry_lock: void pushlock_acquire_exclusive(pushlock_t *pushlock) { retry_lock: pushlock_t saved_value = 0; - if (CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) + if (CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) return; - if ((saved_value & PUSHLOCK_FLAGS_ACQUIRED) == 0 && CAS_ACQUIRE__RELAXED(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) + if ((saved_value & PUSHLOCK_FLAGS_ACQUIRED) == 0) { + if (!CAS_ACQUIRE(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) + goto retry_lock; + return; + } pushlock_wait_block_t wait_block; SEMAPHORE_INIT(&wait_block.semaphore, 0); @@ -60,7 +64,7 @@ retry_lock: __atomic_store_n(&wait_block.shared_count, PUSHLOCK_GET_SHARED_COUNT(saved_value), __ATOMIC_RELAXED); } - if (CAS_RELEASE__RELAXED(pushlock, saved_value, (uintptr_t)&wait_block | (saved_value & PUSHLOCK_MASK_FLAGS) | PUSHLOCK_FLAGS_CONTENDED)) + if (CAS_RELEASE(pushlock, saved_value, (uintptr_t)&wait_block | (saved_value & PUSHLOCK_MASK_FLAGS) | PUSHLOCK_FLAGS_CONTENDED)) semaphore_wait(&wait_block.semaphore, false); goto retry_lock; @@ -69,14 +73,14 @@ retry_lock: bool pushlock_try_acquire_shared(pushlock_t *pushlock) { retry_lock: pushlock_t saved_value = 0; - if (CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(PUSHLOCK_FLAGS_ACQUIRED))) + if (CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(PUSHLOCK_FLAGS_ACQUIRED))) return true; if (saved_value & PUSHLOCK_FLAGS_EXCLUSIVE) return false; if ((saved_value & PUSHLOCK_FLAGS_CONTENDED) == 0) { - if (!CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(saved_value))) + if (!CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(saved_value))) goto retry_lock; return true; @@ -86,7 +90,7 @@ retry_lock: return false; // try to acquire as exclusive - if (!CAS_ACQUIRE__RELAXED(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) + if (!CAS_ACQUIRE(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) goto retry_lock; return true; @@ -95,7 +99,7 @@ retry_lock: void pushlock_acquire_shared(pushlock_t *pushlock) { retry_lock: pushlock_t saved_value = 0; - if (CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(PUSHLOCK_FLAGS_ACQUIRED))) + if (CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(PUSHLOCK_FLAGS_ACQUIRED))) return; if ((saved_value & PUSHLOCK_FLAGS_EXCLUSIVE) || ((saved_value & PUSHLOCK_FLAGS_ACQUIRED) && (saved_value & PUSHLOCK_FLAGS_CONTENDED))) { @@ -105,27 +109,27 @@ retry_lock: wait_block.next = PUSHLOCK_GET_POINTER(saved_value); __atomic_store_n(&wait_block.shared_count, 0, __ATOMIC_RELAXED); - if (CAS_RELEASE__RELAXED(pushlock, saved_value, (uintptr_t)&wait_block | (saved_value & PUSHLOCK_MASK_FLAGS) | PUSHLOCK_FLAGS_CONTENDED)) + if (CAS_RELEASE(pushlock, saved_value, (uintptr_t)&wait_block | (saved_value & PUSHLOCK_MASK_FLAGS) | PUSHLOCK_FLAGS_CONTENDED)) semaphore_wait(&wait_block.semaphore, false); goto retry_lock; } if (saved_value & PUSHLOCK_FLAGS_ACQUIRED) { - if (!CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(saved_value))) + if (!CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(saved_value))) goto retry_lock; return; } - if (!CAS_ACQUIRE__RELAXED(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) + if (!CAS_ACQUIRE(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) goto retry_lock; } // TODO: if we are releasing to a shared waiter, we should wake up every shared waiter void pushlock_release_exclusive(pushlock_t *pushlock) { pushlock_t saved_value = PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE; - if (CAS_RELEASE__RELAXED(pushlock, saved_value, 0)) + if (CAS_RELEASE(pushlock, saved_value, 0)) return; #ifdef DEBUG_LOCKS @@ -136,7 +140,7 @@ void pushlock_release_exclusive(pushlock_t *pushlock) { __atomic_thread_fence(__ATOMIC_ACQUIRE); pushlock_wait_block_t *wait_block = PUSHLOCK_GET_POINTER(saved_value); if (wait_block->next == NULL) { - if (CAS_RELEASE__RELAXED(pushlock, saved_value, 0)) { + if (CAS_RELEASE(pushlock, saved_value, 0)) { semaphore_signal(&wait_block->semaphore); return; } @@ -152,7 +156,7 @@ void pushlock_release_exclusive(pushlock_t *pushlock) { pushlock_wait_block_t *to_wake = wait_block->next; wait_block->next = NULL; - while (!CAS_RELEASE__RELAXED(pushlock, saved_value, (saved_value & PUSHLOCK_MASK_POINTER_SHARED_COUNT) | PUSHLOCK_FLAGS_CONTENDED)); + while (!CAS_RELEASE(pushlock, saved_value, (saved_value & PUSHLOCK_MASK_POINTER_SHARED_COUNT) | PUSHLOCK_FLAGS_CONTENDED)); semaphore_signal(&to_wake->semaphore); } @@ -173,7 +177,7 @@ static void pushlock_release_shared_contended(pushlock_t *pushlock, pushlock_t s __atomic_thread_fence(__ATOMIC_ACQUIRE); if (penultimate_wait_block == NULL) { - if (CAS_RELEASE__RELAXED(pushlock, saved_value, 0)) { + if (CAS_RELEASE(pushlock, saved_value, 0)) { semaphore_signal(&last_wait_block->semaphore); return; } @@ -187,7 +191,7 @@ static void pushlock_release_shared_contended(pushlock_t *pushlock, pushlock_t s penultimate_wait_block->next = NULL; - while (!CAS_RELEASE__RELAXED(pushlock, saved_value, (saved_value & PUSHLOCK_MASK_POINTER_SHARED_COUNT) | PUSHLOCK_FLAGS_CONTENDED)); + while (!CAS_RELEASE(pushlock, saved_value, (saved_value & PUSHLOCK_MASK_POINTER_SHARED_COUNT) | PUSHLOCK_FLAGS_CONTENDED)); semaphore_signal(&last_wait_block->semaphore); } @@ -195,7 +199,7 @@ static void pushlock_release_shared_contended(pushlock_t *pushlock, pushlock_t s void pushlock_release_shared(pushlock_t *pushlock) { retry_fast_path: pushlock_t saved_value = PUSHLOCK_INCREMENT_SHARED_COUNT(PUSHLOCK_FLAGS_ACQUIRED); - if (CAS_RELEASE__RELAXED(pushlock, saved_value, 0)) + if (CAS_RELEASE(pushlock, saved_value, 0)) return; #ifdef DEBUG_LOCKS @@ -213,6 +217,6 @@ retry_fast_path: return; } - if (!CAS_RELEASE__RELAXED(pushlock, saved_value, PUSHLOCK_DECREMENT_SHARED_COUNT(saved_value))) + if (!CAS_RELEASE(pushlock, saved_value, PUSHLOCK_DECREMENT_SHARED_COUNT(saved_value))) goto retry_fast_path; }