From 38a90882a4230bb797ae3f6857e986d70f3bd265 Mon Sep 17 00:00:00 2001
From: Subv <subv2112@gmail.com>
Date: Sun, 1 Jan 2017 11:57:02 -0500
Subject: Kernel/Synch: Do not attempt a reschedule on every syscall.

Not all syscalls should cause reschedules, this commit attempts to remedy that, however, it still does not cover all cases.
---
 src/core/hle/kernel/mutex.cpp |  1 +
 src/core/hle/svc.cpp          | 19 +++++++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

(limited to 'src/core')

diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp
index 736944bae..8d92a9b8e 100644
--- a/src/core/hle/kernel/mutex.cpp
+++ b/src/core/hle/kernel/mutex.cpp
@@ -84,6 +84,7 @@ void Mutex::Release() {
         if (lock_count == 0) {
             holding_thread->held_mutexes.erase(this);
             ResumeWaitingThread(this);
+            Core::System::GetInstance().PrepareReschedule();
         }
     }
 }
diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp
index 2ca270de3..5b538be22 100644
--- a/src/core/hle/svc.cpp
+++ b/src/core/hle/svc.cpp
@@ -248,6 +248,8 @@ static ResultCode SendSyncRequest(Kernel::Handle handle) {
 
     LOG_TRACE(Kernel_SVC, "called handle=0x%08X(%s)", handle, session->GetName().c_str());
 
+    Core::System::GetInstance().PrepareReschedule();
+
     // TODO(Subv): svcSendSyncRequest should put the caller thread to sleep while the server
     // responds and cause a reschedule.
     return session->SendSyncRequest();
@@ -284,6 +286,8 @@ static ResultCode WaitSynchronization1(Kernel::Handle handle, s64 nano_seconds)
         // Create an event to wake the thread up after the specified nanosecond delay has passed
         thread->WakeAfterDelay(nano_seconds);
 
+        Core::System::GetInstance().PrepareReschedule();
+
         // Note: The output of this SVC will be set to RESULT_SUCCESS if the thread
         // resumes due to a signal in its wait objects.
         // Otherwise we retain the default value of timeout.
@@ -366,6 +370,8 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha
         // Create an event to wake the thread up after the specified nanosecond delay has passed
         thread->WakeAfterDelay(nano_seconds);
 
+        Core::System::GetInstance().PrepareReschedule();
+
         // This value gets set to -1 by default in this case, it is not modified after this.
         *out = -1;
         // Note: The output of this SVC will be set to RESULT_SUCCESS if the thread resumes due to
@@ -414,6 +420,8 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha
         // Create an event to wake the thread up after the specified nanosecond delay has passed
         thread->WakeAfterDelay(nano_seconds);
 
+        Core::System::GetInstance().PrepareReschedule();
+
         // Note: The output of this SVC will be set to RESULT_SUCCESS if the thread resumes due to a
         // signal in one of its wait objects.
         // Otherwise we retain the default value of timeout, and -1 in the out parameter
@@ -448,6 +456,9 @@ static ResultCode ArbitrateAddress(Kernel::Handle handle, u32 address, u32 type,
     auto res = arbiter->ArbitrateAddress(static_cast<Kernel::ArbitrationType>(type), address, value,
                                          nanoseconds);
 
+    // TODO(Subv): Identify in which specific cases this call should cause a reschedule.
+    Core::System::GetInstance().PrepareReschedule();
+
     return res;
 }
 
@@ -574,6 +585,8 @@ static ResultCode CreateThread(Kernel::Handle* out_handle, s32 priority, u32 ent
 
     CASCADE_RESULT(*out_handle, Kernel::g_handle_table.Create(std::move(thread)));
 
+    Core::System::GetInstance().PrepareReschedule();
+
     LOG_TRACE(Kernel_SVC, "called entrypoint=0x%08X (%s), arg=0x%08X, stacktop=0x%08X, "
                           "threadpriority=0x%08X, processorid=0x%08X : created handle=0x%08X",
               entry_point, name.c_str(), arg, stack_top, priority, processor_id, *out_handle);
@@ -586,6 +599,7 @@ static void ExitThread() {
     LOG_TRACE(Kernel_SVC, "called, pc=0x%08X", Core::CPU().GetPC());
 
     Kernel::ExitCurrentThread();
+    Core::System::GetInstance().PrepareReschedule();
 }
 
 /// Gets the priority for the specified thread
@@ -605,6 +619,7 @@ static ResultCode SetThreadPriority(Kernel::Handle handle, s32 priority) {
         return ERR_INVALID_HANDLE;
 
     thread->SetPriority(priority);
+    Core::System::GetInstance().PrepareReschedule();
     return RESULT_SUCCESS;
 }
 
@@ -849,6 +864,8 @@ static void SleepThread(s64 nanoseconds) {
 
     // Create an event to wake the thread up after the specified nanosecond delay has passed
     Kernel::GetCurrentThread()->WakeAfterDelay(nanoseconds);
+
+    Core::System::GetInstance().PrepareReschedule();
 }
 
 /// This returns the total CPU ticks elapsed since the CPU was powered-on
@@ -1184,8 +1201,6 @@ void CallSVC(u32 immediate) {
     if (info) {
         if (info->func) {
             info->func();
-            //  TODO(Subv): Not all service functions should cause a reschedule in all cases.
-            Core::System::GetInstance().PrepareReschedule();
         } else {
             LOG_ERROR(Kernel_SVC, "unimplemented SVC function %s(..)", info->name);
         }
-- 
cgit v1.2.3


From e6a7723f2f4b62279cd4f6d4b48eb02a9b60ffb6 Mon Sep 17 00:00:00 2001
From: Subv <subv2112@gmail.com>
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/event.cpp          |  6 +++---
 src/core/hle/kernel/event.h            |  4 ++--
 src/core/hle/kernel/kernel.cpp         | 14 ++++++--------
 src/core/hle/kernel/kernel.h           |  9 +++++----
 src/core/hle/kernel/mutex.cpp          | 24 ++++++------------------
 src/core/hle/kernel/mutex.h            |  5 +++--
 src/core/hle/kernel/semaphore.cpp      |  6 +++---
 src/core/hle/kernel/semaphore.h        |  4 ++--
 src/core/hle/kernel/server_port.cpp    |  6 +++---
 src/core/hle/kernel/server_port.h      |  4 ++--
 src/core/hle/kernel/server_session.cpp |  6 +++---
 src/core/hle/kernel/server_session.h   |  4 ++--
 src/core/hle/kernel/thread.cpp         |  6 +++---
 src/core/hle/kernel/thread.h           |  4 ++--
 src/core/hle/kernel/timer.cpp          |  6 +++---
 src/core/hle/kernel/timer.h            |  4 ++--
 src/core/hle/svc.cpp                   | 12 ++++++------
 17 files changed, 56 insertions(+), 68 deletions(-)

(limited to 'src/core')

diff --git a/src/core/hle/kernel/event.cpp b/src/core/hle/kernel/event.cpp
index 3e116e3df..e1f42af05 100644
--- a/src/core/hle/kernel/event.cpp
+++ b/src/core/hle/kernel/event.cpp
@@ -30,12 +30,12 @@ SharedPtr<Event> Event::Create(ResetType reset_type, std::string name) {
     return evt;
 }
 
-bool Event::ShouldWait() {
+bool Event::ShouldWait(Thread* thread) const {
     return !signaled;
 }
 
-void Event::Acquire() {
-    ASSERT_MSG(!ShouldWait(), "object unavailable!");
+void Event::Acquire(Thread* thread) {
+    ASSERT_MSG(!ShouldWait(thread), "object unavailable!");
 
     // Release the event if it's not sticky...
     if (reset_type != ResetType::Sticky)
diff --git a/src/core/hle/kernel/event.h b/src/core/hle/kernel/event.h
index 8dcd23edb..39452bf33 100644
--- a/src/core/hle/kernel/event.h
+++ b/src/core/hle/kernel/event.h
@@ -35,8 +35,8 @@ public:
     bool signaled;    ///< Whether the event has already been signaled
     std::string name; ///< Name of event (optional)
 
-    bool ShouldWait() override;
-    void Acquire() override;
+    bool ShouldWait(Thread* thread) const override;
+    void Acquire(Thread* thread) override;
 
     void Signal();
     void Clear();
diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp
index 1db8e102f..ef9dbafa5 100644
--- a/src/core/hle/kernel/kernel.cpp
+++ b/src/core/hle/kernel/kernel.cpp
@@ -39,11 +39,6 @@ SharedPtr<Thread> WaitObject::GetHighestPriorityReadyThread() {
                thread->status == THREADSTATUS_DEAD;
     });
 
-    // TODO(Subv): This call should be performed inside the loop below to check if an object can be
-    // acquired by a particular thread. This is useful for things like recursive locking of Mutexes.
-    if (ShouldWait())
-        return nullptr;
-
     Thread* candidate = nullptr;
     s32 candidate_priority = THREADPRIO_LOWEST + 1;
 
@@ -51,9 +46,12 @@ SharedPtr<Thread> WaitObject::GetHighestPriorityReadyThread() {
         if (thread->current_priority >= candidate_priority)
             continue;
 
+        if (ShouldWait(thread.get()))
+            continue;
+
         bool ready_to_run =
             std::none_of(thread->wait_objects.begin(), thread->wait_objects.end(),
-                         [](const SharedPtr<WaitObject>& object) { return object->ShouldWait(); });
+                         [&thread](const SharedPtr<WaitObject>& object) { return object->ShouldWait(thread.get()); });
         if (ready_to_run) {
             candidate = thread.get();
             candidate_priority = thread->current_priority;
@@ -66,7 +64,7 @@ SharedPtr<Thread> WaitObject::GetHighestPriorityReadyThread() {
 void WaitObject::WakeupAllWaitingThreads() {
     while (auto thread = GetHighestPriorityReadyThread()) {
         if (!thread->IsSleepingOnWaitAll()) {
-            Acquire();
+            Acquire(thread.get());
             // Set the output index of the WaitSynchronizationN call to the index of this object.
             if (thread->wait_set_output) {
                 thread->SetWaitSynchronizationOutput(thread->GetWaitObjectIndex(this));
@@ -74,7 +72,7 @@ void WaitObject::WakeupAllWaitingThreads() {
             }
         } else {
             for (auto& object : thread->wait_objects) {
-                object->Acquire();
+                object->Acquire(thread.get());
                 object->RemoveWaitingThread(thread.get());
             }
             // Note: This case doesn't update the output index of WaitSynchronizationN.
diff --git a/src/core/hle/kernel/kernel.h b/src/core/hle/kernel/kernel.h
index 9503e7d04..67eae93f2 100644
--- a/src/core/hle/kernel/kernel.h
+++ b/src/core/hle/kernel/kernel.h
@@ -132,13 +132,14 @@ using SharedPtr = boost::intrusive_ptr<T>;
 class WaitObject : public Object {
 public:
     /**
-     * Check if the current thread should wait until the object is available
+     * Check if the specified thread should wait until the object is available
+     * @param thread The thread about which we're deciding.
      * @return True if the current thread should wait due to this object being unavailable
      */
-    virtual bool ShouldWait() = 0;
+    virtual bool ShouldWait(Thread* thread) const = 0;
 
-    /// Acquire/lock the object if it is available
-    virtual void Acquire() = 0;
+    /// Acquire/lock the object for the specified thread if it is available
+    virtual void Acquire(Thread* thread) = 0;
 
     /**
      * Add a thread to wait on this object
diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp
index 8d92a9b8e..072e4e7c1 100644
--- a/src/core/hle/kernel/mutex.cpp
+++ b/src/core/hle/kernel/mutex.cpp
@@ -40,31 +40,19 @@ SharedPtr<Mutex> Mutex::Create(bool initial_locked, std::string name) {
     mutex->name = std::move(name);
     mutex->holding_thread = nullptr;
 
-    // Acquire mutex with current thread if initialized as locked...
+    // Acquire mutex with current thread if initialized as locked
     if (initial_locked)
-        mutex->Acquire();
+        mutex->Acquire(GetCurrentThread());
 
     return mutex;
 }
 
-bool Mutex::ShouldWait() {
-    auto thread = GetCurrentThread();
-    bool wait = lock_count > 0 && holding_thread != thread;
-
-    // If the holding thread of the mutex is lower priority than this thread, that thread should
-    // temporarily inherit this thread's priority
-    if (wait && thread->current_priority < holding_thread->current_priority)
-        holding_thread->BoostPriority(thread->current_priority);
-
-    return wait;
-}
-
-void Mutex::Acquire() {
-    Acquire(GetCurrentThread());
+bool Mutex::ShouldWait(Thread* thread) const {
+    return lock_count > 0 && thread != holding_thread;
 }
 
-void Mutex::Acquire(SharedPtr<Thread> thread) {
-    ASSERT_MSG(!ShouldWait(), "object unavailable!");
+void Mutex::Acquire(Thread* thread) {
+    ASSERT_MSG(!ShouldWait(thread), "object unavailable!");
 
     // Actually "acquire" the mutex only if we don't already have it...
     if (lock_count == 0) {
diff --git a/src/core/hle/kernel/mutex.h b/src/core/hle/kernel/mutex.h
index 53c3dc1f1..98b3d40b5 100644
--- a/src/core/hle/kernel/mutex.h
+++ b/src/core/hle/kernel/mutex.h
@@ -38,8 +38,9 @@ public:
     std::string name;                 ///< Name of mutex (optional)
     SharedPtr<Thread> holding_thread; ///< Thread that has acquired the mutex
 
-    bool ShouldWait() override;
-    void Acquire() override;
+    bool ShouldWait(Thread* thread) const override;
+    void Acquire(Thread* thread) override;
+
 
     /**
      * Acquires the specified mutex for the specified thread
diff --git a/src/core/hle/kernel/semaphore.cpp b/src/core/hle/kernel/semaphore.cpp
index bf7600780..5e6139265 100644
--- a/src/core/hle/kernel/semaphore.cpp
+++ b/src/core/hle/kernel/semaphore.cpp
@@ -30,12 +30,12 @@ ResultVal<SharedPtr<Semaphore>> Semaphore::Create(s32 initial_count, s32 max_cou
     return MakeResult<SharedPtr<Semaphore>>(std::move(semaphore));
 }
 
-bool Semaphore::ShouldWait() {
+bool Semaphore::ShouldWait(Thread* thread) const {
     return available_count <= 0;
 }
 
-void Semaphore::Acquire() {
-    ASSERT_MSG(!ShouldWait(), "object unavailable!");
+void Semaphore::Acquire(Thread* thread) {
+    ASSERT_MSG(!ShouldWait(thread), "object unavailable!");
     --available_count;
 }
 
diff --git a/src/core/hle/kernel/semaphore.h b/src/core/hle/kernel/semaphore.h
index e01908a25..cde94f7cc 100644
--- a/src/core/hle/kernel/semaphore.h
+++ b/src/core/hle/kernel/semaphore.h
@@ -39,8 +39,8 @@ public:
     s32 available_count; ///< Number of free slots left in the semaphore
     std::string name;    ///< Name of semaphore (optional)
 
-    bool ShouldWait() override;
-    void Acquire() override;
+    bool ShouldWait(Thread* thread) const override;
+    void Acquire(Thread* thread) override;
 
     /**
      * Releases a certain number of slots from a semaphore.
diff --git a/src/core/hle/kernel/server_port.cpp b/src/core/hle/kernel/server_port.cpp
index 6c19aa7c0..fd3bbbcad 100644
--- a/src/core/hle/kernel/server_port.cpp
+++ b/src/core/hle/kernel/server_port.cpp
@@ -14,13 +14,13 @@ namespace Kernel {
 ServerPort::ServerPort() {}
 ServerPort::~ServerPort() {}
 
-bool ServerPort::ShouldWait() {
+bool ServerPort::ShouldWait(Thread* thread) const {
     // If there are no pending sessions, we wait until a new one is added.
     return pending_sessions.size() == 0;
 }
 
-void ServerPort::Acquire() {
-    ASSERT_MSG(!ShouldWait(), "object unavailable!");
+void ServerPort::Acquire(Thread* thread) {
+    ASSERT_MSG(!ShouldWait(thread), "object unavailable!");
 }
 
 std::tuple<SharedPtr<ServerPort>, SharedPtr<ClientPort>> ServerPort::CreatePortPair(
diff --git a/src/core/hle/kernel/server_port.h b/src/core/hle/kernel/server_port.h
index b0f8df62c..6f8bdb6a9 100644
--- a/src/core/hle/kernel/server_port.h
+++ b/src/core/hle/kernel/server_port.h
@@ -53,8 +53,8 @@ public:
     /// ServerSessions created from this port inherit a reference to this handler.
     std::shared_ptr<Service::SessionRequestHandler> hle_handler;
 
-    bool ShouldWait() override;
-    void Acquire() override;
+    bool ShouldWait(Thread* thread) const override;
+    void Acquire(Thread* thread) override;
 
 private:
     ServerPort();
diff --git a/src/core/hle/kernel/server_session.cpp b/src/core/hle/kernel/server_session.cpp
index 146458c1c..9447ff236 100644
--- a/src/core/hle/kernel/server_session.cpp
+++ b/src/core/hle/kernel/server_session.cpp
@@ -29,12 +29,12 @@ ResultVal<SharedPtr<ServerSession>> ServerSession::Create(
     return MakeResult<SharedPtr<ServerSession>>(std::move(server_session));
 }
 
-bool ServerSession::ShouldWait() {
+bool ServerSession::ShouldWait(Thread* thread) const {
     return !signaled;
 }
 
-void ServerSession::Acquire() {
-    ASSERT_MSG(!ShouldWait(), "object unavailable!");
+void ServerSession::Acquire(Thread* thread) {
+    ASSERT_MSG(!ShouldWait(thread), "object unavailable!");
     signaled = false;
 }
 
diff --git a/src/core/hle/kernel/server_session.h b/src/core/hle/kernel/server_session.h
index 458284a5d..c088b9a19 100644
--- a/src/core/hle/kernel/server_session.h
+++ b/src/core/hle/kernel/server_session.h
@@ -57,9 +57,9 @@ public:
      */
     ResultCode HandleSyncRequest();
 
-    bool ShouldWait() override;
+    bool ShouldWait(Thread* thread) const override;
 
-    void Acquire() override;
+    void Acquire(Thread* thread) override;
 
     std::string name; ///< The name of this session (optional)
     bool signaled;    ///< Whether there's new data available to this ServerSession
diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp
index 5fb95dada..7d03a2cf7 100644
--- a/src/core/hle/kernel/thread.cpp
+++ b/src/core/hle/kernel/thread.cpp
@@ -27,12 +27,12 @@ namespace Kernel {
 /// Event type for the thread wake up event
 static int ThreadWakeupEventType;
 
-bool Thread::ShouldWait() {
+bool Thread::ShouldWait(Thread* thread) const {
     return status != THREADSTATUS_DEAD;
 }
 
-void Thread::Acquire() {
-    ASSERT_MSG(!ShouldWait(), "object unavailable!");
+void Thread::Acquire(Thread* thread) {
+    ASSERT_MSG(!ShouldWait(thread), "object unavailable!");
 }
 
 // TODO(yuriks): This can be removed if Thread objects are explicitly pooled in the future, allowing
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
diff --git a/src/core/hle/kernel/timer.cpp b/src/core/hle/kernel/timer.cpp
index b50cf520d..8f2bc4c7f 100644
--- a/src/core/hle/kernel/timer.cpp
+++ b/src/core/hle/kernel/timer.cpp
@@ -39,12 +39,12 @@ SharedPtr<Timer> Timer::Create(ResetType reset_type, std::string name) {
     return timer;
 }
 
-bool Timer::ShouldWait() {
+bool Timer::ShouldWait(Thread* thread) const {
     return !signaled;
 }
 
-void Timer::Acquire() {
-    ASSERT_MSG(!ShouldWait(), "object unavailable!");
+void Timer::Acquire(Thread* thread) {
+    ASSERT_MSG(!ShouldWait(thread), "object unavailable!");
 
     if (reset_type == ResetType::OneShot)
         signaled = false;
diff --git a/src/core/hle/kernel/timer.h b/src/core/hle/kernel/timer.h
index 18ea0236b..2e3b31b23 100644
--- a/src/core/hle/kernel/timer.h
+++ b/src/core/hle/kernel/timer.h
@@ -39,8 +39,8 @@ public:
     u64 initial_delay;  ///< The delay until the timer fires for the first time
     u64 interval_delay; ///< The delay until the timer fires after the first time
 
-    bool ShouldWait() override;
-    void Acquire() override;
+    bool ShouldWait(Thread* thread) const override;
+    void Acquire(Thread* thread) override;
 
     /**
      * Starts the timer, with the specified initial delay and interval.
diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp
index 5b538be22..159ac0bf6 100644
--- a/src/core/hle/svc.cpp
+++ b/src/core/hle/svc.cpp
@@ -272,7 +272,7 @@ static ResultCode WaitSynchronization1(Kernel::Handle handle, s64 nano_seconds)
     LOG_TRACE(Kernel_SVC, "called handle=0x%08X(%s:%s), nanoseconds=%lld", handle,
               object->GetTypeName().c_str(), object->GetName().c_str(), nano_seconds);
 
-    if (object->ShouldWait()) {
+    if (object->ShouldWait(thread)) {
 
         if (nano_seconds == 0)
             return ERR_SYNC_TIMEOUT;
@@ -294,7 +294,7 @@ static ResultCode WaitSynchronization1(Kernel::Handle handle, s64 nano_seconds)
         return ERR_SYNC_TIMEOUT;
     }
 
-    object->Acquire();
+    object->Acquire(thread);
 
     return RESULT_SUCCESS;
 }
@@ -336,11 +336,11 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha
     if (wait_all) {
         bool all_available =
             std::all_of(objects.begin(), objects.end(),
-                        [](const ObjectPtr& object) { return !object->ShouldWait(); });
+                        [thread](const ObjectPtr& object) { return !object->ShouldWait(thread); });
         if (all_available) {
             // We can acquire all objects right now, do so.
             for (auto& object : objects)
-                object->Acquire();
+                object->Acquire(thread);
             // Note: In this case, the `out` parameter is not set,
             // and retains whatever value it had before.
             return RESULT_SUCCESS;
@@ -380,12 +380,12 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha
     } else {
         // Find the first object that is acquirable in the provided list of objects
         auto itr = std::find_if(objects.begin(), objects.end(),
-                                [](const ObjectPtr& object) { return !object->ShouldWait(); });
+                                [thread](const ObjectPtr& object) { return !object->ShouldWait(thread); });
 
         if (itr != objects.end()) {
             // We found a ready object, acquire it and set the result value
             Kernel::WaitObject* object = itr->get();
-            object->Acquire();
+            object->Acquire(thread);
             *out = std::distance(objects.begin(), itr);
             return RESULT_SUCCESS;
         }
-- 
cgit v1.2.3


From 7abf1853907fe086753df0031262b668a2da88b0 Mon Sep 17 00:00:00 2001
From: Subv <subv2112@gmail.com>
Date: Sun, 1 Jan 2017 16:59:30 -0500
Subject: Kernel/Mutex: Implemented priority inheritance.

The implementation is based on reverse engineering of the 3DS's kernel.

A mutex holder's priority will be temporarily boosted to the best priority among any threads that want to acquire any of its held mutexes.
When the holder releases the mutex, it's priority will be boosted to the best priority among the threads that want to acquire any of its remaining held mutexes.
---
 src/core/hle/kernel/kernel.h   |  2 +-
 src/core/hle/kernel/mutex.cpp  | 58 ++++++++++++++++++++++++++++++++----------
 src/core/hle/kernel/mutex.h    |  7 ++---
 src/core/hle/kernel/thread.cpp |  6 ++---
 src/core/hle/svc.cpp           |  9 -------
 5 files changed, 51 insertions(+), 31 deletions(-)

(limited to 'src/core')

diff --git a/src/core/hle/kernel/kernel.h b/src/core/hle/kernel/kernel.h
index 67eae93f2..2680f89c9 100644
--- a/src/core/hle/kernel/kernel.h
+++ b/src/core/hle/kernel/kernel.h
@@ -145,7 +145,7 @@ public:
      * Add a thread to wait on this object
      * @param thread Pointer to thread to add
      */
-    void AddWaitingThread(SharedPtr<Thread> thread);
+    virtual void AddWaitingThread(SharedPtr<Thread> thread);
 
     /**
      * Removes a thread from waiting on this object (e.g. if it was resumed already)
diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp
index 072e4e7c1..e83717e80 100644
--- a/src/core/hle/kernel/mutex.cpp
+++ b/src/core/hle/kernel/mutex.cpp
@@ -6,6 +6,7 @@
 #include <vector>
 #include <boost/range/algorithm_ext/erase.hpp>
 #include "common/assert.h"
+#include "core/core.h"
 #include "core/hle/kernel/kernel.h"
 #include "core/hle/kernel/mutex.h"
 #include "core/hle/kernel/thread.h"
@@ -13,19 +14,25 @@
 namespace Kernel {
 
 /**
- * Resumes a thread waiting for the specified mutex
- * @param mutex The mutex that some thread is waiting on
+ * 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 ResumeWaitingThread(Mutex* mutex) {
-    // Reset mutex lock thread handle, nothing is waiting
-    mutex->lock_count = 0;
-    mutex->holding_thread = nullptr;
-    mutex->WakeupAllWaitingThreads();
+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);
 }
 
 void ReleaseThreadMutexes(Thread* thread) {
     for (auto& mtx : thread->held_mutexes) {
-        ResumeWaitingThread(mtx.get());
+        mtx->lock_count = 0;
+        mtx->holding_thread = nullptr;
+        mtx->WakeupAllWaitingThreads();
     }
     thread->held_mutexes.clear();
 }
@@ -54,27 +61,52 @@ bool Mutex::ShouldWait(Thread* thread) const {
 void Mutex::Acquire(Thread* thread) {
     ASSERT_MSG(!ShouldWait(thread), "object unavailable!");
 
-    // Actually "acquire" the mutex only if we don't already have it...
+    // Actually "acquire" the mutex only if we don't already have it
     if (lock_count == 0) {
+        priority = thread->current_priority;
         thread->held_mutexes.insert(this);
-        holding_thread = std::move(thread);
+        holding_thread = thread;
+
+        UpdateThreadPriority(thread);
+
+        Core::System::GetInstance().PrepareReschedule();
     }
 
     lock_count++;
 }
 
 void Mutex::Release() {
-    // Only release if the mutex is held...
+    // Only release if the mutex is held
     if (lock_count > 0) {
         lock_count--;
 
-        // Yield to the next thread only if we've fully released the mutex...
+        // Yield to the next thread only if we've fully released the mutex
         if (lock_count == 0) {
             holding_thread->held_mutexes.erase(this);
-            ResumeWaitingThread(this);
+            UpdateThreadPriority(holding_thread.get());
+            holding_thread = nullptr;
+            WakeupAllWaitingThreads();
             Core::System::GetInstance().PrepareReschedule();
         }
     }
 }
 
+void Mutex::AddWaitingThread(SharedPtr<Thread> thread) {
+    WaitObject::AddWaitingThread(thread);
+
+    // Elevate the mutex priority to the best priority
+    // among the priorities of all its waiting threads.
+
+    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;
+        UpdateThreadPriority(holding_thread.get());
+    }
+}
+
 } // namespace
diff --git a/src/core/hle/kernel/mutex.h b/src/core/hle/kernel/mutex.h
index 98b3d40b5..3e6adeb17 100644
--- a/src/core/hle/kernel/mutex.h
+++ b/src/core/hle/kernel/mutex.h
@@ -35,18 +35,15 @@ public:
     }
 
     int lock_count;                   ///< Number of times the mutex has been acquired
+    u32 priority;                     ///< The priority of the mutex, used for priority inheritance.
     std::string name;                 ///< Name of mutex (optional)
     SharedPtr<Thread> holding_thread; ///< Thread that has acquired the mutex
 
     bool ShouldWait(Thread* thread) const override;
     void Acquire(Thread* thread) override;
 
+    void AddWaitingThread(SharedPtr<Thread> thread) override;
 
-    /**
-     * Acquires the specified mutex for the specified thread
-     * @param thread Thread that will acquire the mutex
-     */
-    void Acquire(SharedPtr<Thread> thread);
     void Release();
 
 private:
diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp
index 7d03a2cf7..d44010824 100644
--- a/src/core/hle/kernel/thread.cpp
+++ b/src/core/hle/kernel/thread.cpp
@@ -90,9 +90,6 @@ static bool CheckWait_AddressArbiter(const Thread* thread, VAddr wait_address) {
 }
 
 void Thread::Stop() {
-    // Release all the mutexes that this thread holds
-    ReleaseThreadMutexes(this);
-
     // Cancel any outstanding wakeup events for this thread
     CoreTiming::UnscheduleEvent(ThreadWakeupEventType, callback_handle);
     wakeup_callback_handle_table.Close(callback_handle);
@@ -108,6 +105,9 @@ 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);
diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp
index 159ac0bf6..5d6359344 100644
--- a/src/core/hle/svc.cpp
+++ b/src/core/hle/svc.cpp
@@ -278,9 +278,6 @@ static ResultCode WaitSynchronization1(Kernel::Handle handle, s64 nano_seconds)
             return ERR_SYNC_TIMEOUT;
 
         object->AddWaitingThread(thread);
-        // TODO(Subv): Perform things like update the mutex lock owner's priority to
-        // prevent priority inversion. Currently this is done in Mutex::ShouldWait,
-        // but it should be moved to a function that is called from here.
         thread->status = THREADSTATUS_WAIT_SYNCH;
 
         // Create an event to wake the thread up after the specified nanosecond delay has passed
@@ -359,9 +356,6 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha
         // Add the thread to each of the objects' waiting threads.
         for (auto& object : objects) {
             object->AddWaitingThread(thread);
-            // TODO(Subv): Perform things like update the mutex lock owner's priority to
-            // prevent priority inversion. Currently this is done in Mutex::ShouldWait,
-            // but it should be moved to a function that is called from here.
         }
 
         // Set the thread's waitlist to the list of objects passed to WaitSynchronizationN
@@ -409,9 +403,6 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha
             // Set the index of this object in the mapping of Objects -> index for this thread.
             thread->wait_objects_index[object->GetObjectId()] = static_cast<int>(i);
             object->AddWaitingThread(thread);
-            // TODO(Subv): Perform things like update the mutex lock owner's priority to
-            // prevent priority inversion. Currently this is done in Mutex::ShouldWait,
-            // but it should be moved to a function that is called from here.
         }
 
         // Note: If no handles and no timeout were given, then the thread will deadlock, this is
-- 
cgit v1.2.3


From b6a0355568ee327bef8957b9a2498897b96e1278 Mon Sep 17 00:00:00 2001
From: Subv <subv2112@gmail.com>
Date: Mon, 2 Jan 2017 13:53:10 -0500
Subject: Kernel/Mutex: Update a mutex priority when a thread stops waiting on
 it.

---
 src/core/hle/kernel/kernel.cpp | 23 +++++++++++++++--------
 src/core/hle/kernel/kernel.h   |  2 +-
 src/core/hle/kernel/mutex.cpp  | 35 ++++++++++++++++++++++-------------
 src/core/hle/kernel/mutex.h    |  1 +
 src/core/hle/svc.cpp           |  5 +++--
 5 files changed, 42 insertions(+), 24 deletions(-)

(limited to 'src/core')

diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp
index ef9dbafa5..6f61d526a 100644
--- a/src/core/hle/kernel/kernel.cpp
+++ b/src/core/hle/kernel/kernel.cpp
@@ -3,7 +3,6 @@
 // Refer to the license.txt file included.
 
 #include <algorithm>
-#include <boost/range/algorithm_ext/erase.hpp>
 #include "common/assert.h"
 #include "common/logging/log.h"
 #include "core/hle/config_mem.h"
@@ -34,10 +33,17 @@ void WaitObject::RemoveWaitingThread(Thread* thread) {
 
 SharedPtr<Thread> WaitObject::GetHighestPriorityReadyThread() {
     // Remove the threads that are ready or already running from our waitlist
-    boost::range::remove_erase_if(waiting_threads, [](const SharedPtr<Thread>& thread) {
-        return thread->status == THREADSTATUS_RUNNING || thread->status == THREADSTATUS_READY ||
-               thread->status == THREADSTATUS_DEAD;
-    });
+    auto to_remove = waiting_threads.end();
+    do {
+        to_remove = std::find_if(waiting_threads.begin(), waiting_threads.end(),
+                                 [](const SharedPtr<Thread>& thread) {
+                                    return thread->status == THREADSTATUS_RUNNING ||
+                                           thread->status == THREADSTATUS_READY ||
+                                           thread->status == THREADSTATUS_DEAD;
+        });
+        // Call RemoveWaitingThread so that child classes can override the behavior.
+        RemoveWaitingThread(to_remove->get());
+    } while (to_remove != waiting_threads.end());
 
     Thread* candidate = nullptr;
     s32 candidate_priority = THREADPRIO_LOWEST + 1;
@@ -49,9 +55,10 @@ SharedPtr<Thread> WaitObject::GetHighestPriorityReadyThread() {
         if (ShouldWait(thread.get()))
             continue;
 
-        bool ready_to_run =
-            std::none_of(thread->wait_objects.begin(), thread->wait_objects.end(),
-                         [&thread](const SharedPtr<WaitObject>& object) { return object->ShouldWait(thread.get()); });
+        bool ready_to_run = std::none_of(thread->wait_objects.begin(), thread->wait_objects.end(),
+                                        [&thread](const SharedPtr<WaitObject>& 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/kernel.h b/src/core/hle/kernel/kernel.h
index 2680f89c9..05097824b 100644
--- a/src/core/hle/kernel/kernel.h
+++ b/src/core/hle/kernel/kernel.h
@@ -151,7 +151,7 @@ public:
      * Removes a thread from waiting on this object (e.g. if it was resumed already)
      * @param thread Pointer to thread to remove
      */
-    void RemoveWaitingThread(Thread* thread);
+    virtual void RemoveWaitingThread(Thread* thread);
 
     /**
      * Wake up all threads waiting on this object that can be awoken, in priority order,
diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp
index e83717e80..84ff65150 100644
--- a/src/core/hle/kernel/mutex.cpp
+++ b/src/core/hle/kernel/mutex.cpp
@@ -28,6 +28,23 @@ static void UpdateThreadPriority(Thread* thread) {
     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;
@@ -93,20 +110,12 @@ void Mutex::Release() {
 
 void Mutex::AddWaitingThread(SharedPtr<Thread> thread) {
     WaitObject::AddWaitingThread(thread);
+    UpdateMutexPriority(this);
+}
 
-    // Elevate the mutex priority to the best priority
-    // among the priorities of all its waiting threads.
-
-    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;
-        UpdateThreadPriority(holding_thread.get());
-    }
+void Mutex::RemoveWaitingThread(Thread* thread) {
+    WaitObject::RemoveWaitingThread(thread);
+    UpdateMutexPriority(this);
 }
 
 } // namespace
diff --git a/src/core/hle/kernel/mutex.h b/src/core/hle/kernel/mutex.h
index 3e6adeb17..31f920516 100644
--- a/src/core/hle/kernel/mutex.h
+++ b/src/core/hle/kernel/mutex.h
@@ -43,6 +43,7 @@ public:
     void Acquire(Thread* thread) override;
 
     void AddWaitingThread(SharedPtr<Thread> thread) override;
+    void RemoveWaitingThread(Thread* thread) override;
 
     void Release();
 
diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp
index 5d6359344..b6e34a9e9 100644
--- a/src/core/hle/svc.cpp
+++ b/src/core/hle/svc.cpp
@@ -373,8 +373,9 @@ static ResultCode WaitSynchronizationN(s32* out, Kernel::Handle* handles, s32 ha
         return ERR_SYNC_TIMEOUT;
     } else {
         // Find the first object that is acquirable in the provided list of objects
-        auto itr = std::find_if(objects.begin(), objects.end(),
-                                [thread](const ObjectPtr& object) { return !object->ShouldWait(thread); });
+        auto itr = std::find_if(objects.begin(), objects.end(), [thread](const ObjectPtr& object) {
+            return !object->ShouldWait(thread);
+        });
 
         if (itr != objects.end()) {
             // We found a ready object, acquire it and set the result value
-- 
cgit v1.2.3


From d3ff5b91e14356912589f9bac47fccbe79e07279 Mon Sep 17 00:00:00 2001
From: Subv <subv2112@gmail.com>
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')

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> 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<Thread> 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<SharedPtr<Mutex>> held_mutexes;
 
+    /// Mutexes that this thread is currently waiting for.
+    boost::container::flat_set<SharedPtr<Mutex>> pending_mutexes;
+
     SharedPtr<Process> 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 <subv2112@gmail.com>
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/core/hle/kernel/thread.cpp | 11 +++++++----
 src/core/hle/kernel/thread.h   | 19 ++++++++++---------
 src/core/hle/svc.cpp           |  6 +++---
 3 files changed, 20 insertions(+), 16 deletions(-)

(limited to 'src/core')

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<SharedPtr<WaitObject>> 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 <subv2112@gmail.com>
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')

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<Thread> 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<WaitObject>& 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<Process> 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<SharedPtr<WaitObject>> wait_objects;
 
-    /// Mapping of Object ids to their position in the last waitlist that this object waited on.
-    boost::container::flat_map<int, s32> 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<int>(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


From 7f1dca8cd26fe9be0080efee4e456630960af459 Mon Sep 17 00:00:00 2001
From: Subv <subv2112@gmail.com>
Date: Wed, 4 Jan 2017 11:37:19 -0500
Subject: Kernel: Remove a thread from all of its waiting objects'
 waiting_threads list when it is awoken.

This fixes a potential bug where threads would not get removed from said list if they awoke after waiting with WaitSynchronizationN with wait_all = false
---
 src/core/hle/kernel/kernel.cpp | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

(limited to 'src/core')

diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp
index 955f50a9b..47d4df69c 100644
--- a/src/core/hle/kernel/kernel.cpp
+++ b/src/core/hle/kernel/kernel.cpp
@@ -32,19 +32,6 @@ void WaitObject::RemoveWaitingThread(Thread* thread) {
 }
 
 SharedPtr<Thread> WaitObject::GetHighestPriorityReadyThread() {
-    // Remove the threads that are ready or already running from our waitlist
-    auto to_remove = waiting_threads.end();
-    do {
-        to_remove = std::find_if(waiting_threads.begin(), waiting_threads.end(),
-                                 [](const SharedPtr<Thread>& thread) {
-                                    return thread->status == THREADSTATUS_RUNNING ||
-                                           thread->status == THREADSTATUS_READY ||
-                                           thread->status == THREADSTATUS_DEAD;
-        });
-        // Call RemoveWaitingThread so that child classes can override the behavior.
-        RemoveWaitingThread(to_remove->get());
-    } while (to_remove != waiting_threads.end());
-
     Thread* candidate = nullptr;
     s32 candidate_priority = THREADPRIO_LOWEST + 1;
 
@@ -86,17 +73,16 @@ void WaitObject::WakeupAllWaitingThreads() {
         } else {
             for (auto& object : thread->wait_objects) {
                 object->Acquire(thread.get());
-                object->RemoveWaitingThread(thread.get());
             }
             // Note: This case doesn't update the output index of WaitSynchronizationN.
-            // Clear the thread's waitlist
-            thread->wait_objects.clear();
         }
 
+        for (auto& object : thread->wait_objects)
+            object->RemoveWaitingThread(thread.get());
+        thread->wait_objects.clear();
+
         thread->SetWaitSynchronizationResult(RESULT_SUCCESS);
         thread->ResumeFromWait();
-        // Note: Removing the thread from the object's waitlist will be
-        // done by GetHighestPriorityReadyThread.
     }
 }
 
-- 
cgit v1.2.3


From dda4ec93bea089e3286e9a965378d9411f480acd Mon Sep 17 00:00:00 2001
From: Subv <subv2112@gmail.com>
Date: Wed, 4 Jan 2017 12:48:13 -0500
Subject: Kernel: Add some asserts to enforce the invariants in the scheduler.

---
 src/core/hle/kernel/kernel.cpp | 8 ++++++++
 src/core/hle/kernel/thread.cpp | 7 +++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

(limited to 'src/core')

diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp
index 47d4df69c..f599916f0 100644
--- a/src/core/hle/kernel/kernel.cpp
+++ b/src/core/hle/kernel/kernel.cpp
@@ -27,6 +27,9 @@ void WaitObject::AddWaitingThread(SharedPtr<Thread> thread) {
 
 void WaitObject::RemoveWaitingThread(Thread* thread) {
     auto itr = std::find(waiting_threads.begin(), waiting_threads.end(), thread);
+    // If a thread passed multiple handles to the same object,
+    // the kernel might attempt to remove the thread from the object's
+    // waiting threads list multiple times.
     if (itr != waiting_threads.end())
         waiting_threads.erase(itr);
 }
@@ -36,6 +39,11 @@ SharedPtr<Thread> WaitObject::GetHighestPriorityReadyThread() {
     s32 candidate_priority = THREADPRIO_LOWEST + 1;
 
     for (const auto& thread : waiting_threads) {
+        // The list of waiting threads must not contain threads that are not waiting to be awakened.
+        ASSERT_MSG(thread->status == THREADSTATUS_WAIT_SYNCH_ANY ||
+                       thread->status == THREADSTATUS_WAIT_SYNCH_ALL,
+                   "Inconsistent thread statuses in waiting_threads");
+
         if (thread->current_priority >= candidate_priority)
             continue;
 
diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp
index 568cef5b9..9109bd10b 100644
--- a/src/core/hle/kernel/thread.cpp
+++ b/src/core/hle/kernel/thread.cpp
@@ -200,8 +200,8 @@ static void SwitchContext(Thread* new_thread) {
 
     // Load context of new thread
     if (new_thread) {
-        DEBUG_ASSERT_MSG(new_thread->status == THREADSTATUS_READY,
-                         "Thread must be ready to become running.");
+        ASSERT_MSG(new_thread->status == THREADSTATUS_READY,
+                   "Thread must be ready to become running.");
 
         // Cancel any outstanding wakeup events for this thread
         CoreTiming::UnscheduleEvent(ThreadWakeupEventType, new_thread->callback_handle);
@@ -307,6 +307,8 @@ void Thread::WakeAfterDelay(s64 nanoseconds) {
 }
 
 void Thread::ResumeFromWait() {
+    ASSERT_MSG(wait_objects.empty(), "Thread is waking up while waiting for objects");
+
     switch (status) {
     case THREADSTATUS_WAIT_SYNCH_ALL:
     case THREADSTATUS_WAIT_SYNCH_ANY:
@@ -580,6 +582,7 @@ void Thread::SetWaitSynchronizationOutput(s32 output) {
 }
 
 s32 Thread::GetWaitObjectIndex(WaitObject* object) const {
+    ASSERT_MSG(!wait_objects.empty(), "Thread is not waiting for anything");
     auto match = std::find(wait_objects.rbegin(), wait_objects.rend(), object);
     return std::distance(match, wait_objects.rend()) - 1;
 }
-- 
cgit v1.2.3