diff --git a/headers/private/kernel/lock.h b/headers/private/kernel/lock.h index 256b2e66fc..6c8bd5f023 100644 --- a/headers/private/kernel/lock.h +++ b/headers/private/kernel/lock.h @@ -72,23 +72,27 @@ typedef struct rw_lock { #if KDEBUG # define KDEBUG_RW_LOCK_DEBUG 0 // Define to 1 if you want to use ASSERT_READ_LOCKED_RW_LOCK(). - // The rw_lock will just behave like a recursive locker then. # define ASSERT_LOCKED_RECURSIVE(r) \ { ASSERT(find_thread(NULL) == (r)->lock.holder); } # define ASSERT_LOCKED_MUTEX(m) { ASSERT(find_thread(NULL) == (m)->holder); } # define ASSERT_WRITE_LOCKED_RW_LOCK(l) \ { ASSERT(find_thread(NULL) == (l)->holder); } # if KDEBUG_RW_LOCK_DEBUG + extern bool _rw_lock_is_read_locked(rw_lock* lock); # define ASSERT_READ_LOCKED_RW_LOCK(l) \ - { ASSERT(find_thread(NULL) == (l)->holder); } + { ASSERT_PRINT(_rw_lock_is_read_locked(l), "rwlock %p", l); } +# define ASSERT_UNLOCKED_RW_LOCK(l) \ + { ASSERT_PRINT(!_rw_lock_is_read_locked(l), "rwlock %p", l); } # else # define ASSERT_READ_LOCKED_RW_LOCK(l) do {} while (false) +# define ASSERT_UNLOCKED_RW_LOCK(l) do {} while (false) # endif #else # define ASSERT_LOCKED_RECURSIVE(r) do {} while (false) # define ASSERT_LOCKED_MUTEX(m) do {} while (false) # define ASSERT_WRITE_LOCKED_RW_LOCK(m) do {} while (false) # define ASSERT_READ_LOCKED_RW_LOCK(l) do {} while (false) +# define ASSERT_UNLOCKED_RW_LOCK(l) do {} while (false) #endif @@ -180,7 +184,7 @@ static inline status_t rw_lock_read_lock(rw_lock* lock) { #if KDEBUG_RW_LOCK_DEBUG - return rw_lock_write_lock(lock); + return _rw_lock_read_lock(lock); #else int32 oldCount = atomic_add(&lock->count, 1); if (oldCount >= RW_LOCK_WRITER_COUNT_BASE) @@ -195,7 +199,7 @@ rw_lock_read_lock_with_timeout(rw_lock* lock, uint32 timeoutFlags, bigtime_t timeout) { #if KDEBUG_RW_LOCK_DEBUG - return mutex_lock_with_timeout(lock, timeoutFlags, timeout); + return _rw_lock_read_lock_with_timeout(lock, timeoutFlags, timeout); #else int32 oldCount = atomic_add(&lock->count, 1); if (oldCount >= RW_LOCK_WRITER_COUNT_BASE) @@ -209,7 +213,7 @@ static inline void rw_lock_read_unlock(rw_lock* lock) { #if KDEBUG_RW_LOCK_DEBUG - rw_lock_write_unlock(lock); + _rw_lock_read_unlock(lock); #else int32 oldCount = atomic_add(&lock->count, -1); if (oldCount >= RW_LOCK_WRITER_COUNT_BASE) diff --git a/headers/private/kernel/thread_types.h b/headers/private/kernel/thread_types.h index b5b95c952c..84bc006c5f 100644 --- a/headers/private/kernel/thread_types.h +++ b/headers/private/kernel/thread_types.h @@ -547,6 +547,10 @@ struct Thread : TeamThreadIteratorEntry, KernelReferenceable { void (*post_interrupt_callback)(void*); void* post_interrupt_data; +#if KDEBUG_RW_LOCK_DEBUG + rw_lock* held_read_locks[64] = {}; // only modified by this thread +#endif + // architecture dependent section struct arch_thread arch_info; diff --git a/src/system/kernel/locks/lock.cpp b/src/system/kernel/locks/lock.cpp index 3488fee626..8f29f05bcf 100644 --- a/src/system/kernel/locks/lock.cpp +++ b/src/system/kernel/locks/lock.cpp @@ -441,7 +441,53 @@ rw_lock_destroy(rw_lock* lock) } -#if !KDEBUG_RW_LOCK_DEBUG +#if KDEBUG_RW_LOCK_DEBUG + +bool +_rw_lock_is_read_locked(rw_lock* lock) +{ + if (lock->holder == thread_get_current_thread_id()) + return true; + + Thread* thread = thread_get_current_thread(); + for (size_t i = 0; i < B_COUNT_OF(Thread::held_read_locks); i++) { + if (thread->held_read_locks[i] == lock) + return true; + } + return false; +} + + +static void +_rw_lock_set_read_locked(rw_lock* lock) +{ + Thread* thread = thread_get_current_thread(); + for (size_t i = 0; i < B_COUNT_OF(Thread::held_read_locks); i++) { + if (thread->held_read_locks[i] != NULL) + continue; + + thread->held_read_locks[i] = lock; + return; + } + panic("too many read locks!"); +} + + +static void +_rw_lock_unset_read_locked(rw_lock* lock) +{ + Thread* thread = thread_get_current_thread(); + for (size_t i = 0; i < B_COUNT_OF(Thread::held_read_locks); i++) { + if (thread->held_read_locks[i] != lock) + continue; + + thread->held_read_locks[i] = NULL; + return; + } +} + +#endif + status_t _rw_lock_read_lock(rw_lock* lock) @@ -452,6 +498,14 @@ _rw_lock_read_lock(rw_lock* lock) lock); } #endif +#if KDEBUG_RW_LOCK_DEBUG + int32 oldCount = atomic_add(&lock->count, 1); + if (oldCount < RW_LOCK_WRITER_COUNT_BASE) { + ASSERT_UNLOCKED_RW_LOCK(lock); + _rw_lock_set_read_locked(lock); + return B_OK; + } +#endif InterruptsSpinLocker locker(lock->lock); @@ -461,6 +515,8 @@ _rw_lock_read_lock(rw_lock* lock) return B_OK; } + ASSERT_UNLOCKED_RW_LOCK(lock); + // The writer that originally had the lock when we called atomic_add() might // already have gone and another writer could have overtaken us. In this // case the original writer set pending_readers, so we know that we don't @@ -471,13 +527,23 @@ _rw_lock_read_lock(rw_lock* lock) if (lock->count >= RW_LOCK_WRITER_COUNT_BASE) lock->active_readers++; +#if KDEBUG_RW_LOCK_DEBUG + _rw_lock_set_read_locked(lock); +#endif return B_OK; } ASSERT(lock->count >= RW_LOCK_WRITER_COUNT_BASE); // we need to wait - return rw_lock_wait(lock, false, locker); + status_t status = rw_lock_wait(lock, false, locker); + +#if KDEBUG_RW_LOCK_DEBUG + if (status == B_OK) + _rw_lock_set_read_locked(lock); +#endif + + return status; } @@ -491,6 +557,14 @@ _rw_lock_read_lock_with_timeout(rw_lock* lock, uint32 timeoutFlags, "disabled for lock %p", lock); } #endif +#if KDEBUG_RW_LOCK_DEBUG + int32 oldCount = atomic_add(&lock->count, 1); + if (oldCount < RW_LOCK_WRITER_COUNT_BASE) { + ASSERT_UNLOCKED_RW_LOCK(lock); + _rw_lock_set_read_locked(lock); + return B_OK; + } +#endif InterruptsSpinLocker locker(lock->lock); @@ -500,6 +574,8 @@ _rw_lock_read_lock_with_timeout(rw_lock* lock, uint32 timeoutFlags, return B_OK; } + ASSERT_UNLOCKED_RW_LOCK(lock); + // The writer that originally had the lock when we called atomic_add() might // already have gone and another writer could have overtaken us. In this // case the original writer set pending_readers, so we know that we don't @@ -510,6 +586,9 @@ _rw_lock_read_lock_with_timeout(rw_lock* lock, uint32 timeoutFlags, if (lock->count >= RW_LOCK_WRITER_COUNT_BASE) lock->active_readers++; +#if KDEBUG_RW_LOCK_DEBUG + _rw_lock_set_read_locked(lock); +#endif return B_OK; } @@ -538,6 +617,9 @@ _rw_lock_read_lock_with_timeout(rw_lock* lock, uint32 timeoutFlags, if (error == B_OK || waiter.thread == NULL) { // We were unblocked successfully -- potentially our unblocker overtook // us after we already failed. In either case, we've got the lock, now. +#if KDEBUG_RW_LOCK_DEBUG + _rw_lock_set_read_locked(lock); +#endif return B_OK; } @@ -586,6 +668,14 @@ _rw_lock_read_unlock(rw_lock* lock) return; } +#if KDEBUG_RW_LOCK_DEBUG + _rw_lock_unset_read_locked(lock); + + int32 oldCount = atomic_add(&lock->count, -1); + if (oldCount < RW_LOCK_WRITER_COUNT_BASE) + return; +#endif + if (--lock->active_readers > 0) return; @@ -598,8 +688,6 @@ _rw_lock_read_unlock(rw_lock* lock) rw_lock_unblock(lock); } -#endif // !KDEBUG_RW_LOCK_DEBUG - status_t rw_lock_write_lock(rw_lock* lock) @@ -621,6 +709,8 @@ rw_lock_write_lock(rw_lock* lock) return B_OK; } + ASSERT_UNLOCKED_RW_LOCK(lock); + // announce our claim int32 oldCount = atomic_add(&lock->count, RW_LOCK_WRITER_COUNT_BASE); @@ -668,6 +758,11 @@ _rw_lock_write_unlock(rw_lock* lock) lock->holder = -1; lock->owner_count = 0; +#if KDEBUG_RW_LOCK_DEBUG + if (readerCount != 0) + _rw_lock_set_read_locked(lock); +#endif + int32 oldCount = atomic_add(&lock->count, -RW_LOCK_WRITER_COUNT_BASE); oldCount -= RW_LOCK_WRITER_COUNT_BASE;