From 7b9f428b23e1761e7b6c177d2e8eb9219ac6b7f6 Mon Sep 17 00:00:00 2001
From: bunnei <bunneidev@gmail.com>
Date: Mon, 23 Mar 2015 23:55:21 -0400
Subject: [PATCH] Thread: Implement priority boost for starved threads.

SVC: Return correct error code on invalid CreateThread processor ID.

SVC: Assert when creating a thread with an invalid userland priority.
---
 src/common/thread_queue_list.h   | 18 ++++++++++++
 src/core/hle/function_wrappers.h |  7 +++++
 src/core/hle/kernel/thread.cpp   | 48 ++++++++++++++++++++++++--------
 src/core/hle/kernel/thread.h     | 25 ++++++++++-------
 src/core/hle/svc.cpp             | 22 +++++++++++----
 5 files changed, 92 insertions(+), 28 deletions(-)

diff --git a/src/common/thread_queue_list.h b/src/common/thread_queue_list.h
index 444abf1151..4f27fc8992 100644
--- a/src/common/thread_queue_list.h
+++ b/src/common/thread_queue_list.h
@@ -40,6 +40,18 @@ struct ThreadQueueList {
         return -1;
     }
 
+    T get_first() {
+        Queue *cur = first;
+        while (cur != nullptr) {
+            if (!cur->data.empty()) {
+                return cur->data.front();
+            }
+            cur = cur->next_nonempty;
+        }
+
+        return T();
+    }
+
     T pop_first() {
         Queue *cur = first;
         while (cur != nullptr) {
@@ -79,6 +91,12 @@ struct ThreadQueueList {
         cur->data.push_back(thread_id);
     }
 
+    void move(const T& thread_id, Priority old_priority, Priority new_priority) {
+        remove(old_priority, thread_id);
+        prepare(new_priority);
+        push_back(new_priority, thread_id);
+    }
+
     void remove(Priority priority, const T& thread_id) {
         Queue *cur = &queues[priority];
         boost::remove_erase(cur->data, thread_id);
diff --git a/src/core/hle/function_wrappers.h b/src/core/hle/function_wrappers.h
index 0b6b6f5180..be2626eef6 100644
--- a/src/core/hle/function_wrappers.h
+++ b/src/core/hle/function_wrappers.h
@@ -46,6 +46,13 @@ template<ResultCode func(u32*, u32, u32, u32, u32, u32)> void Wrap(){
     FuncReturn(retval);
 }
 
+template<ResultCode func(u32*, s32, u32, u32, u32, s32)> void Wrap() {
+    u32 param_1 = 0;
+    u32 retval = func(&param_1, PARAM(0), PARAM(1), PARAM(2), PARAM(3), PARAM(4)).raw;
+    Core::g_app_core->SetReg(1, param_1);
+    FuncReturn(retval);
+}
+
 template<ResultCode func(s32*, u32*, s32, bool, s64)> void Wrap() {
     s32 param_1 = 0;
     s32 retval = func(&param_1, (Handle*)Memory::GetPointer(PARAM(1)), (s32)PARAM(2),
diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp
index be1aed615a..3a1e15ac67 100644
--- a/src/core/hle/kernel/thread.cpp
+++ b/src/core/hle/kernel/thread.cpp
@@ -140,6 +140,29 @@ void ArbitrateAllThreads(u32 address) {
     }
 }
 
+/// Boost low priority threads (temporarily) that have been starved
+static void PriorityBoostStarvedThreads() {
+    u64 current_ticks = CoreTiming::GetTicks();
+
+    for (auto& thread : thread_list) {
+        // TODO(bunnei): Threads that have been waiting to be scheduled for `boost_ticks` (or
+        // longer) will have their priority temporarily adjusted to 1 higher than the highest
+        // priority thread to prevent thread starvation. This general behavior has been verified
+        // on hardware. However, this is almost certainly not perfect, and the real CTR OS scheduler
+        // should probably be reversed to verify this.
+
+        const u64 boost_timeout = 2000000;  // Boost threads that have been ready for > this long
+
+        u64 delta = current_ticks - thread->last_running_ticks;
+
+        if (thread->status == THREADSTATUS_READY && delta > boost_timeout && !thread->idle) {
+            const s32 boost_priority = std::max(ready_queue.get_first()->current_priority - 1, 0);
+            ready_queue.move(thread, thread->current_priority, boost_priority);
+            thread->current_priority = boost_priority;
+        }
+    }
+}
+
 /** 
  * Switches the CPU's active thread context to that of the specified thread
  * @param new_thread The thread to switch to
@@ -151,6 +174,7 @@ static void SwitchContext(Thread* new_thread) {
 
     // Save context for previous thread
     if (previous_thread) {
+        previous_thread->last_running_ticks = CoreTiming::GetTicks();
         Core::g_app_core->SaveContext(previous_thread->context);
 
         if (previous_thread->status == THREADSTATUS_RUNNING) {
@@ -168,6 +192,9 @@ static void SwitchContext(Thread* new_thread) {
         ready_queue.remove(new_thread->current_priority, new_thread);
         new_thread->status = THREADSTATUS_RUNNING;
 
+        // Restores thread to its nominal priority if it has been temporarily changed
+        new_thread->current_priority = new_thread->nominal_priority;
+
         Core::g_app_core->LoadContext(new_thread->context);
     } else {
         current_thread = nullptr;
@@ -364,7 +391,8 @@ ResultVal<SharedPtr<Thread>> Thread::Create(std::string name, VAddr entry_point,
     thread->status = THREADSTATUS_DORMANT;
     thread->entry_point = entry_point;
     thread->stack_top = stack_top;
-    thread->initial_priority = thread->current_priority = priority;
+    thread->nominal_priority = thread->current_priority = priority;
+    thread->last_running_ticks = CoreTiming::GetTicks();
     thread->processor_id = processor_id;
     thread->wait_set_output = false;
     thread->wait_all = false;
@@ -400,18 +428,11 @@ static void ClampPriority(const Thread* thread, s32* priority) {
 void Thread::SetPriority(s32 priority) {
     ClampPriority(this, &priority);
 
-    if (current_priority == priority) {
-        return;
-    }
+    // If thread was ready, adjust queues
+    if (status == THREADSTATUS_READY)
+        ready_queue.move(this, current_priority, priority);
 
-    if (status == THREADSTATUS_READY) {
-        // If thread was ready, adjust queues
-        ready_queue.remove(current_priority, this);
-        ready_queue.prepare(priority);
-        ready_queue.push_back(priority, this);
-    }
-    
-    current_priority = priority;
+    nominal_priority = current_priority = priority;
 }
 
 SharedPtr<Thread> SetupIdleThread() {
@@ -440,6 +461,9 @@ SharedPtr<Thread> SetupMainThread(u32 stack_size, u32 entry_point, s32 priority)
 
 void Reschedule() {
     Thread* prev = GetCurrentThread();
+
+    PriorityBoostStarvedThreads();
+
     Thread* next = PopNextReadyThread();
     HLE::g_reschedule = false;
 
diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h
index bde4eecf2c..92f2c423b0 100644
--- a/src/core/hle/kernel/thread.h
+++ b/src/core/hle/kernel/thread.h
@@ -17,16 +17,19 @@
 #include "core/hle/kernel/kernel.h"
 #include "core/hle/result.h"
 
-enum ThreadPriority {
-    THREADPRIO_HIGHEST      = 0x0,  ///< Highest thread priority
-    THREADPRIO_DEFAULT      = 0x30, ///< Default thread priority for userland apps
-    THREADPRIO_LOWEST       = 0x3F, ///< Lowest thread priority
+enum ThreadPriority : s32{
+    THREADPRIO_HIGHEST          = 0,  ///< Highest thread priority
+    THREADPRIO_USERLAND_MAX     = 24, ///< Highest thread priority for userland apps
+    THREADPRIO_DEFAULT          = 48, ///< Default thread priority for userland apps
+    THREADPRIO_LOWEST           = 63, ///< Lowest thread priority
 };
 
-enum ThreadProcessorId {
-    THREADPROCESSORID_0     = 0xFFFFFFFE,   ///< Enables core appcode
-    THREADPROCESSORID_1     = 0xFFFFFFFD,   ///< Enables core syscore
-    THREADPROCESSORID_ALL   = 0xFFFFFFFC,   ///< Enables both cores
+enum ThreadProcessorId : s32 {
+    THREADPROCESSORID_DEFAULT   = -2, ///< Run thread on default core specified by exheader
+    THREADPROCESSORID_ALL       = -1, ///< Run thread on either core
+    THREADPROCESSORID_0         =  0, ///< Run thread on core 0 (AppCore)
+    THREADPROCESSORID_1         =  1, ///< Run thread on core 1 (SysCore)
+    THREADPROCESSORID_MAX       =  2, ///< Processor ID must be less than this
 };
 
 enum ThreadStatus {
@@ -134,8 +137,10 @@ public:
     u32 entry_point;
     u32 stack_top;
 
-    s32 initial_priority;
-    s32 current_priority;
+    s32 nominal_priority;   ///< Nominal thread priority, as set by the emulated application
+    s32 current_priority;   ///< Current thread priority, can be temporarily changed
+
+    u64 last_running_ticks; ///< CPU tick when thread was last running
 
     s32 processor_id;
 
diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp
index e89e972387..82e187466b 100644
--- a/src/core/hle/svc.cpp
+++ b/src/core/hle/svc.cpp
@@ -312,7 +312,7 @@ static ResultCode GetResourceLimitCurrentValues(s64* values, Handle resource_lim
 }
 
 /// Creates a new thread
-static ResultCode CreateThread(u32* out_handle, u32 priority, u32 entry_point, u32 arg, u32 stack_top, u32 processor_id) {
+static ResultCode CreateThread(Handle* out_handle, s32 priority, u32 entry_point, u32 arg, u32 stack_top, s32 processor_id) {
     using Kernel::Thread;
 
     std::string name;
@@ -323,6 +323,21 @@ static ResultCode CreateThread(u32* out_handle, u32 priority, u32 entry_point, u
         name = Common::StringFromFormat("unknown-%08x", entry_point);
     }
 
+    // TODO(bunnei): Implement resource limits to return an error code instead of the below assert.
+    // The error code should be: Description::NotAuthorized, Module::OS, Summary::WrongArgument,
+    // Level::Permanent
+    ASSERT_MSG(priority >= THREADPRIO_USERLAND_MAX, "Unexpected thread priority!");
+
+    if (priority > THREADPRIO_LOWEST) {
+        return ResultCode(ErrorDescription::OutOfRange, ErrorModule::OS,
+                          ErrorSummary::InvalidArgument, ErrorLevel::Usage);
+    }
+
+    if (processor_id > THREADPROCESSORID_MAX) {
+        return ResultCode(ErrorDescription::OutOfRange, ErrorModule::Kernel,
+                          ErrorSummary::InvalidArgument, ErrorLevel::Permanent);
+    }
+
     CASCADE_RESULT(SharedPtr<Thread> thread, Kernel::Thread::Create(
             name, entry_point, priority, arg, processor_id, stack_top));
     CASCADE_RESULT(*out_handle, Kernel::g_handle_table.Create(std::move(thread)));
@@ -331,11 +346,6 @@ static ResultCode CreateThread(u32* out_handle, u32 priority, u32 entry_point, u
         "threadpriority=0x%08X, processorid=0x%08X : created handle=0x%08X", entry_point,
         name.c_str(), arg, stack_top, priority, processor_id, *out_handle);
 
-    if (THREADPROCESSORID_1 == processor_id) {
-        LOG_WARNING(Kernel_SVC,
-            "thread designated for system CPU core (UNIMPLEMENTED) will be run with app core scheduling");
-    }
-
     HLE::Reschedule(__func__);
 
     return RESULT_SUCCESS;
-- 
GitLab