diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt index c1a645460..b3807c204 100644 --- a/src/core/CMakeLists.txt +++ b/src/core/CMakeLists.txt @@ -42,8 +42,6 @@ add_library(core STATIC hle/kernel/client_port.h hle/kernel/client_session.cpp hle/kernel/client_session.h - hle/kernel/condition_variable.cpp - hle/kernel/condition_variable.h hle/kernel/errors.h hle/kernel/event.cpp hle/kernel/event.h diff --git a/src/core/hle/kernel/condition_variable.cpp b/src/core/hle/kernel/condition_variable.cpp deleted file mode 100644 index a786d7f74..000000000 --- a/src/core/hle/kernel/condition_variable.cpp +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2018 yuzu emulator team -// Licensed under GPLv2 or any later version -// Refer to the license.txt file included. - -#include "common/assert.h" -#include "core/hle/kernel/condition_variable.h" -#include "core/hle/kernel/errors.h" -#include "core/hle/kernel/kernel.h" -#include "core/hle/kernel/object_address_table.h" -#include "core/hle/kernel/thread.h" - -namespace Kernel { - -ConditionVariable::ConditionVariable() {} -ConditionVariable::~ConditionVariable() {} - -ResultVal> ConditionVariable::Create(VAddr guest_addr, - std::string name) { - SharedPtr condition_variable(new ConditionVariable); - - condition_variable->name = std::move(name); - condition_variable->guest_addr = guest_addr; - condition_variable->mutex_addr = 0; - - // Condition variables are referenced by guest address, so track this in the kernel - g_object_address_table.Insert(guest_addr, condition_variable); - - return MakeResult>(std::move(condition_variable)); -} - -bool ConditionVariable::ShouldWait(Thread* thread) const { - return GetAvailableCount() <= 0; -} - -void ConditionVariable::Acquire(Thread* thread) { - if (GetAvailableCount() <= 0) - return; - - SetAvailableCount(GetAvailableCount() - 1); -} - -ResultCode ConditionVariable::Release(s32 target) { - if (target == -1) { - // When -1, wake up all waiting threads - SetAvailableCount(static_cast(GetWaitingThreads().size())); - WakeupAllWaitingThreads(); - } else { - // Otherwise, wake up just a single thread - SetAvailableCount(target); - WakeupWaitingThread(GetHighestPriorityReadyThread()); - } - - return RESULT_SUCCESS; -} - -s32 ConditionVariable::GetAvailableCount() const { - return Memory::Read32(guest_addr); -} - -void ConditionVariable::SetAvailableCount(s32 value) const { - Memory::Write32(guest_addr, value); -} - -} // namespace Kernel diff --git a/src/core/hle/kernel/condition_variable.h b/src/core/hle/kernel/condition_variable.h deleted file mode 100644 index 1c9f06769..000000000 --- a/src/core/hle/kernel/condition_variable.h +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright 2018 yuzu emulator team -// Licensed under GPLv2 or any later version -// Refer to the license.txt file included. - -#pragma once - -#include -#include -#include "common/common_types.h" -#include "core/hle/kernel/kernel.h" -#include "core/hle/kernel/wait_object.h" -#include "core/hle/result.h" - -namespace Kernel { - -class ConditionVariable final : public WaitObject { -public: - /** - * Creates a condition variable. - * @param guest_addr Address of the object tracking the condition variable in guest memory. If - * specified, this condition variable will update the guest object when its state changes. - * @param name Optional name of condition variable. - * @return The created condition variable. - */ - static ResultVal> Create(VAddr guest_addr, - std::string name = "Unknown"); - - std::string GetTypeName() const override { - return "ConditionVariable"; - } - std::string GetName() const override { - return name; - } - - static const HandleType HANDLE_TYPE = HandleType::ConditionVariable; - HandleType GetHandleType() const override { - return HANDLE_TYPE; - } - - s32 GetAvailableCount() const; - void SetAvailableCount(s32 value) const; - - std::string name; ///< Name of condition variable (optional) - VAddr guest_addr; ///< Address of the guest condition variable value - VAddr mutex_addr; ///< (optional) Address of guest mutex value associated with this condition - ///< variable, used for implementing events - - bool ShouldWait(Thread* thread) const override; - void Acquire(Thread* thread) override; - - /** - * Releases a slot from a condition variable. - * @param target The number of threads to wakeup, -1 is all. - * @return ResultCode indicating if the operation succeeded. - */ - ResultCode Release(s32 target); - -private: - ConditionVariable(); - ~ConditionVariable() override; -}; - -} // namespace Kernel diff --git a/src/core/hle/kernel/errors.h b/src/core/hle/kernel/errors.h index 29d8dfdaa..5be20c878 100644 --- a/src/core/hle/kernel/errors.h +++ b/src/core/hle/kernel/errors.h @@ -20,6 +20,7 @@ enum { MaxConnectionsReached = 52, // Confirmed Switch OS error codes + MisalignedAddress = 102, InvalidHandle = 114, Timeout = 117, SynchronizationCanceled = 118, diff --git a/src/core/hle/kernel/kernel.h b/src/core/hle/kernel/kernel.h index 053bf4e17..402ae900f 100644 --- a/src/core/hle/kernel/kernel.h +++ b/src/core/hle/kernel/kernel.h @@ -18,12 +18,10 @@ using Handle = u32; enum class HandleType : u32 { Unknown, Event, - Mutex, SharedMemory, Thread, Process, AddressArbiter, - ConditionVariable, Timer, ResourceLimit, CodeSet, @@ -63,9 +61,7 @@ public: bool IsWaitable() const { switch (GetHandleType()) { case HandleType::Event: - case HandleType::Mutex: case HandleType::Thread: - case HandleType::ConditionVariable: case HandleType::Timer: case HandleType::ServerPort: case HandleType::ServerSession: diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index 0b9dc700c..63733ad79 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/src/core/hle/kernel/mutex.cpp @@ -7,6 +7,7 @@ #include #include "common/assert.h" #include "core/core.h" +#include "core/hle/kernel/errors.h" #include "core/hle/kernel/handle_table.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/mutex.h" @@ -15,124 +16,120 @@ namespace Kernel { -void ReleaseThreadMutexes(Thread* thread) { - for (auto& mtx : thread->held_mutexes) { - mtx->SetHasWaiters(false); - mtx->SetHoldingThread(nullptr); - mtx->WakeupAllWaitingThreads(); - } - thread->held_mutexes.clear(); -} +/// Returns the number of threads that are waiting for a mutex, and the highest priority one among +/// those. +static std::pair, u32> GetHighestPriorityMutexWaitingThread( + SharedPtr current_thread, VAddr mutex_addr) { -Mutex::Mutex() {} -Mutex::~Mutex() {} + SharedPtr highest_priority_thread; + u32 num_waiters = 0; -SharedPtr Mutex::Create(SharedPtr holding_thread, VAddr guest_addr, - std::string name) { - SharedPtr mutex(new Mutex); + for (auto& thread : current_thread->wait_mutex_threads) { + if (thread->mutex_wait_address != mutex_addr) + continue; - mutex->guest_addr = guest_addr; - mutex->name = std::move(name); + ASSERT(thread->status == THREADSTATUS_WAIT_MUTEX); - // If mutex was initialized with a holding thread, acquire it by the holding thread - if (holding_thread) { - mutex->Acquire(holding_thread.get()); + ++num_waiters; + if (highest_priority_thread == nullptr || + thread->GetPriority() < highest_priority_thread->GetPriority()) { + highest_priority_thread = thread; + } } - // Mutexes are referenced by guest address, so track this in the kernel - g_object_address_table.Insert(guest_addr, mutex); - - return mutex; + return {highest_priority_thread, num_waiters}; } -bool Mutex::ShouldWait(Thread* thread) const { - auto holding_thread = GetHoldingThread(); - return holding_thread != nullptr && thread != holding_thread; +/// Update the mutex owner field of all threads waiting on the mutex to point to the new owner. +static void TransferMutexOwnership(VAddr mutex_addr, SharedPtr current_thread, + SharedPtr new_owner) { + auto threads = current_thread->wait_mutex_threads; + for (auto& thread : threads) { + if (thread->mutex_wait_address != mutex_addr) + continue; + + ASSERT(thread->lock_owner == current_thread); + current_thread->RemoveMutexWaiter(thread); + if (new_owner != thread) + new_owner->AddMutexWaiter(thread); + } } -void Mutex::Acquire(Thread* thread) { - ASSERT_MSG(!ShouldWait(thread), "object unavailable!"); +ResultCode Mutex::TryAcquire(VAddr address, Handle holding_thread_handle, + Handle requesting_thread_handle) { + // The mutex address must be 4-byte aligned + if ((address % sizeof(u32)) != 0) { + return ResultCode(ErrorModule::Kernel, ErrCodes::MisalignedAddress); + } - priority = thread->current_priority; - thread->held_mutexes.insert(this); - SetHoldingThread(thread); - thread->UpdatePriority(); - Core::System::GetInstance().PrepareReschedule(); -} + SharedPtr holding_thread = g_handle_table.Get(holding_thread_handle); + SharedPtr requesting_thread = g_handle_table.Get(requesting_thread_handle); -ResultCode Mutex::Release(Thread* thread) { - auto holding_thread = GetHoldingThread(); - ASSERT(holding_thread); + // TODO(Subv): It is currently unknown if it is possible to lock a mutex in behalf of another + // thread. + ASSERT(requesting_thread == GetCurrentThread()); - // We can only release the mutex if it's held by the calling thread. - ASSERT(thread == holding_thread); + u32 addr_value = Memory::Read32(address); + + // If the mutex isn't being held, just return success. + if (addr_value != (holding_thread_handle | Mutex::MutexHasWaitersFlag)) { + return RESULT_SUCCESS; + } + + if (holding_thread == nullptr) + return ERR_INVALID_HANDLE; + + // Wait until the mutex is released + GetCurrentThread()->mutex_wait_address = address; + GetCurrentThread()->wait_handle = requesting_thread_handle; + + GetCurrentThread()->status = THREADSTATUS_WAIT_MUTEX; + GetCurrentThread()->wakeup_callback = nullptr; + + // Update the lock holder thread's priority to prevent priority inversion. + holding_thread->AddMutexWaiter(GetCurrentThread()); - holding_thread->held_mutexes.erase(this); - holding_thread->UpdatePriority(); - SetHoldingThread(nullptr); - SetHasWaiters(!GetWaitingThreads().empty()); - WakeupAllWaitingThreads(); Core::System::GetInstance().PrepareReschedule(); return RESULT_SUCCESS; } -void Mutex::AddWaitingThread(SharedPtr thread) { - WaitObject::AddWaitingThread(thread); - thread->pending_mutexes.insert(this); - SetHasWaiters(true); - UpdatePriority(); -} - -void Mutex::RemoveWaitingThread(Thread* thread) { - WaitObject::RemoveWaitingThread(thread); - thread->pending_mutexes.erase(this); - if (!GetHasWaiters()) - SetHasWaiters(!GetWaitingThreads().empty()); - UpdatePriority(); -} - -void Mutex::UpdatePriority() { - if (!GetHoldingThread()) - return; - - u32 best_priority = THREADPRIO_LOWEST; - for (auto& waiter : GetWaitingThreads()) { - if (waiter->current_priority < best_priority) - best_priority = waiter->current_priority; +ResultCode Mutex::Release(VAddr address) { + // The mutex address must be 4-byte aligned + if ((address % sizeof(u32)) != 0) { + return ResultCode(ErrorModule::Kernel, ErrCodes::MisalignedAddress); } - if (best_priority != priority) { - priority = best_priority; - GetHoldingThread()->UpdatePriority(); + auto [thread, num_waiters] = GetHighestPriorityMutexWaitingThread(GetCurrentThread(), address); + + // There are no more threads waiting for the mutex, release it completely. + if (thread == nullptr) { + ASSERT(GetCurrentThread()->wait_mutex_threads.empty()); + Memory::Write32(address, 0); + return RESULT_SUCCESS; } -} -Handle Mutex::GetOwnerHandle() const { - GuestState guest_state{Memory::Read32(guest_addr)}; - return guest_state.holding_thread_handle; -} + // Transfer the ownership of the mutex from the previous owner to the new one. + TransferMutexOwnership(address, GetCurrentThread(), thread); -SharedPtr Mutex::GetHoldingThread() const { - GuestState guest_state{Memory::Read32(guest_addr)}; - return g_handle_table.Get(guest_state.holding_thread_handle); -} + u32 mutex_value = thread->wait_handle; -void Mutex::SetHoldingThread(SharedPtr thread) { - GuestState guest_state{Memory::Read32(guest_addr)}; - guest_state.holding_thread_handle.Assign(thread ? thread->guest_handle : 0); - Memory::Write32(guest_addr, guest_state.raw); -} + if (num_waiters >= 2) { + // Notify the guest that there are still some threads waiting for the mutex + mutex_value |= Mutex::MutexHasWaitersFlag; + } -bool Mutex::GetHasWaiters() const { - GuestState guest_state{Memory::Read32(guest_addr)}; - return guest_state.has_waiters != 0; -} + // Grant the mutex to the next waiting thread and resume it. + Memory::Write32(address, mutex_value); -void Mutex::SetHasWaiters(bool has_waiters) { - GuestState guest_state{Memory::Read32(guest_addr)}; - guest_state.has_waiters.Assign(has_waiters ? 1 : 0); - Memory::Write32(guest_addr, guest_state.raw); -} + ASSERT(thread->status == THREADSTATUS_WAIT_MUTEX); + thread->ResumeFromWait(); + thread->lock_owner = nullptr; + thread->condvar_wait_address = 0; + thread->mutex_wait_address = 0; + thread->wait_handle = 0; + + return RESULT_SUCCESS; +} } // namespace Kernel diff --git a/src/core/hle/kernel/mutex.h b/src/core/hle/kernel/mutex.h index 38db21005..3117e7c70 100644 --- a/src/core/hle/kernel/mutex.h +++ b/src/core/hle/kernel/mutex.h @@ -15,87 +15,23 @@ namespace Kernel { class Thread; -class Mutex final : public WaitObject { +class Mutex final { public: - /** - * Creates a mutex. - * @param holding_thread Specifies a thread already holding the mutex. If not nullptr, this - * thread will acquire the mutex. - * @param guest_addr Address of the object tracking the mutex in guest memory. If specified, - * this mutex will update the guest object when its state changes. - * @param name Optional name of mutex - * @return Pointer to new Mutex object - */ - static SharedPtr Create(SharedPtr holding_thread, VAddr guest_addr = 0, - std::string name = "Unknown"); + /// Flag that indicates that a mutex still has threads waiting for it. + static constexpr u32 MutexHasWaitersFlag = 0x40000000; + /// Mask of the bits in a mutex address value that contain the mutex owner. + static constexpr u32 MutexOwnerMask = 0xBFFFFFFF; - std::string GetTypeName() const override { - return "Mutex"; - } - std::string GetName() const override { - return name; - } + /// Attempts to acquire a mutex at the specified address. + static ResultCode TryAcquire(VAddr address, Handle holding_thread_handle, + Handle requesting_thread_handle); - static const HandleType HANDLE_TYPE = HandleType::Mutex; - HandleType GetHandleType() const override { - return HANDLE_TYPE; - } - - u32 priority; ///< The priority of the mutex, used for priority inheritance. - std::string name; ///< Name of mutex (optional) - VAddr guest_addr; ///< Address of the guest mutex value - - /** - * Elevate the mutex priority to the best priority - * among the priorities of all its waiting threads. - */ - void UpdatePriority(); - - bool ShouldWait(Thread* thread) const override; - void Acquire(Thread* thread) override; - - void AddWaitingThread(SharedPtr thread) override; - void RemoveWaitingThread(Thread* thread) override; - - /** - * Attempts to release the mutex from the specified thread. - * @param thread Thread that wants to release the mutex. - * @returns The result code of the operation. - */ - ResultCode Release(Thread* thread); - - /// Gets the handle to the holding process stored in the guest state. - Handle GetOwnerHandle() const; - - /// Gets the Thread pointed to by the owner handle - SharedPtr GetHoldingThread() const; - /// Sets the holding process handle in the guest state. - void SetHoldingThread(SharedPtr thread); - - /// Returns the has_waiters bit in the guest state. - bool GetHasWaiters() const; - /// Sets the has_waiters bit in the guest state. - void SetHasWaiters(bool has_waiters); + /// Releases the mutex at the specified address. + static ResultCode Release(VAddr address); private: - Mutex(); - ~Mutex() override; - - /// Object in guest memory used to track the mutex state - union GuestState { - u32_le raw; - /// Handle of the thread that currently holds the mutex, 0 if available - BitField<0, 30, u32_le> holding_thread_handle; - /// 1 when there are threads waiting for this mutex, otherwise 0 - BitField<30, 1, u32_le> has_waiters; - }; - static_assert(sizeof(GuestState) == 4, "GuestState size is incorrect"); + Mutex() = default; + ~Mutex() = default; }; -/** - * Releases all the mutexes held by the specified thread - * @param thread Thread that is holding the mutexes - */ -void ReleaseThreadMutexes(Thread* thread); - } // namespace Kernel diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 633740992..c22da6e47 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -13,7 +13,6 @@ #include "core/core_timing.h" #include "core/hle/kernel/client_port.h" #include "core/hle/kernel/client_session.h" -#include "core/hle/kernel/condition_variable.h" #include "core/hle/kernel/event.h" #include "core/hle/kernel/handle_table.h" #include "core/hle/kernel/mutex.h" @@ -262,32 +261,14 @@ static ResultCode ArbitrateLock(Handle holding_thread_handle, VAddr mutex_addr, "requesting_current_thread_handle=0x%08X", holding_thread_handle, mutex_addr, requesting_thread_handle); - SharedPtr holding_thread = g_handle_table.Get(holding_thread_handle); - SharedPtr requesting_thread = g_handle_table.Get(requesting_thread_handle); - - ASSERT(requesting_thread); - ASSERT(requesting_thread == GetCurrentThread()); - - SharedPtr mutex = g_object_address_table.Get(mutex_addr); - if (!mutex) { - // Create a new mutex for the specified address if one does not already exist - mutex = Mutex::Create(holding_thread, mutex_addr); - mutex->name = Common::StringFromFormat("mutex-%llx", mutex_addr); - } - - ASSERT(holding_thread == mutex->GetHoldingThread()); - - return WaitSynchronization1(mutex, requesting_thread.get()); + return Mutex::TryAcquire(mutex_addr, holding_thread_handle, requesting_thread_handle); } /// Unlock a mutex static ResultCode ArbitrateUnlock(VAddr mutex_addr) { LOG_TRACE(Kernel_SVC, "called mutex_addr=0x%llx", mutex_addr); - SharedPtr mutex = g_object_address_table.Get(mutex_addr); - ASSERT(mutex); - - return mutex->Release(GetCurrentThread()); + return Mutex::Release(mutex_addr); } /// Break program execution @@ -412,11 +393,6 @@ static ResultCode SetThreadPriority(Handle handle, u32 priority) { } thread->SetPriority(priority); - thread->UpdatePriority(); - - // Update the mutexes that this thread is waiting for - for (auto& mutex : thread->pending_mutexes) - mutex->UpdatePriority(); Core::System::GetInstance().PrepareReschedule(); return RESULT_SUCCESS; @@ -634,77 +610,20 @@ static ResultCode WaitProcessWideKeyAtomic(VAddr mutex_addr, VAddr condition_var SharedPtr thread = g_handle_table.Get(thread_handle); ASSERT(thread); - SharedPtr mutex = g_object_address_table.Get(mutex_addr); - if (!mutex) { - // Create a new mutex for the specified address if one does not already exist - mutex = Mutex::Create(thread, mutex_addr); - mutex->name = Common::StringFromFormat("mutex-%llx", mutex_addr); - } + CASCADE_CODE(Mutex::Release(mutex_addr)); - SharedPtr condition_variable = - g_object_address_table.Get(condition_variable_addr); - if (!condition_variable) { - // Create a new condition_variable for the specified address if one does not already exist - condition_variable = ConditionVariable::Create(condition_variable_addr).Unwrap(); - condition_variable->name = - Common::StringFromFormat("condition-variable-%llx", condition_variable_addr); - } + SharedPtr current_thread = GetCurrentThread(); + current_thread->condvar_wait_address = condition_variable_addr; + current_thread->mutex_wait_address = mutex_addr; + current_thread->wait_handle = thread_handle; + current_thread->status = THREADSTATUS_WAIT_MUTEX; + current_thread->wakeup_callback = nullptr; - if (condition_variable->mutex_addr) { - // Previously created the ConditionVariable using WaitProcessWideKeyAtomic, verify - // everything is correct - ASSERT(condition_variable->mutex_addr == mutex_addr); - } else { - // Previously created the ConditionVariable using SignalProcessWideKey, set the mutex - // associated with it - condition_variable->mutex_addr = mutex_addr; - } + current_thread->WakeAfterDelay(nano_seconds); - if (mutex->GetOwnerHandle()) { - // Release the mutex if the current thread is holding it - mutex->Release(thread.get()); - } - - auto wakeup_callback = [mutex, nano_seconds](ThreadWakeupReason reason, - SharedPtr thread, - SharedPtr object, size_t index) { - ASSERT(thread->status == THREADSTATUS_WAIT_SYNCH_ANY); - - if (reason == ThreadWakeupReason::Timeout) { - thread->SetWaitSynchronizationResult(RESULT_TIMEOUT); - return true; - } - - ASSERT(reason == ThreadWakeupReason::Signal); - - // Now try to acquire the mutex and don't resume if it's not available. - if (!mutex->ShouldWait(thread.get())) { - mutex->Acquire(thread.get()); - thread->SetWaitSynchronizationResult(RESULT_SUCCESS); - return true; - } - - if (nano_seconds == 0) { - thread->SetWaitSynchronizationResult(RESULT_TIMEOUT); - return true; - } - - thread->wait_objects = {mutex}; - mutex->AddWaitingThread(thread); - thread->status = THREADSTATUS_WAIT_SYNCH_ANY; - - // Create an event to wake the thread up after the - // specified nanosecond delay has passed - thread->WakeAfterDelay(nano_seconds); - thread->wakeup_callback = DefaultThreadWakeupCallback; - - Core::System::GetInstance().PrepareReschedule(); - - return false; - }; - CASCADE_CODE( - WaitSynchronization1(condition_variable, thread.get(), nano_seconds, wakeup_callback)); + // Note: Deliberately don't attempt to inherit the lock owner's priority. + Core::System::GetInstance().PrepareReschedule(); return RESULT_SUCCESS; } @@ -713,24 +632,53 @@ static ResultCode SignalProcessWideKey(VAddr condition_variable_addr, s32 target LOG_TRACE(Kernel_SVC, "called, condition_variable_addr=0x%llx, target=0x%08x", condition_variable_addr, target); - // Wakeup all or one thread - Any other value is unimplemented - ASSERT(target == -1 || target == 1); + u32 processed = 0; + auto& thread_list = Core::System::GetInstance().Scheduler().GetThreadList(); - SharedPtr condition_variable = - g_object_address_table.Get(condition_variable_addr); - if (!condition_variable) { - // Create a new condition_variable for the specified address if one does not already exist - condition_variable = ConditionVariable::Create(condition_variable_addr).Unwrap(); - condition_variable->name = - Common::StringFromFormat("condition-variable-%llx", condition_variable_addr); - } + for (auto& thread : thread_list) { + if (thread->condvar_wait_address != condition_variable_addr) + continue; - CASCADE_CODE(condition_variable->Release(target)); + // Only process up to 'target' threads, unless 'target' is -1, in which case process + // them all. + if (target != -1 && processed >= target) + break; - if (condition_variable->mutex_addr) { - // If a mutex was created for this condition_variable, wait the current thread on it - SharedPtr mutex = g_object_address_table.Get(condition_variable->mutex_addr); - return WaitSynchronization1(mutex, GetCurrentThread()); + // If the mutex is not yet acquired, acquire it. + u32 mutex_val = Memory::Read32(thread->mutex_wait_address); + + if (mutex_val == 0) { + // We were able to acquire the mutex, resume this thread. + Memory::Write32(thread->mutex_wait_address, thread->wait_handle); + ASSERT(thread->status == THREADSTATUS_WAIT_MUTEX); + thread->ResumeFromWait(); + + auto lock_owner = thread->lock_owner; + if (lock_owner) + lock_owner->RemoveMutexWaiter(thread); + + thread->lock_owner = nullptr; + thread->mutex_wait_address = 0; + thread->condvar_wait_address = 0; + thread->wait_handle = 0; + } else { + // Couldn't acquire the mutex, block the thread. + Handle owner_handle = static_cast(mutex_val & Mutex::MutexOwnerMask); + auto owner = g_handle_table.Get(owner_handle); + ASSERT(owner); + ASSERT(thread->status != THREADSTATUS_RUNNING); + thread->status = THREADSTATUS_WAIT_MUTEX; + thread->wakeup_callback = nullptr; + + // Signal that the mutex now has a waiting thread. + Memory::Write32(thread->mutex_wait_address, mutex_val | Mutex::MutexHasWaitersFlag); + + owner->AddMutexWaiter(thread); + + Core::System::GetInstance().PrepareReschedule(); + } + + ++processed; } return RESULT_SUCCESS; diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index f3a8aa4aa..36222d45f 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -77,9 +77,6 @@ void Thread::Stop() { } wait_objects.clear(); - // Release all the mutexes that this thread holds - ReleaseThreadMutexes(this); - // Mark the TLS slot in the thread's page as free. u64 tls_page = (tls_address - Memory::TLS_AREA_VADDR) / Memory::PAGE_SIZE; u64 tls_slot = @@ -126,6 +123,19 @@ static void ThreadWakeupCallback(u64 thread_handle, int cycles_late) { resume = thread->wakeup_callback(ThreadWakeupReason::Timeout, thread, nullptr, 0); } + if (thread->mutex_wait_address != 0 || thread->condvar_wait_address != 0 || + thread->wait_handle) { + ASSERT(thread->status == THREADSTATUS_WAIT_MUTEX); + thread->mutex_wait_address = 0; + thread->condvar_wait_address = 0; + thread->wait_handle = 0; + + auto lock_owner = thread->lock_owner; + // Threads waking up by timeout from WaitProcessWideKey do not perform priority inheritance + // and don't have a lock owner. + ASSERT(lock_owner == nullptr); + } + if (resume) thread->ResumeFromWait(); } @@ -151,6 +161,7 @@ void Thread::ResumeFromWait() { case THREADSTATUS_WAIT_HLE_EVENT: case THREADSTATUS_WAIT_SLEEP: case THREADSTATUS_WAIT_IPC: + case THREADSTATUS_WAIT_MUTEX: break; case THREADSTATUS_READY: @@ -256,7 +267,9 @@ ResultVal> Thread::Create(std::string name, VAddr entry_point, thread->last_running_ticks = CoreTiming::GetTicks(); thread->processor_id = processor_id; thread->wait_objects.clear(); - thread->wait_address = 0; + thread->mutex_wait_address = 0; + thread->condvar_wait_address = 0; + thread->wait_handle = 0; thread->name = std::move(name); thread->callback_handle = wakeup_callback_handle_table.Create(thread).Unwrap(); thread->owner_process = owner_process; @@ -317,17 +330,8 @@ ResultVal> Thread::Create(std::string name, VAddr entry_point, void Thread::SetPriority(u32 priority) { ASSERT_MSG(priority <= THREADPRIO_LOWEST && priority >= THREADPRIO_HIGHEST, "Invalid priority value."); - Core::System::GetInstance().Scheduler().SetThreadPriority(this, priority); - nominal_priority = current_priority = priority; -} - -void Thread::UpdatePriority() { - u32 best_priority = nominal_priority; - for (auto& mutex : held_mutexes) { - if (mutex->priority < best_priority) - best_priority = mutex->priority; - } - BoostPriority(best_priority); + nominal_priority = priority; + UpdatePriority(); } void Thread::BoostPriority(u32 priority) { @@ -377,6 +381,38 @@ VAddr Thread::GetCommandBufferAddress() const { return GetTLSAddress() + CommandHeaderOffset; } +void Thread::AddMutexWaiter(SharedPtr thread) { + thread->lock_owner = this; + wait_mutex_threads.emplace_back(std::move(thread)); + UpdatePriority(); +} + +void Thread::RemoveMutexWaiter(SharedPtr thread) { + boost::remove_erase(wait_mutex_threads, thread); + thread->lock_owner = nullptr; + UpdatePriority(); +} + +void Thread::UpdatePriority() { + // Find the highest priority among all the threads that are waiting for this thread's lock + u32 new_priority = nominal_priority; + for (const auto& thread : wait_mutex_threads) { + if (thread->nominal_priority < new_priority) + new_priority = thread->nominal_priority; + } + + if (new_priority == current_priority) + return; + + Core::System::GetInstance().Scheduler().SetThreadPriority(this, new_priority); + + current_priority = new_priority; + + // Recursively update the priority of the thread that depends on the priority of this one. + if (lock_owner) + lock_owner->UpdatePriority(); +} + //////////////////////////////////////////////////////////////////////////////////////////////////// /** diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index dbf47e269..e0a3c0934 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -18,7 +18,7 @@ enum ThreadPriority : u32 { THREADPRIO_HIGHEST = 0, ///< Highest thread priority THREADPRIO_USERLAND_MAX = 24, ///< Highest thread priority for userland apps - THREADPRIO_DEFAULT = 48, ///< Default thread priority for userland apps + THREADPRIO_DEFAULT = 44, ///< Default thread priority for userland apps THREADPRIO_LOWEST = 63, ///< Lowest thread priority }; @@ -43,6 +43,7 @@ enum ThreadStatus { THREADSTATUS_WAIT_IPC, ///< Waiting for the reply from an IPC request THREADSTATUS_WAIT_SYNCH_ANY, ///< Waiting due to WaitSynch1 or WaitSynchN with wait_all = false THREADSTATUS_WAIT_SYNCH_ALL, ///< Waiting due to WaitSynchronizationN with wait_all = true + THREADSTATUS_WAIT_MUTEX, ///< Waiting due to an ArbitrateLock/WaitProcessWideKey svc THREADSTATUS_DORMANT, ///< Created but not yet made ready THREADSTATUS_DEAD ///< Run to completion, or forcefully terminated }; @@ -54,7 +55,6 @@ enum class ThreadWakeupReason { namespace Kernel { -class Mutex; class Process; class Thread final : public WaitObject { @@ -103,18 +103,21 @@ public: */ void SetPriority(u32 priority); - /** - * Boost's a thread's priority to the best priority among the thread's held mutexes. - * This prevents priority inversion via priority inheritance. - */ - void UpdatePriority(); - /** * Temporarily boosts the thread's priority until the next time it is scheduled * @param priority The new priority */ void BoostPriority(u32 priority); + /// Adds a thread to the list of threads that are waiting for a lock held by this thread. + void AddMutexWaiter(SharedPtr thread); + + /// Removes a thread from the list of threads that are waiting for a lock held by this thread. + void RemoveMutexWaiter(SharedPtr thread); + + /// Recalculates the current priority taking into account priority inheritance. + void UpdatePriority(); + /** * Gets the thread's thread ID * @return The thread's ID @@ -205,19 +208,22 @@ public: VAddr tls_address; ///< Virtual address of the Thread Local Storage of the thread - /// Mutexes currently held by this thread, which will be released when it exits. - boost::container::flat_set> held_mutexes; - - /// Mutexes that this thread is currently waiting for. - boost::container::flat_set> pending_mutexes; - SharedPtr owner_process; ///< Process that owns this thread /// Objects that the thread is waiting on, in the same order as they were // passed to WaitSynchronization1/N. std::vector> wait_objects; - VAddr wait_address; ///< If waiting on an AddressArbiter, this is the arbitration address + /// List of threads that are waiting for a mutex that is held by this thread. + std::vector> wait_mutex_threads; + + /// Thread that owns the lock that this thread is waiting for. + SharedPtr lock_owner; + + // If waiting on a ConditionVariable, this is the ConditionVariable address + VAddr condvar_wait_address; + VAddr mutex_wait_address; ///< If waiting on a Mutex, this is the mutex address + Handle wait_handle; ///< The handle used to wait for the mutex. std::string name; diff --git a/src/core/hle/service/nvflinger/buffer_queue.cpp b/src/core/hle/service/nvflinger/buffer_queue.cpp index 03a4fed59..e4ff2e267 100644 --- a/src/core/hle/service/nvflinger/buffer_queue.cpp +++ b/src/core/hle/service/nvflinger/buffer_queue.cpp @@ -9,7 +9,8 @@ #include "core/core_timing.h" #include "core/hle/service/nvflinger/buffer_queue.h" -namespace Service::NVFlinger { +namespace Service { +namespace NVFlinger { BufferQueue::BufferQueue(u32 id, u64 layer_id) : id(id), layer_id(layer_id) { native_handle = Kernel::Event::Create(Kernel::ResetType::OneShot, "BufferQueue NativeHandle"); @@ -110,4 +111,5 @@ void BufferQueue::SetBufferWaitEvent(Kernel::SharedPtr&& wait_eve buffer_wait_event = std::move(wait_event); } -} // namespace Service::NVFlinger +} // namespace NVFlinger +} // namespace Service diff --git a/src/core/hle/service/nvflinger/buffer_queue.h b/src/core/hle/service/nvflinger/buffer_queue.h index 95adc4706..1de5767cb 100644 --- a/src/core/hle/service/nvflinger/buffer_queue.h +++ b/src/core/hle/service/nvflinger/buffer_queue.h @@ -13,7 +13,8 @@ namespace CoreTiming { struct EventType; } -namespace Service::NVFlinger { +namespace Service { +namespace NVFlinger { struct IGBPBuffer { u32_le magic; @@ -97,4 +98,5 @@ private: Kernel::SharedPtr buffer_wait_event; }; -} // namespace Service::NVFlinger +} // namespace NVFlinger +} // namespace Service diff --git a/src/core/loader/nro.cpp b/src/core/loader/nro.cpp index b5133e4d6..3853cfa1a 100644 --- a/src/core/loader/nro.cpp +++ b/src/core/loader/nro.cpp @@ -137,7 +137,7 @@ ResultStatus AppLoader_NRO::Load(Kernel::SharedPtr& process) { process->address_mappings = default_address_mappings; process->resource_limit = Kernel::ResourceLimit::GetForCategory(Kernel::ResourceLimitCategory::APPLICATION); - process->Run(base_addr, 48, Memory::DEFAULT_STACK_SIZE); + process->Run(base_addr, THREADPRIO_DEFAULT, Memory::DEFAULT_STACK_SIZE); is_loaded = true; return ResultStatus::Success; diff --git a/src/core/loader/nso.cpp b/src/core/loader/nso.cpp index 3bc10ed0d..962bed2ab 100644 --- a/src/core/loader/nso.cpp +++ b/src/core/loader/nso.cpp @@ -165,7 +165,7 @@ ResultStatus AppLoader_NSO::Load(Kernel::SharedPtr& process) { process->address_mappings = default_address_mappings; process->resource_limit = Kernel::ResourceLimit::GetForCategory(Kernel::ResourceLimitCategory::APPLICATION); - process->Run(Memory::PROCESS_IMAGE_VADDR, 48, Memory::DEFAULT_STACK_SIZE); + process->Run(Memory::PROCESS_IMAGE_VADDR, THREADPRIO_DEFAULT, Memory::DEFAULT_STACK_SIZE); is_loaded = true; return ResultStatus::Success; diff --git a/src/yuzu/debugger/wait_tree.cpp b/src/yuzu/debugger/wait_tree.cpp index cae2864e5..acc4c2e0b 100644 --- a/src/yuzu/debugger/wait_tree.cpp +++ b/src/yuzu/debugger/wait_tree.cpp @@ -6,8 +6,8 @@ #include "yuzu/util/util.h" #include "core/core.h" -#include "core/hle/kernel/condition_variable.h" #include "core/hle/kernel/event.h" +#include "core/hle/kernel/handle_table.h" #include "core/hle/kernel/mutex.h" #include "core/hle/kernel/thread.h" #include "core/hle/kernel/timer.h" @@ -67,6 +67,29 @@ QString WaitTreeText::GetText() const { return text; } +WaitTreeMutexInfo::WaitTreeMutexInfo(VAddr mutex_address) : mutex_address(mutex_address) { + mutex_value = Memory::Read32(mutex_address); + owner_handle = static_cast(mutex_value & Kernel::Mutex::MutexOwnerMask); + owner = Kernel::g_handle_table.Get(owner_handle); +} + +QString WaitTreeMutexInfo::GetText() const { + return tr("waiting for mutex 0x%1").arg(mutex_address, 16, 16, QLatin1Char('0')); +} + +std::vector> WaitTreeMutexInfo::GetChildren() const { + std::vector> list; + + bool has_waiters = (mutex_value & Kernel::Mutex::MutexHasWaitersFlag) != 0; + + list.push_back(std::make_unique(tr("has waiters: %1").arg(has_waiters))); + list.push_back(std::make_unique( + tr("owner handle: 0x%1").arg(owner_handle, 8, 16, QLatin1Char('0')))); + if (owner != nullptr) + list.push_back(std::make_unique(*owner)); + return list; +} + WaitTreeWaitObject::WaitTreeWaitObject(const Kernel::WaitObject& o) : object(o) {} bool WaitTreeExpandableItem::IsExpandable() const { @@ -84,11 +107,6 @@ std::unique_ptr WaitTreeWaitObject::make(const Kernel::WaitO switch (object.GetHandleType()) { case Kernel::HandleType::Event: return std::make_unique(static_cast(object)); - case Kernel::HandleType::Mutex: - return std::make_unique(static_cast(object)); - case Kernel::HandleType::ConditionVariable: - return std::make_unique( - static_cast(object)); case Kernel::HandleType::Timer: return std::make_unique(static_cast(object)); case Kernel::HandleType::Thread: @@ -160,6 +178,9 @@ QString WaitTreeThread::GetText() const { case THREADSTATUS_WAIT_SYNCH_ANY: status = tr("waiting for objects"); break; + case THREADSTATUS_WAIT_MUTEX: + status = tr("waiting for mutex"); + break; case THREADSTATUS_DORMANT: status = tr("dormant"); break; @@ -186,6 +207,7 @@ QColor WaitTreeThread::GetColor() const { return QColor(Qt::GlobalColor::darkYellow); case THREADSTATUS_WAIT_SYNCH_ALL: case THREADSTATUS_WAIT_SYNCH_ANY: + case THREADSTATUS_WAIT_MUTEX: return QColor(Qt::GlobalColor::red); case THREADSTATUS_DORMANT: return QColor(Qt::GlobalColor::darkCyan); @@ -225,11 +247,11 @@ std::vector> WaitTreeThread::GetChildren() const { list.push_back(std::make_unique( tr("last running ticks = %1").arg(thread.last_running_ticks))); - if (thread.held_mutexes.empty()) { - list.push_back(std::make_unique(tr("not holding mutex"))); - } else { - list.push_back(std::make_unique(thread.held_mutexes)); - } + if (thread.mutex_wait_address != 0) + list.push_back(std::make_unique(thread.mutex_wait_address)); + else + list.push_back(std::make_unique(tr("not waiting for mutex"))); + if (thread.status == THREADSTATUS_WAIT_SYNCH_ANY || thread.status == THREADSTATUS_WAIT_SYNCH_ALL) { list.push_back(std::make_unique(thread.wait_objects, @@ -250,33 +272,6 @@ std::vector> WaitTreeEvent::GetChildren() const { return list; } -WaitTreeMutex::WaitTreeMutex(const Kernel::Mutex& object) : WaitTreeWaitObject(object) {} - -std::vector> WaitTreeMutex::GetChildren() const { - std::vector> list(WaitTreeWaitObject::GetChildren()); - - const auto& mutex = static_cast(object); - if (mutex.GetHasWaiters()) { - list.push_back(std::make_unique(tr("locked by thread:"))); - list.push_back(std::make_unique(*mutex.GetHoldingThread())); - } else { - list.push_back(std::make_unique(tr("free"))); - } - return list; -} - -WaitTreeConditionVariable::WaitTreeConditionVariable(const Kernel::ConditionVariable& object) - : WaitTreeWaitObject(object) {} - -std::vector> WaitTreeConditionVariable::GetChildren() const { - std::vector> list(WaitTreeWaitObject::GetChildren()); - - const auto& condition_variable = static_cast(object); - list.push_back(std::make_unique( - tr("available count = %1").arg(condition_variable.GetAvailableCount()))); - return list; -} - WaitTreeTimer::WaitTreeTimer(const Kernel::Timer& object) : WaitTreeWaitObject(object) {} std::vector> WaitTreeTimer::GetChildren() const { @@ -293,21 +288,6 @@ std::vector> WaitTreeTimer::GetChildren() const { return list; } -WaitTreeMutexList::WaitTreeMutexList( - const boost::container::flat_set>& list) - : mutex_list(list) {} - -QString WaitTreeMutexList::GetText() const { - return tr("holding mutexes"); -} - -std::vector> WaitTreeMutexList::GetChildren() const { - std::vector> list(mutex_list.size()); - std::transform(mutex_list.begin(), mutex_list.end(), list.begin(), - [](const auto& t) { return std::make_unique(*t); }); - return list; -} - WaitTreeThreadList::WaitTreeThreadList(const std::vector>& list) : thread_list(list) {} diff --git a/src/yuzu/debugger/wait_tree.h b/src/yuzu/debugger/wait_tree.h index e538174eb..300ba9ae4 100644 --- a/src/yuzu/debugger/wait_tree.h +++ b/src/yuzu/debugger/wait_tree.h @@ -16,8 +16,6 @@ class EmuThread; namespace Kernel { class WaitObject; class Event; -class Mutex; -class ConditionVariable; class Thread; class Timer; } // namespace Kernel @@ -61,6 +59,20 @@ public: bool IsExpandable() const override; }; +class WaitTreeMutexInfo : public WaitTreeExpandableItem { + Q_OBJECT +public: + explicit WaitTreeMutexInfo(VAddr mutex_address); + QString GetText() const override; + std::vector> GetChildren() const override; + +private: + VAddr mutex_address; + u32 mutex_value; + Kernel::Handle owner_handle; + Kernel::SharedPtr owner; +}; + class WaitTreeWaitObject : public WaitTreeExpandableItem { Q_OBJECT public: @@ -104,20 +116,6 @@ public: std::vector> GetChildren() const override; }; -class WaitTreeMutex : public WaitTreeWaitObject { - Q_OBJECT -public: - explicit WaitTreeMutex(const Kernel::Mutex& object); - std::vector> GetChildren() const override; -}; - -class WaitTreeConditionVariable : public WaitTreeWaitObject { - Q_OBJECT -public: - explicit WaitTreeConditionVariable(const Kernel::ConditionVariable& object); - std::vector> GetChildren() const override; -}; - class WaitTreeTimer : public WaitTreeWaitObject { Q_OBJECT public: @@ -125,19 +123,6 @@ public: std::vector> GetChildren() const override; }; -class WaitTreeMutexList : public WaitTreeExpandableItem { - Q_OBJECT -public: - explicit WaitTreeMutexList( - const boost::container::flat_set>& list); - - QString GetText() const override; - std::vector> GetChildren() const override; - -private: - const boost::container::flat_set>& mutex_list; -}; - class WaitTreeThreadList : public WaitTreeExpandableItem { Q_OBJECT public: