From c9f3613fcd4204434888c559bf8edb28c1e02918 Mon Sep 17 00:00:00 2001 From: Mike Gerdts Date: Fri, 16 Dec 2022 21:11:40 -0600 Subject: [PATCH] thread: detect spinlocks that are not initialized If spdk_spin_lock() is called on an uninitialized spinlock, it will deadlock. This commit detects whether a lock is initialized and aborts instead of deadlocking. Signed-off-by: Mike Gerdts Change-Id: Ie7497633091edd4127c06ca0530e9a1dff530d1b Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/16002 Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto Tested-by: SPDK CI Jenkins --- include/spdk/thread.h | 2 ++ lib/thread/thread.c | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/include/spdk/thread.h b/include/spdk/thread.h index 198946a89..d0de4d17e 100644 --- a/include/spdk/thread.h +++ b/include/spdk/thread.h @@ -905,6 +905,8 @@ struct spdk_spinlock { pthread_spinlock_t spinlock; struct spdk_thread *thread; struct spdk_spinlock_internal *internal; + bool initialized; + bool destroyed; }; /** diff --git a/lib/thread/thread.c b/lib/thread/thread.c index 9f8eb7bfb..30d94b2e3 100644 --- a/lib/thread/thread.c +++ b/lib/thread/thread.c @@ -185,6 +185,11 @@ enum spin_error { * deadlock when another SPDK thread on the same pthread tries to take that lock. */ SPIN_ERR_HOLD_DURING_SWITCH, + /* Trying to use a lock that was destroyed (but not re-initialized) */ + SPIN_ERR_DESTROYED, + /* Trying to use a lock that is not initialized */ + SPIN_ERR_NOT_INITIALIZED, + /* Must be last, not an actual error code */ SPIN_ERR_LAST }; @@ -198,6 +203,8 @@ static const char *spin_error_strings[] = { [SPIN_ERR_LOCK_HELD] = "Destroying a held spinlock", [SPIN_ERR_LOCK_COUNT] = "Lock count is invalid", [SPIN_ERR_HOLD_DURING_SWITCH] = "Lock(s) held while SPDK thread going off CPU", + [SPIN_ERR_DESTROYED] = "Lock has been destroyed", + [SPIN_ERR_NOT_INITIALIZED] = "Lock has not been initialized", }; #define SPIN_ERROR_STRING(err) (err < 0 || err >= SPDK_COUNTOF(spin_error_strings)) \ @@ -2971,6 +2978,7 @@ spdk_spin_init(struct spdk_spinlock *sspin) SPIN_ASSERT_LOG_STACKS(rc == 0, SPIN_ERR_PTHREAD, sspin); sspin_init_internal(sspin); SSPIN_GET_STACK(sspin, init); + sspin->initialized = true; } void @@ -2978,12 +2986,16 @@ spdk_spin_destroy(struct spdk_spinlock *sspin) { int rc; + SPIN_ASSERT_LOG_STACKS(!sspin->destroyed, SPIN_ERR_DESTROYED, sspin); + SPIN_ASSERT_LOG_STACKS(sspin->initialized, SPIN_ERR_NOT_INITIALIZED, sspin); SPIN_ASSERT_LOG_STACKS(sspin->thread == NULL, SPIN_ERR_LOCK_HELD, sspin); rc = pthread_spin_destroy(&sspin->spinlock); SPIN_ASSERT_LOG_STACKS(rc == 0, SPIN_ERR_PTHREAD, sspin); sspin_fini_internal(sspin); + sspin->initialized = false; + sspin->destroyed = true; } void @@ -2992,6 +3004,8 @@ spdk_spin_lock(struct spdk_spinlock *sspin) struct spdk_thread *thread = spdk_get_thread(); int rc; + SPIN_ASSERT_LOG_STACKS(!sspin->destroyed, SPIN_ERR_DESTROYED, sspin); + SPIN_ASSERT_LOG_STACKS(sspin->initialized, SPIN_ERR_NOT_INITIALIZED, sspin); SPIN_ASSERT_LOG_STACKS(thread != NULL, SPIN_ERR_NOT_SPDK_THREAD, sspin); SPIN_ASSERT_LOG_STACKS(thread != sspin->thread, SPIN_ERR_DEADLOCK, sspin); @@ -3010,6 +3024,8 @@ spdk_spin_unlock(struct spdk_spinlock *sspin) struct spdk_thread *thread = spdk_get_thread(); int rc; + SPIN_ASSERT_LOG_STACKS(!sspin->destroyed, SPIN_ERR_DESTROYED, sspin); + SPIN_ASSERT_LOG_STACKS(sspin->initialized, SPIN_ERR_NOT_INITIALIZED, sspin); SPIN_ASSERT_LOG_STACKS(thread != NULL, SPIN_ERR_NOT_SPDK_THREAD, sspin); SPIN_ASSERT_LOG_STACKS(thread == sspin->thread, SPIN_ERR_WRONG_THREAD, sspin);