diff --git a/src/system/kernel/locks/user_mutex.cpp b/src/system/kernel/locks/user_mutex.cpp index 5900a25289..215ddd50d6 100644 --- a/src/system/kernel/locks/user_mutex.cpp +++ b/src/system/kernel/locks/user_mutex.cpp @@ -1,4 +1,5 @@ /* + * Copyright 2023, Haiku, Inc. All rights reserved. * Copyright 2018, Jérôme Duval, jerome.duval@gmail.com. * Copyright 2015, Hamish Morrison, hamishm53@gmail.com. * Copyright 2010, Ingo Weinhold, ingo_weinhold@gmx.de. @@ -20,11 +21,19 @@ #include +/*! One UserMutexEntry corresponds to one mutex address. + * + * The mutex's "waiting" state is controlled by the rw_lock: a waiter acquires + * a "read" lock before initiating a wait, and an unblocker acquires a "write" + * lock. That way, unblockers can be sure that no waiters will start waiting + * during unblock, and they can thus safely (without races) unset WAITING. + */ struct UserMutexEntry { phys_addr_t address; UserMutexEntry* hash_next; int32 ref_count; + rw_lock lock; ConditionVariable condition; }; @@ -57,7 +66,7 @@ typedef BOpenHashTable UserMutexTable; static UserMutexTable sUserMutexTable; -static mutex sUserMutexTableLock = MUTEX_INITIALIZER("user mutex table"); +static rw_lock sUserMutexTableLock = RW_LOCK_INITIALIZER("user mutex table"); // #pragma mark - user atomics @@ -107,9 +116,23 @@ user_atomic_test_and_set(int32* value, int32 newValue, int32 testAgainst) static UserMutexEntry* -get_user_mutex_entry(phys_addr_t address) +get_user_mutex_entry(phys_addr_t address, bool noInsert = false, bool alreadyLocked = false) { + ReadLocker tableReadLocker; + if (!alreadyLocked) + tableReadLocker.SetTo(sUserMutexTableLock, false); + UserMutexEntry* entry = sUserMutexTable.Lookup(address); + if (entry != NULL) { + atomic_add(&entry->ref_count, 1); + return entry; + } else if (noInsert) + return entry; + + tableReadLocker.Unlock(); + WriteLocker tableWriteLocker(sUserMutexTableLock); + + entry = sUserMutexTable.Lookup(address); if (entry != NULL) { atomic_add(&entry->ref_count, 1); return entry; @@ -121,6 +144,7 @@ get_user_mutex_entry(phys_addr_t address) entry->address = address; entry->ref_count = 1; + rw_lock_init(&entry->lock, "UserMutexEntry lock"); entry->condition.Init(entry, "UserMutexEntry"); sUserMutexTable.Insert(entry); @@ -129,13 +153,16 @@ get_user_mutex_entry(phys_addr_t address) static void -put_user_mutex_entry(UserMutexEntry* entry, MutexLocker& locker) +put_user_mutex_entry(UserMutexEntry* entry) { + if (entry == NULL) + return; + const phys_addr_t address = entry->address; if (atomic_add(&entry->ref_count, -1) != 1) return; - locker.Lock(); + WriteLocker tableWriteLocker(sUserMutexTableLock); // Was it removed & deleted while we were waiting for the lock? if (sUserMutexTable.Lookup(address) != entry) @@ -146,43 +173,43 @@ put_user_mutex_entry(UserMutexEntry* entry, MutexLocker& locker) return; sUserMutexTable.Remove(entry); + tableWriteLocker.Unlock(); + + rw_lock_destroy(&entry->lock); delete entry; } static status_t -user_mutex_wait_locked(int32* mutex, phys_addr_t physicalAddress, const char* name, - uint32 flags, bigtime_t timeout, MutexLocker& locker) +user_mutex_wait_locked(UserMutexEntry* entry, + uint32 flags, bigtime_t timeout, ReadLocker& locker) { - // add or get the entry from the table - UserMutexEntry* entry = get_user_mutex_entry(physicalAddress); - if (entry == NULL) - return B_NO_MEMORY; - - // wait ConditionVariableEntry waiter; entry->condition.Add(&waiter); locker.Unlock(); - status_t error = waiter.Wait(flags, timeout); - - // this will re-lock only if necessary - put_user_mutex_entry(entry, locker); - - return error; + return waiter.Wait(flags, timeout); } static bool -user_mutex_prepare_to_lock(int32* mutex) +user_mutex_prepare_to_lock(UserMutexEntry* entry, int32* mutex) { + ASSERT_READ_LOCKED_RW_LOCK(&entry->lock); + int32 oldValue = user_atomic_or(mutex, B_USER_MUTEX_LOCKED | B_USER_MUTEX_WAITING); if ((oldValue & B_USER_MUTEX_LOCKED) == 0 || (oldValue & B_USER_MUTEX_DISABLED) != 0) { - // clear the waiting flag and be done - if ((oldValue & B_USER_MUTEX_WAITING) == 0) - user_atomic_and(mutex, ~(int32)B_USER_MUTEX_WAITING); + // possibly unset waiting flag + if ((oldValue & B_USER_MUTEX_WAITING) == 0) { + rw_lock_read_unlock(&entry->lock); + rw_lock_write_lock(&entry->lock); + if (entry->condition.EntriesCount() == 0) + user_atomic_and(mutex, ~(int32)B_USER_MUTEX_WAITING); + rw_lock_write_unlock(&entry->lock); + rw_lock_read_lock(&entry->lock); + } return true; } @@ -191,23 +218,31 @@ user_mutex_prepare_to_lock(int32* mutex) static status_t -user_mutex_lock_locked(int32* mutex, phys_addr_t physicalAddress, - const char* name, uint32 flags, bigtime_t timeout, MutexLocker& locker) +user_mutex_lock_locked(UserMutexEntry* entry, int32* mutex, + uint32 flags, bigtime_t timeout, ReadLocker& locker) { - if (user_mutex_prepare_to_lock(mutex)) + if (user_mutex_prepare_to_lock(entry, mutex)) return B_OK; - return user_mutex_wait_locked(mutex, physicalAddress, name, - flags, timeout, locker); + status_t error = user_mutex_wait_locked(entry, flags, timeout, locker); + + // possibly unset waiting flag + if (error != B_OK && entry->condition.EntriesCount() == 0) { + WriteLocker writeLocker(entry->lock); + if (entry->condition.EntriesCount() == 0) + user_atomic_and(mutex, ~(int32)B_USER_MUTEX_WAITING); + } + + return error; } static void -user_mutex_unblock_locked(int32* mutex, phys_addr_t physicalAddress, uint32 flags) +user_mutex_unblock(UserMutexEntry* entry, int32* mutex, uint32 flags) { - UserMutexEntry* entry = sUserMutexTable.Lookup(physicalAddress); - if (entry == NULL) { - // no one is waiting + WriteLocker entryLocker(entry->lock); + if (entry->condition.EntriesCount() == 0) { + // Nobody is actually waiting at present. user_atomic_and(mutex, ~(int32)B_USER_MUTEX_WAITING); return; } @@ -234,8 +269,8 @@ user_mutex_unblock_locked(int32* mutex, phys_addr_t physicalAddress, uint32 flag static status_t -user_mutex_sem_acquire_locked(int32* sem, phys_addr_t physicalAddress, - const char* name, uint32 flags, bigtime_t timeout, MutexLocker& locker) +user_mutex_sem_acquire_locked(UserMutexEntry* entry, int32* sem, + uint32 flags, bigtime_t timeout, ReadLocker& locker) { // The semaphore may have been released in the meantime, and we also // need to mark it as contended if it isn't already. @@ -247,16 +282,15 @@ user_mutex_sem_acquire_locked(int32* sem, phys_addr_t physicalAddress, oldValue = value; } - return user_mutex_wait_locked(sem, physicalAddress, name, flags, + return user_mutex_wait_locked(entry, flags, timeout, locker); } static void -user_mutex_sem_release_locked(int32* sem, phys_addr_t physicalAddress) +user_mutex_sem_release(UserMutexEntry* entry, int32* sem) { - UserMutexEntry* entry = sUserMutexTable.Lookup(physicalAddress); - if (!entry) { + if (entry == NULL) { // no waiters - mark as uncontended and release int32 oldValue = user_atomic_get(sem); while (true) { @@ -268,6 +302,7 @@ user_mutex_sem_release_locked(int32* sem, phys_addr_t physicalAddress) } } + WriteLocker entryLocker(entry->lock); entry->condition.NotifyOne(B_OK); if (entry->condition.EntriesCount() == 0) { // mark the semaphore uncontended @@ -287,11 +322,15 @@ user_mutex_lock(int32* mutex, const char* name, uint32 flags, bigtime_t timeout) return error; // get the lock + UserMutexEntry* entry = get_user_mutex_entry(wiringInfo.physicalAddress); + if (entry == NULL) + return B_NO_MEMORY; { - MutexLocker locker(sUserMutexTableLock); - error = user_mutex_lock_locked(mutex, wiringInfo.physicalAddress, name, - flags, timeout, locker); + ReadLocker entryLocker(entry->lock); + error = user_mutex_lock_locked(entry, mutex, + flags, timeout, entryLocker); } + put_user_mutex_entry(entry); // unwire the page vm_unwire_page(&wiringInfo); @@ -319,19 +358,32 @@ user_mutex_switch_lock(int32* fromMutex, int32* toMutex, const char* name, } // unlock the first mutex and lock the second one + UserMutexEntry* fromEntry = NULL, + *toEntry = get_user_mutex_entry(toWiringInfo.physicalAddress); + if (toEntry == NULL) + return B_NO_MEMORY; { - MutexLocker locker(sUserMutexTableLock); + ConditionVariableEntry waiter; - const bool alreadyLocked = user_mutex_prepare_to_lock(toMutex); - user_atomic_and(fromMutex, ~(int32)B_USER_MUTEX_LOCKED); - user_mutex_unblock_locked(fromMutex, fromWiringInfo.physicalAddress, - flags); - - if (!alreadyLocked) { - error = user_mutex_lock_locked(toMutex, toWiringInfo.physicalAddress, - name, flags, timeout, locker); + bool alreadyLocked = false; + { + ReadLocker entryLocker(toEntry->lock); + alreadyLocked = user_mutex_prepare_to_lock(toEntry, toMutex); + if (!alreadyLocked) + toEntry->condition.Add(&waiter); } + + const int32 oldValue = user_atomic_and(fromMutex, ~(int32)B_USER_MUTEX_LOCKED); + if ((oldValue & B_USER_MUTEX_WAITING) != 0) + fromEntry = get_user_mutex_entry(fromWiringInfo.physicalAddress, true); + if (fromEntry != NULL) + user_mutex_unblock(fromEntry, fromMutex, flags); + + if (!alreadyLocked) + error = waiter.Wait(flags, timeout); } + put_user_mutex_entry(fromEntry); + put_user_mutex_entry(toEntry); // unwire the pages vm_unwire_page(&toWiringInfo); @@ -384,10 +436,18 @@ _user_mutex_unblock(int32* mutex, uint32 flags) if (error != B_OK) return error; - { - MutexLocker locker(sUserMutexTableLock); - user_mutex_unblock_locked(mutex, wiringInfo.physicalAddress, flags); + // In the case where there is no entry, we must hold the read lock until we + // unset WAITING, because otherwise some other thread could initiate a wait. + ReadLocker tableReadLocker(sUserMutexTableLock); + UserMutexEntry* entry = get_user_mutex_entry(wiringInfo.physicalAddress, true, true); + if (entry == NULL) { + user_atomic_and(mutex, ~(int32)B_USER_MUTEX_WAITING); + tableReadLocker.Unlock(); + } else { + tableReadLocker.Unlock(); + user_mutex_unblock(entry, mutex, flags); } + put_user_mutex_entry(entry); vm_unwire_page(&wiringInfo); @@ -426,11 +486,15 @@ _user_mutex_sem_acquire(int32* sem, const char* name, uint32 flags, if (error != B_OK) return error; + UserMutexEntry* entry = get_user_mutex_entry(wiringInfo.physicalAddress); + if (entry == NULL) + return B_NO_MEMORY; { - MutexLocker locker(sUserMutexTableLock); - error = user_mutex_sem_acquire_locked(sem, wiringInfo.physicalAddress, - name, flags | B_CAN_INTERRUPT, timeout, locker); + ReadLocker entryLocker(entry->lock); + error = user_mutex_sem_acquire_locked(entry, sem, + flags | B_CAN_INTERRUPT, timeout, entryLocker); } + put_user_mutex_entry(entry); vm_unwire_page(&wiringInfo); return syscall_restart_handle_timeout_post(error, timeout); @@ -450,10 +514,11 @@ _user_mutex_sem_release(int32* sem) if (error != B_OK) return error; + UserMutexEntry* entry = get_user_mutex_entry(wiringInfo.physicalAddress, true); { - MutexLocker locker(sUserMutexTableLock); - user_mutex_sem_release_locked(sem, wiringInfo.physicalAddress); + user_mutex_sem_release(entry, sem); } + put_user_mutex_entry(entry); vm_unwire_page(&wiringInfo); return B_OK;