From 8dae411c1efbf28fbd76c3ed0db66ffec475f2dc Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Sun, 4 Aug 2019 19:08:03 -0400 Subject: [PATCH] kernel/locks: Tweak mutex_unlock KDEBUG semantics slightly. Previously, both the "old" owner and the "new" lock owner set the lock holder to the "new" lock holder. Now the old owner does it before calling unblock(), and we check in the lock() functions that we are indeed the new owner of the lock. This may affect what the panic is for #15211 and its duplicates: I have a suspicion that these threads are getting unblocked when they have no business being unblocked, and this should catch that condition. --- src/system/kernel/locks/lock.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/system/kernel/locks/lock.cpp b/src/system/kernel/locks/lock.cpp index db0f0dc1e0..488d413865 100644 --- a/src/system/kernel/locks/lock.cpp +++ b/src/system/kernel/locks/lock.cpp @@ -781,8 +781,9 @@ _mutex_lock(mutex* lock, void* _locker) status_t error = thread_block(); #if KDEBUG - if (error == B_OK) - atomic_set(&lock->holder, waiter.thread->id); + if (error == B_OK) { + ASSERT(lock->holder == waiter.thread->id); + } #endif return error; } @@ -813,23 +814,19 @@ _mutex_unlock(mutex* lock) lock->waiters = waiter->next; if (lock->waiters != NULL) lock->waiters->last = waiter->last; -#if KDEBUG - thread_id unblockedThread = waiter->thread->id; -#endif - - // unblock thread - thread_unblock(waiter->thread, B_OK); #if KDEBUG // Already set the holder to the unblocked thread. Besides that this // actually reflects the current situation, setting it to -1 would // cause a race condition, since another locker could think the lock // is not held by anyone. - lock->holder = unblockedThread; + lock->holder = waiter->thread->id; #endif + + // unblock thread + thread_unblock(waiter->thread, B_OK); } else { - // We've acquired the spinlock before the locker that is going to wait. - // Just mark the lock as released. + // Nobody is waiting to acquire this lock. Just mark it as released. #if KDEBUG lock->holder = -1; #else @@ -907,7 +904,7 @@ _mutex_lock_with_timeout(mutex* lock, uint32 timeoutFlags, bigtime_t timeout) if (error == B_OK) { #if KDEBUG - lock->holder = waiter.thread->id; + ASSERT(lock->holder == waiter.thread->id); #endif } else { locker.Lock();