From 2d16507f9fa06e868349d6f57a78585aec8628fd Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Thu, 14 Nov 2019 20:13:18 -0400 Subject: [PATCH 1/3] Kernel: Correct behavior of Condition Variables to be more similar to real hardware. This commit ensures cond var threads act exactly as they do in the real console. The original implementation uses an RBTree and the behavior of cond var threads is that at the same priority level they act like a FIFO. --- src/core/hle/kernel/kernel.cpp | 3 +++ src/core/hle/kernel/process.cpp | 46 +++++++++++++++++++++++++++++++++ src/core/hle/kernel/process.h | 12 +++++++++ src/core/hle/kernel/svc.cpp | 20 ++++---------- src/core/hle/kernel/thread.cpp | 8 ++++++ 5 files changed, 74 insertions(+), 15 deletions(-) diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index f94ac150d0..cdf7944f78 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -66,6 +66,9 @@ static void ThreadWakeupCallback(u64 thread_handle, [[maybe_unused]] s64 cycles_ thread->SetMutexWaitAddress(0); thread->SetCondVarWaitAddress(0); thread->SetWaitHandle(0); + if (thread->GetStatus() == ThreadStatus::WaitCondVar) { + thread->GetOwnerProcess()->RemoveConditionVariableThread(thread); + } auto* const lock_owner = thread->GetLockOwner(); // Threads waking up by timeout from WaitProcessWideKey do not perform priority inheritance diff --git a/src/core/hle/kernel/process.cpp b/src/core/hle/kernel/process.cpp index 12a900bcc4..43576c6abc 100644 --- a/src/core/hle/kernel/process.cpp +++ b/src/core/hle/kernel/process.cpp @@ -142,6 +142,52 @@ u64 Process::GetTotalPhysicalMemoryUsedWithoutSystemResource() const { return GetTotalPhysicalMemoryUsed() - GetSystemResourceUsage(); } +void Process::InsertConditionVariableThread(SharedPtr thread) { + auto it = cond_var_threads.begin(); + while (it != cond_var_threads.end()) { + const SharedPtr current_thread = *it; + if (current_thread->GetCondVarWaitAddress() < thread->GetCondVarWaitAddress()) { + if (current_thread->GetCondVarWaitAddress() == thread->GetCondVarWaitAddress()) { + if (current_thread->GetPriority() > thread->GetPriority()) { + cond_var_threads.insert(it, thread); + return; + } + } else { + cond_var_threads.insert(it, thread); + return; + } + } + ++it; + } + cond_var_threads.push_back(thread); +} + +void Process::RemoveConditionVariableThread(SharedPtr thread) { + auto it = cond_var_threads.begin(); + while (it != cond_var_threads.end()) { + const SharedPtr current_thread = *it; + if (current_thread.get() == thread.get()) { + cond_var_threads.erase(it); + return; + } + ++it; + } + UNREACHABLE(); +} + +std::vector> Process::GetConditionVariableThreads(const VAddr cond_var_addr) { + std::vector> result{}; + auto it = cond_var_threads.begin(); + while (it != cond_var_threads.end()) { + SharedPtr current_thread = *it; + if (current_thread->GetCondVarWaitAddress() == cond_var_addr) { + result.push_back(current_thread); + } + ++it; + } + return result; +} + void Process::RegisterThread(const Thread* thread) { thread_list.push_back(thread); } diff --git a/src/core/hle/kernel/process.h b/src/core/hle/kernel/process.h index c2df451f3d..e8bff709bb 100644 --- a/src/core/hle/kernel/process.h +++ b/src/core/hle/kernel/process.h @@ -232,6 +232,15 @@ public: return thread_list; } + /// Insert a thread into the condition variable wait container + void InsertConditionVariableThread(SharedPtr thread); + + /// Remove a thread from the condition variable wait container + void RemoveConditionVariableThread(SharedPtr thread); + + /// Obtain all condition variable threads waiting for some address + std::vector> GetConditionVariableThreads(VAddr cond_var_addr); + /// Registers a thread as being created under this process, /// adding it to this process' thread list. void RegisterThread(const Thread* thread); @@ -375,6 +384,9 @@ private: /// List of threads that are running with this process as their owner. std::list thread_list; + /// List of threads waiting for a condition variable + std::list> cond_var_threads; + /// System context Core::System& system; diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index c63a9ba8bb..c27529f4d9 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1626,6 +1626,7 @@ static ResultCode WaitProcessWideKeyAtomic(Core::System& system, VAddr mutex_add current_thread->SetWaitHandle(thread_handle); current_thread->SetStatus(ThreadStatus::WaitCondVar); current_thread->InvalidateWakeupCallback(); + current_process->InsertConditionVariableThread(current_thread); current_thread->WakeAfterDelay(nano_seconds); @@ -1644,21 +1645,9 @@ static ResultCode SignalProcessWideKey(Core::System& system, VAddr condition_var ASSERT(condition_variable_addr == Common::AlignDown(condition_variable_addr, 4)); // Retrieve a list of all threads that are waiting for this condition variable. - std::vector> waiting_threads; - const auto& scheduler = system.GlobalScheduler(); - const auto& thread_list = scheduler.GetThreadList(); - - for (const auto& thread : thread_list) { - if (thread->GetCondVarWaitAddress() == condition_variable_addr) { - waiting_threads.push_back(thread); - } - } - - // Sort them by priority, such that the highest priority ones come first. - std::sort(waiting_threads.begin(), waiting_threads.end(), - [](const SharedPtr& lhs, const SharedPtr& rhs) { - return lhs->GetPriority() < rhs->GetPriority(); - }); + auto* const current_process = system.Kernel().CurrentProcess(); + std::vector> waiting_threads = + current_process->GetConditionVariableThreads(condition_variable_addr); // Only process up to 'target' threads, unless 'target' is -1, in which case process // them all. @@ -1677,6 +1666,7 @@ static ResultCode SignalProcessWideKey(Core::System& system, VAddr condition_var // liberate Cond Var Thread. thread->SetCondVarWaitAddress(0); + current_process->RemoveConditionVariableThread(thread); const std::size_t current_core = system.CurrentCoreIndex(); auto& monitor = system.Monitor(); diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index ee7531f2d5..6dafa311d2 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -306,8 +306,16 @@ void Thread::UpdatePriority() { return; } + if (GetStatus() == ThreadStatus::WaitCondVar) { + owner_process->RemoveConditionVariableThread(this); + } + SetCurrentPriority(new_priority); + if (GetStatus() == ThreadStatus::WaitCondVar) { + owner_process->InsertConditionVariableThread(this); + } + if (!lock_owner) { return; } From 2ab41ceff469cfa9b13f6357ce558b3388b0fe30 Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Sat, 16 Nov 2019 13:55:21 -0400 Subject: [PATCH 2/3] Kernel: Correct SignalProcessWideKey When the target is 0, all threads must be processed. --- src/core/hle/kernel/svc.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index c27529f4d9..4fdb6d429e 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1649,16 +1649,12 @@ static ResultCode SignalProcessWideKey(Core::System& system, VAddr condition_var std::vector> waiting_threads = current_process->GetConditionVariableThreads(condition_variable_addr); - // Only process up to 'target' threads, unless 'target' is -1, in which case process + // Only process up to 'target' threads, unless 'target' is less equal 0, in which case process // them all. std::size_t last = waiting_threads.size(); - if (target != -1) + if (target > 0) last = std::min(waiting_threads.size(), static_cast(target)); - // If there are no threads waiting on this condition variable, just exit - if (last == 0) - return RESULT_SUCCESS; - for (std::size_t index = 0; index < last; ++index) { auto& thread = waiting_threads[index]; From 46bb6099814a6ff404d337164ced016ec04ea7b9 Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Thu, 21 Nov 2019 11:03:37 -0400 Subject: [PATCH 3/3] Kernel: Optimize condition variable threads management. --- src/core/hle/kernel/kernel.cpp | 2 +- src/core/hle/kernel/process.cpp | 38 +++++++++++++++------------------ src/core/hle/kernel/process.h | 3 ++- src/core/hle/kernel/svc.cpp | 2 +- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index cdf7944f78..9d3b309b3e 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -64,10 +64,10 @@ static void ThreadWakeupCallback(u64 thread_handle, [[maybe_unused]] s64 cycles_ } else if (thread->GetStatus() == ThreadStatus::WaitMutex || thread->GetStatus() == ThreadStatus::WaitCondVar) { thread->SetMutexWaitAddress(0); - thread->SetCondVarWaitAddress(0); thread->SetWaitHandle(0); if (thread->GetStatus() == ThreadStatus::WaitCondVar) { thread->GetOwnerProcess()->RemoveConditionVariableThread(thread); + thread->SetCondVarWaitAddress(0); } auto* const lock_owner = thread->GetLockOwner(); diff --git a/src/core/hle/kernel/process.cpp b/src/core/hle/kernel/process.cpp index 43576c6abc..a4e0dd3857 100644 --- a/src/core/hle/kernel/process.cpp +++ b/src/core/hle/kernel/process.cpp @@ -143,31 +143,28 @@ u64 Process::GetTotalPhysicalMemoryUsedWithoutSystemResource() const { } void Process::InsertConditionVariableThread(SharedPtr thread) { - auto it = cond_var_threads.begin(); - while (it != cond_var_threads.end()) { + VAddr cond_var_addr = thread->GetCondVarWaitAddress(); + std::list>& thread_list = cond_var_threads[cond_var_addr]; + auto it = thread_list.begin(); + while (it != thread_list.end()) { const SharedPtr current_thread = *it; - if (current_thread->GetCondVarWaitAddress() < thread->GetCondVarWaitAddress()) { - if (current_thread->GetCondVarWaitAddress() == thread->GetCondVarWaitAddress()) { - if (current_thread->GetPriority() > thread->GetPriority()) { - cond_var_threads.insert(it, thread); - return; - } - } else { - cond_var_threads.insert(it, thread); - return; - } + if (current_thread->GetPriority() > thread->GetPriority()) { + thread_list.insert(it, thread); + return; } ++it; } - cond_var_threads.push_back(thread); + thread_list.push_back(thread); } void Process::RemoveConditionVariableThread(SharedPtr thread) { - auto it = cond_var_threads.begin(); - while (it != cond_var_threads.end()) { + VAddr cond_var_addr = thread->GetCondVarWaitAddress(); + std::list>& thread_list = cond_var_threads[cond_var_addr]; + auto it = thread_list.begin(); + while (it != thread_list.end()) { const SharedPtr current_thread = *it; if (current_thread.get() == thread.get()) { - cond_var_threads.erase(it); + thread_list.erase(it); return; } ++it; @@ -177,12 +174,11 @@ void Process::RemoveConditionVariableThread(SharedPtr thread) { std::vector> Process::GetConditionVariableThreads(const VAddr cond_var_addr) { std::vector> result{}; - auto it = cond_var_threads.begin(); - while (it != cond_var_threads.end()) { + std::list>& thread_list = cond_var_threads[cond_var_addr]; + auto it = thread_list.begin(); + while (it != thread_list.end()) { SharedPtr current_thread = *it; - if (current_thread->GetCondVarWaitAddress() == cond_var_addr) { - result.push_back(current_thread); - } + result.push_back(current_thread); ++it; } return result; diff --git a/src/core/hle/kernel/process.h b/src/core/hle/kernel/process.h index e8bff709bb..e2eda26b90 100644 --- a/src/core/hle/kernel/process.h +++ b/src/core/hle/kernel/process.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "common/common_types.h" #include "core/hle/kernel/address_arbiter.h" @@ -385,7 +386,7 @@ private: std::list thread_list; /// List of threads waiting for a condition variable - std::list> cond_var_threads; + std::unordered_map>> cond_var_threads; /// System context Core::System& system; diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 4fdb6d429e..e2cf846243 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1661,8 +1661,8 @@ static ResultCode SignalProcessWideKey(Core::System& system, VAddr condition_var ASSERT(thread->GetCondVarWaitAddress() == condition_variable_addr); // liberate Cond Var Thread. - thread->SetCondVarWaitAddress(0); current_process->RemoveConditionVariableThread(thread); + thread->SetCondVarWaitAddress(0); const std::size_t current_core = system.CurrentCoreIndex(); auto& monitor = system.Monitor();