From e6a7723f2f4b62279cd4f6d4b48eb02a9b60ffb6 Mon Sep 17 00:00:00 2001 From: Subv Date: Sun, 1 Jan 2017 16:53:22 -0500 Subject: Kernel: Object ShouldWait and Acquire calls now take a thread as a parameter. This will be useful when implementing mutex priority inheritance. --- src/core/hle/kernel/thread.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/core/hle/kernel/thread.h') diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index c77ac644d..f2bc1ec9c 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -72,8 +72,8 @@ public: return HANDLE_TYPE; } - bool ShouldWait() override; - void Acquire() override; + bool ShouldWait(Thread* thread) const override; + void Acquire(Thread* thread) override; /** * Gets the thread's current priority -- cgit v1.2.3 From d3ff5b91e14356912589f9bac47fccbe79e07279 Mon Sep 17 00:00:00 2001 From: Subv Date: Mon, 2 Jan 2017 19:38:08 -0500 Subject: Kernel/Mutex: Propagate thread priority changes to other threads inheriting the priority via mutexes --- src/core/hle/kernel/mutex.cpp | 60 ++++++++++++++++-------------------------- src/core/hle/kernel/mutex.h | 6 +++++ src/core/hle/kernel/thread.cpp | 21 ++++++++++++--- src/core/hle/kernel/thread.h | 9 +++++++ src/core/hle/svc.cpp | 6 +++++ 5 files changed, 60 insertions(+), 42 deletions(-) (limited to 'src/core/hle/kernel/thread.h') diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index 84ff65150..cef961289 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/src/core/hle/kernel/mutex.cpp @@ -13,38 +13,6 @@ namespace Kernel { -/** - * Boost's a thread's priority to the best priority among the thread's held mutexes. - * This prevents priority inversion via priority inheritance. - */ -static void UpdateThreadPriority(Thread* thread) { - s32 best_priority = THREADPRIO_LOWEST; - for (auto& mutex : thread->held_mutexes) { - if (mutex->priority < best_priority) - best_priority = mutex->priority; - } - - best_priority = std::min(best_priority, thread->nominal_priority); - thread->SetPriority(best_priority); -} - -/** - * Elevate the mutex priority to the best priority - * among the priorities of all its waiting threads. - */ -static void UpdateMutexPriority(Mutex* mutex) { - s32 best_priority = THREADPRIO_LOWEST; - for (auto& waiter : mutex->GetWaitingThreads()) { - if (waiter->current_priority < best_priority) - best_priority = waiter->current_priority; - } - - if (best_priority != mutex->priority) { - mutex->priority = best_priority; - UpdateThreadPriority(mutex->holding_thread.get()); - } -} - void ReleaseThreadMutexes(Thread* thread) { for (auto& mtx : thread->held_mutexes) { mtx->lock_count = 0; @@ -83,9 +51,7 @@ void Mutex::Acquire(Thread* thread) { priority = thread->current_priority; thread->held_mutexes.insert(this); holding_thread = thread; - - UpdateThreadPriority(thread); - + thread->UpdatePriority(); Core::System::GetInstance().PrepareReschedule(); } @@ -100,7 +66,7 @@ void Mutex::Release() { // Yield to the next thread only if we've fully released the mutex if (lock_count == 0) { holding_thread->held_mutexes.erase(this); - UpdateThreadPriority(holding_thread.get()); + holding_thread->UpdatePriority(); holding_thread = nullptr; WakeupAllWaitingThreads(); Core::System::GetInstance().PrepareReschedule(); @@ -110,12 +76,30 @@ void Mutex::Release() { void Mutex::AddWaitingThread(SharedPtr thread) { WaitObject::AddWaitingThread(thread); - UpdateMutexPriority(this); + thread->pending_mutexes.insert(this); + UpdatePriority(); } void Mutex::RemoveWaitingThread(Thread* thread) { WaitObject::RemoveWaitingThread(thread); - UpdateMutexPriority(this); + thread->pending_mutexes.erase(this); + UpdatePriority(); +} + +void Mutex::UpdatePriority() { + if (!holding_thread) + return; + + s32 best_priority = THREADPRIO_LOWEST; + for (auto& waiter : GetWaitingThreads()) { + if (waiter->current_priority < best_priority) + best_priority = waiter->current_priority; + } + + if (best_priority != priority) { + priority = best_priority; + holding_thread->UpdatePriority(); + } } } // namespace diff --git a/src/core/hle/kernel/mutex.h b/src/core/hle/kernel/mutex.h index 31f920516..c57adf400 100644 --- a/src/core/hle/kernel/mutex.h +++ b/src/core/hle/kernel/mutex.h @@ -39,6 +39,12 @@ public: std::string name; ///< Name of mutex (optional) SharedPtr holding_thread; ///< Thread that has acquired the mutex + /** + * 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; diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index d44010824..3a5a67450 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -105,15 +105,15 @@ void Thread::Stop() { WakeupAllWaitingThreads(); - // Release all the mutexes that this thread holds - ReleaseThreadMutexes(this); - // Clean up any dangling references in objects that this thread was waiting for for (auto& wait_object : wait_objects) { wait_object->RemoveWaitingThread(this); } wait_objects.clear(); + // Release all the mutexes that this thread holds + ReleaseThreadMutexes(this); + // Mark the TLS slot in the thread's page as free. u32 tls_page = (tls_address - Memory::TLS_AREA_VADDR) / Memory::PAGE_SIZE; u32 tls_slot = @@ -515,8 +515,21 @@ void Thread::SetPriority(s32 priority) { nominal_priority = current_priority = priority; } +void Thread::UpdatePriority() { + s32 best_priority = nominal_priority; + for (auto& mutex : held_mutexes) { + if (mutex->priority < best_priority) + best_priority = mutex->priority; + } + BoostPriority(best_priority); +} + void Thread::BoostPriority(s32 priority) { - ready_queue.move(this, current_priority, priority); + // If thread was ready, adjust queues + if (status == THREADSTATUS_READY) + ready_queue.move(this, current_priority, priority); + else + ready_queue.prepare(priority); current_priority = priority; } diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index f2bc1ec9c..e2f0cc831 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -89,6 +89,12 @@ public: */ void SetPriority(s32 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 @@ -178,6 +184,9 @@ public: /// 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. diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp index b6e34a9e9..160f27c98 100644 --- a/src/core/hle/svc.cpp +++ b/src/core/hle/svc.cpp @@ -611,6 +611,12 @@ static ResultCode SetThreadPriority(Kernel::Handle handle, s32 priority) { return ERR_INVALID_HANDLE; 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; } -- cgit v1.2.3 From cef5f45de2fd64f0728d4504d0ad7434cb8ac519 Mon Sep 17 00:00:00 2001 From: Subv Date: Wed, 4 Jan 2017 10:44:38 -0500 Subject: Kernel: Use different thread statuses when a thread calls WaitSynchronization1 and WaitSynchronizationN with wait_all = true. This commit removes the overly general THREADSTATUS_WAIT_SYNCH and replaces it with two more granular statuses: THREADSTATUS_WAIT_SYNCH_ANY when a thread waits on objects via WaitSynchronization1 or WaitSynchronizationN with wait_all = false. THREADSTATUS_WAIT_SYNCH_ALL when a thread waits on objects via WaitSynchronizationN with wait_all = true. --- src/citra_qt/debugger/wait_tree.cpp | 9 ++++++--- src/core/hle/kernel/thread.cpp | 11 +++++++---- src/core/hle/kernel/thread.h | 19 ++++++++++--------- src/core/hle/svc.cpp | 6 +++--- 4 files changed, 26 insertions(+), 19 deletions(-) (limited to 'src/core/hle/kernel/thread.h') diff --git a/src/citra_qt/debugger/wait_tree.cpp b/src/citra_qt/debugger/wait_tree.cpp index 1d2de5185..b6ecf3819 100644 --- a/src/citra_qt/debugger/wait_tree.cpp +++ b/src/citra_qt/debugger/wait_tree.cpp @@ -153,7 +153,8 @@ QString WaitTreeThread::GetText() const { case THREADSTATUS_WAIT_SLEEP: status = tr("sleeping"); break; - case THREADSTATUS_WAIT_SYNCH: + case THREADSTATUS_WAIT_SYNCH_ALL: + case THREADSTATUS_WAIT_SYNCH_ANY: status = tr("waiting for objects"); break; case THREADSTATUS_DORMANT: @@ -180,7 +181,8 @@ QColor WaitTreeThread::GetColor() const { return QColor(Qt::GlobalColor::darkRed); case THREADSTATUS_WAIT_SLEEP: return QColor(Qt::GlobalColor::darkYellow); - case THREADSTATUS_WAIT_SYNCH: + case THREADSTATUS_WAIT_SYNCH_ALL: + case THREADSTATUS_WAIT_SYNCH_ANY: return QColor(Qt::GlobalColor::red); case THREADSTATUS_DORMANT: return QColor(Qt::GlobalColor::darkCyan); @@ -228,7 +230,8 @@ std::vector> WaitTreeThread::GetChildren() const { } else { list.push_back(std::make_unique(thread.held_mutexes)); } - if (thread.status == THREADSTATUS_WAIT_SYNCH) { + if (thread.status == THREADSTATUS_WAIT_SYNCH_ANY || + thread.status == THREADSTATUS_WAIT_SYNCH_ALL) { list.push_back(std::make_unique(thread.wait_objects, thread.IsSleepingOnWaitAll())); } diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 3a5a67450..aa99d18c7 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -72,7 +72,8 @@ Thread* GetCurrentThread() { * @return True if the thread is waiting, false otherwise */ static bool CheckWait_WaitObject(const Thread* thread, WaitObject* wait_object) { - if (thread->status != THREADSTATUS_WAIT_SYNCH) + if (thread->status != THREADSTATUS_WAIT_SYNCH_ALL && + thread->status != THREADSTATUS_WAIT_SYNCH_ANY) return false; auto itr = std::find(thread->wait_objects.begin(), thread->wait_objects.end(), wait_object); @@ -253,7 +254,7 @@ void WaitCurrentThread_WaitSynchronization(std::vector> wa Thread* thread = GetCurrentThread(); thread->wait_set_output = wait_set_output; thread->wait_objects = std::move(wait_objects); - thread->status = THREADSTATUS_WAIT_SYNCH; + thread->status = THREADSTATUS_WAIT_SYNCH_ANY; } void WaitCurrentThread_ArbitrateAddress(VAddr wait_address) { @@ -281,7 +282,8 @@ static void ThreadWakeupCallback(u64 thread_handle, int cycles_late) { return; } - if (thread->status == THREADSTATUS_WAIT_SYNCH || thread->status == THREADSTATUS_WAIT_ARB) { + if (thread->status == THREADSTATUS_WAIT_SYNCH_ANY || + thread->status == THREADSTATUS_WAIT_SYNCH_ALL || thread->status == THREADSTATUS_WAIT_ARB) { thread->wait_set_output = false; // Remove the thread from each of its waiting objects' waitlists for (auto& object : thread->wait_objects) @@ -306,7 +308,8 @@ void Thread::WakeAfterDelay(s64 nanoseconds) { void Thread::ResumeFromWait() { switch (status) { - case THREADSTATUS_WAIT_SYNCH: + case THREADSTATUS_WAIT_SYNCH_ALL: + case THREADSTATUS_WAIT_SYNCH_ANY: case THREADSTATUS_WAIT_ARB: case THREADSTATUS_WAIT_SLEEP: break; diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index e2f0cc831..6cd8c20e2 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -31,13 +31,14 @@ enum ThreadProcessorId : s32 { }; enum ThreadStatus { - THREADSTATUS_RUNNING, ///< Currently running - THREADSTATUS_READY, ///< Ready to run - THREADSTATUS_WAIT_ARB, ///< Waiting on an address arbiter - THREADSTATUS_WAIT_SLEEP, ///< Waiting due to a SleepThread SVC - THREADSTATUS_WAIT_SYNCH, ///< Waiting due to a WaitSynchronization SVC - THREADSTATUS_DORMANT, ///< Created but not yet made ready - THREADSTATUS_DEAD ///< Run to completion, or forcefully terminated + THREADSTATUS_RUNNING, ///< Currently running + THREADSTATUS_READY, ///< Ready to run + THREADSTATUS_WAIT_ARB, ///< Waiting on an address arbiter + THREADSTATUS_WAIT_SLEEP, ///< Waiting due to a SleepThread SVC + 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_DORMANT, ///< Created but not yet made ready + THREADSTATUS_DEAD ///< Run to completion, or forcefully terminated }; namespace Kernel { @@ -158,10 +159,10 @@ public: /** * Returns whether this thread is waiting for all the objects in * its wait list to become ready, as a result of a WaitSynchronizationN call - * with wait_all = true, or a ReplyAndReceive call. + * with wait_all = true. */ bool IsSleepingOnWaitAll() const { - return !wait_objects.empty(); + return status == THREADSTATUS_WAIT_SYNCH_ALL; } ARM_Interface::ThreadContext context; diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp index 160f27c98..1e1ca5180 100644 --- a/src/core/hle/svc.cpp +++ b/src/core/hle/svc.cpp @@ -278,7 +278,7 @@ static ResultCode WaitSynchronization1(Kernel::Handle handle, s64 nano_seconds) return ERR_SYNC_TIMEOUT; object->AddWaitingThread(thread); - thread->status = THREADSTATUS_WAIT_SYNCH; + 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); @@ -351,7 +351,7 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha return ERR_SYNC_TIMEOUT; // Put the thread to sleep - thread->status = THREADSTATUS_WAIT_SYNCH; + thread->status = THREADSTATUS_WAIT_SYNCH_ALL; // Add the thread to each of the objects' waiting threads. for (auto& object : objects) { @@ -393,7 +393,7 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha return ERR_SYNC_TIMEOUT; // Put the thread to sleep - thread->status = THREADSTATUS_WAIT_SYNCH; + thread->status = THREADSTATUS_WAIT_SYNCH_ANY; // Clear the thread's waitlist, we won't use it for wait_all = false thread->wait_objects.clear(); -- cgit v1.2.3 From fd95b6ee2606da4cd47c5f2916ad3b4f86c0e0f4 Mon Sep 17 00:00:00 2001 From: Subv Date: Wed, 4 Jan 2017 10:53:01 -0500 Subject: Kernel: Remove Thread::wait_objects_index and use wait_objects to hold all the objects that a thread is waiting on. --- src/core/hle/kernel/kernel.cpp | 8 +++++++- src/core/hle/kernel/thread.cpp | 5 +++++ src/core/hle/kernel/thread.h | 16 +++++++--------- src/core/hle/svc.cpp | 14 +++----------- 4 files changed, 22 insertions(+), 21 deletions(-) (limited to 'src/core/hle/kernel/thread.h') diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index 6f61d526a..955f50a9b 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -55,10 +55,16 @@ SharedPtr WaitObject::GetHighestPriorityReadyThread() { if (ShouldWait(thread.get())) continue; - bool ready_to_run = std::none_of(thread->wait_objects.begin(), thread->wait_objects.end(), + // A thread is ready to run if it's either in THREADSTATUS_WAIT_SYNCH_ANY or + // in THREADSTATUS_WAIT_SYNCH_ALL and the rest of the objects it is waiting on are ready. + bool ready_to_run = true; + if (thread->status == THREADSTATUS_WAIT_SYNCH_ALL) { + ready_to_run = std::none_of(thread->wait_objects.begin(), thread->wait_objects.end(), [&thread](const SharedPtr& object) { return object->ShouldWait(thread.get()); }); + } + if (ready_to_run) { candidate = thread.get(); candidate_priority = thread->current_priority; diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index aa99d18c7..568cef5b9 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -579,6 +579,11 @@ void Thread::SetWaitSynchronizationOutput(s32 output) { context.cpu_registers[1] = output; } +s32 Thread::GetWaitObjectIndex(WaitObject* object) const { + auto match = std::find(wait_objects.rbegin(), wait_objects.rend(), object); + return std::distance(match, wait_objects.rend()) - 1; +} + //////////////////////////////////////////////////////////////////////////////////////////////////// void ThreadingInit() { diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index 6cd8c20e2..af72b76ea 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -135,13 +135,14 @@ public: /** * Retrieves the index that this particular object occupies in the list of objects - * that the thread passed to WaitSynchronizationN. + * that the thread passed to WaitSynchronizationN, starting the search from the last element. * It is used to set the output value of WaitSynchronizationN when the thread is awakened. + * When a thread wakes up due to an object signal, the kernel will use the index of the last + * matching object in the wait objects list in case of having multiple instances of the same + * object in the list. * @param object Object to query the index of. */ - s32 GetWaitObjectIndex(const WaitObject* object) const { - return wait_objects_index.at(object->GetObjectId()); - } + s32 GetWaitObjectIndex(WaitObject* object) const; /** * Stops a thread, invalidating it from further use @@ -190,13 +191,10 @@ public: SharedPtr owner_process; ///< Process that owns this thread - /// Objects that the thread is waiting on. - /// This is only populated when the thread should wait for all the objects to become ready. + /// Objects that the thread is waiting on, in the same order as they were + // passed to WaitSynchronization1/N. std::vector> wait_objects; - /// Mapping of Object ids to their position in the last waitlist that this object waited on. - boost::container::flat_map wait_objects_index; - VAddr wait_address; ///< If waiting on an AddressArbiter, this is the arbitration address /// True if the WaitSynchronizationN output parameter should be set on thread wakeup. diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp index 1e1ca5180..855f3af82 100644 --- a/src/core/hle/svc.cpp +++ b/src/core/hle/svc.cpp @@ -277,6 +277,7 @@ static ResultCode WaitSynchronization1(Kernel::Handle handle, s64 nano_seconds) if (nano_seconds == 0) return ERR_SYNC_TIMEOUT; + thread->wait_objects = {object}; object->AddWaitingThread(thread); thread->status = THREADSTATUS_WAIT_SYNCH_ANY; @@ -325,11 +326,6 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha objects[i] = object; } - // Clear the mapping of wait object indices. - // We don't want any lingering state in this map. - // It will be repopulated later in the wait_all = false case. - thread->wait_objects_index.clear(); - if (wait_all) { bool all_available = std::all_of(objects.begin(), objects.end(), @@ -358,7 +354,6 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha object->AddWaitingThread(thread); } - // Set the thread's waitlist to the list of objects passed to WaitSynchronizationN thread->wait_objects = std::move(objects); // Create an event to wake the thread up after the specified nanosecond delay has passed @@ -395,17 +390,14 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha // Put the thread to sleep thread->status = THREADSTATUS_WAIT_SYNCH_ANY; - // Clear the thread's waitlist, we won't use it for wait_all = false - thread->wait_objects.clear(); - // Add the thread to each of the objects' waiting threads. for (size_t i = 0; i < objects.size(); ++i) { Kernel::WaitObject* object = objects[i].get(); - // Set the index of this object in the mapping of Objects -> index for this thread. - thread->wait_objects_index[object->GetObjectId()] = static_cast(i); object->AddWaitingThread(thread); } + thread->wait_objects = std::move(objects); + // Note: If no handles and no timeout were given, then the thread will deadlock, this is // consistent with hardware behavior. -- cgit v1.2.3