From 12dc918937467b2579b9e2738820c542ae4fdd4f Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 12 Nov 2019 03:32:53 -0500 Subject: [PATCH] kernel: Resolve sign conversion warnings Uncovered a bug within Thread's SetCoreAndAffinityMask() where an unsigned variable (ideal_core) was being compared against "< 0", which would always be a false condition. We can also get rid of an unused function (GetNextProcessorId) which contained a sign mismatch warning. --- src/core/hle/kernel/scheduler.cpp | 46 ++++++++++++++-------------- src/core/hle/kernel/scheduler.h | 36 +++++++++++----------- src/core/hle/kernel/thread.cpp | 48 +++++++++++------------------- src/core/hle/kernel/vm_manager.cpp | 2 +- 4 files changed, 60 insertions(+), 72 deletions(-) diff --git a/src/core/hle/kernel/scheduler.cpp b/src/core/hle/kernel/scheduler.cpp index 0e2dbf13e1..16e95381b0 100644 --- a/src/core/hle/kernel/scheduler.cpp +++ b/src/core/hle/kernel/scheduler.cpp @@ -35,12 +35,12 @@ void GlobalScheduler::RemoveThread(const Thread* thread) { thread_list.end()); } -void GlobalScheduler::UnloadThread(s32 core) { +void GlobalScheduler::UnloadThread(std::size_t core) { Scheduler& sched = system.Scheduler(core); sched.UnloadThread(); } -void GlobalScheduler::SelectThread(u32 core) { +void GlobalScheduler::SelectThread(std::size_t core) { const auto update_thread = [](Thread* thread, Scheduler& sched) { if (thread != sched.selected_thread) { if (thread == nullptr) { @@ -77,9 +77,9 @@ void GlobalScheduler::SelectThread(u32 core) { // if we got a suggested thread, select it, else do a second pass. if (winner && winner->GetPriority() > 2) { if (winner->IsRunning()) { - UnloadThread(winner->GetProcessorID()); + UnloadThread(static_cast(winner->GetProcessorID())); } - TransferToCore(winner->GetPriority(), core, winner); + TransferToCore(winner->GetPriority(), static_cast(core), winner); update_thread(winner, sched); return; } @@ -91,9 +91,9 @@ void GlobalScheduler::SelectThread(u32 core) { Thread* thread_on_core = scheduled_queue[src_core].front(); Thread* to_change = *it; if (thread_on_core->IsRunning() || to_change->IsRunning()) { - UnloadThread(src_core); + UnloadThread(static_cast(src_core)); } - TransferToCore(thread_on_core->GetPriority(), core, thread_on_core); + TransferToCore(thread_on_core->GetPriority(), static_cast(core), thread_on_core); current_thread = thread_on_core; break; } @@ -154,9 +154,9 @@ bool GlobalScheduler::YieldThreadAndBalanceLoad(Thread* yielding_thread) { if (winner != nullptr) { if (winner != yielding_thread) { if (winner->IsRunning()) { - UnloadThread(winner->GetProcessorID()); + UnloadThread(static_cast(winner->GetProcessorID())); } - TransferToCore(winner->GetPriority(), core_id, winner); + TransferToCore(winner->GetPriority(), s32(core_id), winner); } } else { winner = next_thread; @@ -196,9 +196,9 @@ bool GlobalScheduler::YieldThreadAndWaitForLoadBalancing(Thread* yielding_thread if (winner != nullptr) { if (winner != yielding_thread) { if (winner->IsRunning()) { - UnloadThread(winner->GetProcessorID()); + UnloadThread(static_cast(winner->GetProcessorID())); } - TransferToCore(winner->GetPriority(), core_id, winner); + TransferToCore(winner->GetPriority(), static_cast(core_id), winner); } } else { winner = yielding_thread; @@ -248,7 +248,7 @@ void GlobalScheduler::PreemptThreads() { if (winner != nullptr) { if (winner->IsRunning()) { - UnloadThread(winner->GetProcessorID()); + UnloadThread(static_cast(winner->GetProcessorID())); } TransferToCore(winner->GetPriority(), s32(core_id), winner); current_thread = @@ -281,7 +281,7 @@ void GlobalScheduler::PreemptThreads() { if (winner != nullptr) { if (winner->IsRunning()) { - UnloadThread(winner->GetProcessorID()); + UnloadThread(static_cast(winner->GetProcessorID())); } TransferToCore(winner->GetPriority(), s32(core_id), winner); current_thread = winner; @@ -292,30 +292,30 @@ void GlobalScheduler::PreemptThreads() { } } -void GlobalScheduler::Suggest(u32 priority, u32 core, Thread* thread) { +void GlobalScheduler::Suggest(u32 priority, std::size_t core, Thread* thread) { suggested_queue[core].add(thread, priority); } -void GlobalScheduler::Unsuggest(u32 priority, u32 core, Thread* thread) { +void GlobalScheduler::Unsuggest(u32 priority, std::size_t core, Thread* thread) { suggested_queue[core].remove(thread, priority); } -void GlobalScheduler::Schedule(u32 priority, u32 core, Thread* thread) { +void GlobalScheduler::Schedule(u32 priority, std::size_t core, Thread* thread) { ASSERT_MSG(thread->GetProcessorID() == s32(core), "Thread must be assigned to this core."); scheduled_queue[core].add(thread, priority); } -void GlobalScheduler::SchedulePrepend(u32 priority, u32 core, Thread* thread) { +void GlobalScheduler::SchedulePrepend(u32 priority, std::size_t core, Thread* thread) { ASSERT_MSG(thread->GetProcessorID() == s32(core), "Thread must be assigned to this core."); scheduled_queue[core].add(thread, priority, false); } -void GlobalScheduler::Reschedule(u32 priority, u32 core, Thread* thread) { +void GlobalScheduler::Reschedule(u32 priority, std::size_t core, Thread* thread) { scheduled_queue[core].remove(thread, priority); scheduled_queue[core].add(thread, priority); } -void GlobalScheduler::Unschedule(u32 priority, u32 core, Thread* thread) { +void GlobalScheduler::Unschedule(u32 priority, std::size_t core, Thread* thread) { scheduled_queue[core].remove(thread, priority); } @@ -327,14 +327,14 @@ void GlobalScheduler::TransferToCore(u32 priority, s32 destination_core, Thread* } thread->SetProcessorID(destination_core); if (source_core >= 0) { - Unschedule(priority, source_core, thread); + Unschedule(priority, static_cast(source_core), thread); } if (destination_core >= 0) { - Unsuggest(priority, destination_core, thread); - Schedule(priority, destination_core, thread); + Unsuggest(priority, static_cast(destination_core), thread); + Schedule(priority, static_cast(destination_core), thread); } if (source_core >= 0) { - Suggest(priority, source_core, thread); + Suggest(priority, static_cast(source_core), thread); } } @@ -357,7 +357,7 @@ void GlobalScheduler::Shutdown() { thread_list.clear(); } -Scheduler::Scheduler(Core::System& system, Core::ARM_Interface& cpu_core, u32 core_id) +Scheduler::Scheduler(Core::System& system, Core::ARM_Interface& cpu_core, std::size_t core_id) : system(system), cpu_core(cpu_core), core_id(core_id) {} Scheduler::~Scheduler() = default; diff --git a/src/core/hle/kernel/scheduler.h b/src/core/hle/kernel/scheduler.h index f2d6311b81..311849dfb3 100644 --- a/src/core/hle/kernel/scheduler.h +++ b/src/core/hle/kernel/scheduler.h @@ -42,41 +42,34 @@ public: * Add a thread to the suggested queue of a cpu core. Suggested threads may be * picked if no thread is scheduled to run on the core. */ - void Suggest(u32 priority, u32 core, Thread* thread); + void Suggest(u32 priority, std::size_t core, Thread* thread); /** * Remove a thread to the suggested queue of a cpu core. Suggested threads may be * picked if no thread is scheduled to run on the core. */ - void Unsuggest(u32 priority, u32 core, Thread* thread); + void Unsuggest(u32 priority, std::size_t core, Thread* thread); /** * Add a thread to the scheduling queue of a cpu core. The thread is added at the * back the queue in its priority level. */ - void Schedule(u32 priority, u32 core, Thread* thread); + void Schedule(u32 priority, std::size_t core, Thread* thread); /** * Add a thread to the scheduling queue of a cpu core. The thread is added at the * front the queue in its priority level. */ - void SchedulePrepend(u32 priority, u32 core, Thread* thread); + void SchedulePrepend(u32 priority, std::size_t core, Thread* thread); /// Reschedule an already scheduled thread based on a new priority - void Reschedule(u32 priority, u32 core, Thread* thread); + void Reschedule(u32 priority, std::size_t core, Thread* thread); /// Unschedules a thread. - void Unschedule(u32 priority, u32 core, Thread* thread); - - /** - * Transfers a thread into an specific core. If the destination_core is -1 - * it will be unscheduled from its source code and added into its suggested - * queue. - */ - void TransferToCore(u32 priority, s32 destination_core, Thread* thread); + void Unschedule(u32 priority, std::size_t core, Thread* thread); /// Selects a core and forces it to unload its current thread's context - void UnloadThread(s32 core); + void UnloadThread(std::size_t core); /** * Takes care of selecting the new scheduled thread in three steps: @@ -90,9 +83,9 @@ public: * 3. Third is no suggested thread is found, we do a second pass and pick a running * thread in another core and swap it with its current thread. */ - void SelectThread(u32 core); + void SelectThread(std::size_t core); - bool HaveReadyThreads(u32 core_id) const { + bool HaveReadyThreads(std::size_t core_id) const { return !scheduled_queue[core_id].empty(); } @@ -145,6 +138,13 @@ public: void Shutdown(); private: + /** + * Transfers a thread into an specific core. If the destination_core is -1 + * it will be unscheduled from its source code and added into its suggested + * queue. + */ + void TransferToCore(u32 priority, s32 destination_core, Thread* thread); + bool AskForReselectionOrMarkRedundant(Thread* current_thread, const Thread* winner); static constexpr u32 min_regular_priority = 2; @@ -163,7 +163,7 @@ private: class Scheduler final { public: - explicit Scheduler(Core::System& system, Core::ARM_Interface& cpu_core, u32 core_id); + explicit Scheduler(Core::System& system, Core::ARM_Interface& cpu_core, std::size_t core_id); ~Scheduler(); /// Returns whether there are any threads that are ready to run. @@ -220,7 +220,7 @@ private: Core::ARM_Interface& cpu_core; u64 last_context_switch_time = 0; u64 idle_selection_count = 0; - const u32 core_id; + const std::size_t core_id; bool is_context_switch_pending = false; }; diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 962530d2d2..ee7531f2d5 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -77,18 +77,6 @@ void Thread::CancelWakeupTimer() { callback_handle); } -static std::optional GetNextProcessorId(u64 mask) { - for (s32 index = 0; index < Core::NUM_CPU_CORES; ++index) { - if (mask & (1ULL << index)) { - if (!Core::System::GetInstance().Scheduler(index).GetCurrentThread()) { - // Core is enabled and not running any threads, use this one - return index; - } - } - } - return {}; -} - void Thread::ResumeFromWait() { ASSERT_MSG(wait_objects.empty(), "Thread is waking up while waiting for objects"); @@ -173,7 +161,7 @@ ResultVal> Thread::Create(KernelCore& kernel, std::string name if (!Memory::IsValidVirtualAddress(owner_process, entry_point)) { LOG_ERROR(Kernel_SVC, "(name={}): invalid entry {:016X}", name, entry_point); // TODO (bunnei): Find the correct error code to use here - return ResultCode(-1); + return RESULT_UNKNOWN; } auto& system = Core::System::GetInstance(); @@ -401,7 +389,7 @@ void Thread::SetCurrentPriority(u32 new_priority) { ResultCode Thread::SetCoreAndAffinityMask(s32 new_core, u64 new_affinity_mask) { const auto HighestSetCore = [](u64 mask, u32 max_cores) { - for (s32 core = max_cores - 1; core >= 0; core--) { + for (s32 core = static_cast(max_cores - 1); core >= 0; core--) { if (((mask >> core) & 1) != 0) { return core; } @@ -425,7 +413,7 @@ ResultCode Thread::SetCoreAndAffinityMask(s32 new_core, u64 new_affinity_mask) { if (old_affinity_mask != new_affinity_mask) { const s32 old_core = processor_id; if (processor_id >= 0 && ((affinity_mask >> processor_id) & 1) == 0) { - if (ideal_core < 0) { + if (static_cast(ideal_core) < 0) { processor_id = HighestSetCore(affinity_mask, GlobalScheduler::NUM_CPU_CORES); } else { processor_id = ideal_core; @@ -447,23 +435,23 @@ void Thread::AdjustSchedulingOnStatus(u32 old_flags) { ThreadSchedStatus::Runnable) { // In this case the thread was running, now it's pausing/exitting if (processor_id >= 0) { - scheduler.Unschedule(current_priority, processor_id, this); + scheduler.Unschedule(current_priority, static_cast(processor_id), this); } - for (s32 core = 0; core < GlobalScheduler::NUM_CPU_CORES; core++) { - if (core != processor_id && ((affinity_mask >> core) & 1) != 0) { - scheduler.Unsuggest(current_priority, static_cast(core), this); + for (u32 core = 0; core < GlobalScheduler::NUM_CPU_CORES; core++) { + if (core != static_cast(processor_id) && ((affinity_mask >> core) & 1) != 0) { + scheduler.Unsuggest(current_priority, core, this); } } } else if (GetSchedulingStatus() == ThreadSchedStatus::Runnable) { // The thread is now set to running from being stopped if (processor_id >= 0) { - scheduler.Schedule(current_priority, processor_id, this); + scheduler.Schedule(current_priority, static_cast(processor_id), this); } - for (s32 core = 0; core < GlobalScheduler::NUM_CPU_CORES; core++) { - if (core != processor_id && ((affinity_mask >> core) & 1) != 0) { - scheduler.Suggest(current_priority, static_cast(core), this); + for (u32 core = 0; core < GlobalScheduler::NUM_CPU_CORES; core++) { + if (core != static_cast(processor_id) && ((affinity_mask >> core) & 1) != 0) { + scheduler.Suggest(current_priority, core, this); } } } @@ -477,11 +465,11 @@ void Thread::AdjustSchedulingOnPriority(u32 old_priority) { } auto& scheduler = Core::System::GetInstance().GlobalScheduler(); if (processor_id >= 0) { - scheduler.Unschedule(old_priority, processor_id, this); + scheduler.Unschedule(old_priority, static_cast(processor_id), this); } for (u32 core = 0; core < GlobalScheduler::NUM_CPU_CORES; core++) { - if (core != processor_id && ((affinity_mask >> core) & 1) != 0) { + if (core != static_cast(processor_id) && ((affinity_mask >> core) & 1) != 0) { scheduler.Unsuggest(old_priority, core, this); } } @@ -491,14 +479,14 @@ void Thread::AdjustSchedulingOnPriority(u32 old_priority) { if (processor_id >= 0) { if (current_thread == this) { - scheduler.SchedulePrepend(current_priority, processor_id, this); + scheduler.SchedulePrepend(current_priority, static_cast(processor_id), this); } else { - scheduler.Schedule(current_priority, processor_id, this); + scheduler.Schedule(current_priority, static_cast(processor_id), this); } } for (u32 core = 0; core < GlobalScheduler::NUM_CPU_CORES; core++) { - if (core != processor_id && ((affinity_mask >> core) & 1) != 0) { + if (core != static_cast(processor_id) && ((affinity_mask >> core) & 1) != 0) { scheduler.Suggest(current_priority, core, this); } } @@ -515,7 +503,7 @@ void Thread::AdjustSchedulingOnAffinity(u64 old_affinity_mask, s32 old_core) { for (u32 core = 0; core < GlobalScheduler::NUM_CPU_CORES; core++) { if (((old_affinity_mask >> core) & 1) != 0) { - if (core == old_core) { + if (core == static_cast(old_core)) { scheduler.Unschedule(current_priority, core, this); } else { scheduler.Unsuggest(current_priority, core, this); @@ -525,7 +513,7 @@ void Thread::AdjustSchedulingOnAffinity(u64 old_affinity_mask, s32 old_core) { for (u32 core = 0; core < GlobalScheduler::NUM_CPU_CORES; core++) { if (((affinity_mask >> core) & 1) != 0) { - if (core == processor_id) { + if (core == static_cast(processor_id)) { scheduler.Schedule(current_priority, core, this); } else { scheduler.Suggest(current_priority, core, this); diff --git a/src/core/hle/kernel/vm_manager.cpp b/src/core/hle/kernel/vm_manager.cpp index c7af870732..e6eee09d7a 100644 --- a/src/core/hle/kernel/vm_manager.cpp +++ b/src/core/hle/kernel/vm_manager.cpp @@ -167,7 +167,7 @@ ResultVal VMManager::FindFreeRegion(VAddr begin, VAddr end, u64 size) con if (vma_handle == vma_map.cend()) { // TODO(Subv): Find the correct error code here. - return ResultCode(-1); + return RESULT_UNKNOWN; } const VAddr target = std::max(begin, vma_handle->second.base);