From 37e385e7832c3dd897a583e648300b5a1d5d2cd1 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Sun, 11 Jul 2021 18:11:47 -0700 Subject: [PATCH] [kutil] Fix spinlock release during contention Spinlock release uses __atomic_compare_exchange_n, which overwrites the `desired` parameter with the actual value when the compare fails. This was causing releases to always spin when there was lock contention. --- src/libraries/kutil/include/kutil/spinlock.h | 2 +- src/libraries/kutil/spinlock.cpp | 21 +++++++++----------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/libraries/kutil/include/kutil/spinlock.h b/src/libraries/kutil/include/kutil/spinlock.h index 68a059c..96bbfbe 100644 --- a/src/libraries/kutil/include/kutil/spinlock.h +++ b/src/libraries/kutil/include/kutil/spinlock.h @@ -15,7 +15,7 @@ public: /// A node in the wait queue. struct waiter { - bool locked; + bool blocked; waiter *next; }; diff --git a/src/libraries/kutil/spinlock.cpp b/src/libraries/kutil/spinlock.cpp index 6c549b5..73b57de 100644 --- a/src/libraries/kutil/spinlock.cpp +++ b/src/libraries/kutil/spinlock.cpp @@ -11,7 +11,7 @@ void spinlock::acquire(waiter *w) { w->next = nullptr; - w->locked = true; + w->blocked = true; // Point the lock at this waiter waiter *prev = __atomic_exchange_n(&m_lock, w, memorder); @@ -19,30 +19,27 @@ spinlock::acquire(waiter *w) // If there was a previous waiter, wait for them to // unblock us prev->next = w; - while (w->locked) { + while (w->blocked) asm ("pause"); - } } else { - w->locked = false; + w->blocked = false; } } void spinlock::release(waiter *w) { - if (!w->next) { - // If we're still the last waiter, we're done - if(__atomic_compare_exchange_n(&m_lock, &w, nullptr, false, memorder, memorder)) - return; - } + // If we're still the last waiter, we're done + waiter *expected = w; + if(__atomic_compare_exchange_n(&m_lock, &expected, nullptr, false, memorder, memorder)) + return; // Wait for the subseqent waiter to tell us who they are - while (!w->next) { + while (!w->next) asm ("pause"); - } // Unblock the subseqent waiter - w->next->locked = false; + w->next->blocked = false; }