From d219a96cc828d17932beebead209ba696b92a911 Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Sat, 22 Feb 2020 10:27:40 -0400 Subject: [PATCH] Kernel: Address Feedback. --- src/core/hle/kernel/kernel.cpp | 27 ++++++++++++++++----------- src/core/hle/kernel/kernel.h | 6 +++--- src/core/hle/kernel/scheduler.cpp | 21 ++++++++++++--------- src/core/hle/kernel/scheduler.h | 8 ++++---- src/core/hle/kernel/time_manager.cpp | 6 ++++-- src/core/hle/kernel/time_manager.h | 9 ++++++++- 6 files changed, 47 insertions(+), 30 deletions(-) diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index de14e1936..9232f4d7e 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -3,6 +3,7 @@ // Refer to the license.txt file included. #include +#include #include #include #include @@ -17,6 +18,7 @@ #include "core/core.h" #include "core/core_timing.h" #include "core/core_timing_util.h" +#include "core/hardware_properties.h" #include "core/hle/kernel/client_port.h" #include "core/hle/kernel/errors.h" #include "core/hle/kernel/handle_table.h" @@ -188,6 +190,7 @@ struct KernelCore::Impl { } void RegisterCoreThread(std::size_t core_id) { + std::unique_lock lock{register_thread_mutex}; const std::thread::id this_id = std::this_thread::get_id(); const auto it = host_thread_ids.find(this_id); ASSERT(core_id < Core::Hardware::NUM_CPU_CORES); @@ -198,13 +201,14 @@ struct KernelCore::Impl { } void RegisterHostThread() { + std::unique_lock lock{register_thread_mutex}; const std::thread::id this_id = std::this_thread::get_id(); const auto it = host_thread_ids.find(this_id); ASSERT(it == host_thread_ids.end()); host_thread_ids[this_id] = registered_thread_ids++; } - u32 GetCurrentHostThreadId() const { + u32 GetCurrentHostThreadID() const { const std::thread::id this_id = std::this_thread::get_id(); const auto it = host_thread_ids.find(this_id); if (it == host_thread_ids.end()) { @@ -213,9 +217,9 @@ struct KernelCore::Impl { return it->second; } - Core::EmuThreadHandle GetCurrentEmuThreadId() const { + Core::EmuThreadHandle GetCurrentEmuThreadID() const { Core::EmuThreadHandle result = Core::EmuThreadHandle::InvalidHandle(); - result.host_handle = GetCurrentHostThreadId(); + result.host_handle = GetCurrentHostThreadID(); if (result.host_handle >= Core::Hardware::NUM_CPU_CORES) { return result; } @@ -246,8 +250,8 @@ struct KernelCore::Impl { std::shared_ptr thread_wakeup_event_type; std::shared_ptr preemption_event; - // TODO(yuriks): This can be removed if Thread objects are explicitly pooled in the future, - // allowing us to simply use a pool index or similar. + // This is the kernel's handle table or supervisor handle table which + // stores all the objects in place. Kernel::HandleTable global_handle_table; /// Map of named ports managed by the kernel, which can be retrieved using @@ -257,10 +261,11 @@ struct KernelCore::Impl { std::unique_ptr exclusive_monitor; std::vector cores; - // 0-3 Ids represent core threads, >3 represent others + // 0-3 IDs represent core threads, >3 represent others std::unordered_map host_thread_ids; u32 registered_thread_ids{Core::Hardware::NUM_CPU_CORES}; - std::bitset registered_core_threads{}; + std::bitset registered_core_threads; + std::mutex register_thread_mutex; // System context Core::System& system; @@ -420,12 +425,12 @@ void KernelCore::RegisterHostThread() { impl->RegisterHostThread(); } -u32 KernelCore::GetCurrentHostThreadId() const { - return impl->GetCurrentHostThreadId(); +u32 KernelCore::GetCurrentHostThreadID() const { + return impl->GetCurrentHostThreadID(); } -Core::EmuThreadHandle KernelCore::GetCurrentEmuThreadId() const { - return impl->GetCurrentEmuThreadId(); +Core::EmuThreadHandle KernelCore::GetCurrentEmuThreadID() const { + return impl->GetCurrentEmuThreadID(); } } // namespace Kernel diff --git a/src/core/hle/kernel/kernel.h b/src/core/hle/kernel/kernel.h index 76fd12ace..c4f78ab71 100644 --- a/src/core/hle/kernel/kernel.h +++ b/src/core/hle/kernel/kernel.h @@ -8,10 +8,10 @@ #include #include #include -#include "core/hardware_properties.h" #include "core/hle/kernel/object.h" namespace Core { +struct EmuThreadHandle; class ExclusiveMonitor; class System; } // namespace Core @@ -136,10 +136,10 @@ public: bool IsValidNamedPort(NamedPortTable::const_iterator port) const; /// Gets the current host_thread/guest_thread handle. - Core::EmuThreadHandle GetCurrentEmuThreadId() const; + Core::EmuThreadHandle GetCurrentEmuThreadID() const; /// Gets the current host_thread handle. - u32 GetCurrentHostThreadId() const; + u32 GetCurrentHostThreadID() const; /// Register the current thread as a CPU Core Thread. void RegisterCoreThread(std::size_t core_id); diff --git a/src/core/hle/kernel/scheduler.cpp b/src/core/hle/kernel/scheduler.cpp index 9556df951..e5892727e 100644 --- a/src/core/hle/kernel/scheduler.cpp +++ b/src/core/hle/kernel/scheduler.cpp @@ -358,26 +358,29 @@ void GlobalScheduler::Shutdown() { } void GlobalScheduler::Lock() { - Core::EmuThreadHandle current_thread = kernel.GetCurrentEmuThreadId(); + Core::EmuThreadHandle current_thread = kernel.GetCurrentEmuThreadID(); if (current_thread == current_owner) { ++scope_lock; } else { inner_lock.lock(); current_owner = current_thread; + ASSERT(current_owner != Core::EmuThreadHandle::InvalidHandle()); scope_lock = 1; } } void GlobalScheduler::Unlock() { - if (--scope_lock == 0) { - for (std::size_t i = 0; i < Core::Hardware::NUM_CPU_CORES; i++) { - SelectThread(i); - } - current_owner = Core::EmuThreadHandle::InvalidHandle(); - scope_lock = 1; - inner_lock.unlock(); - // TODO(Blinkhawk): Setup the interrupts and change context on current core. + if (--scope_lock != 0) { + ASSERT(scope_lock > 0); + return; } + for (std::size_t i = 0; i < Core::Hardware::NUM_CPU_CORES; i++) { + SelectThread(i); + } + current_owner = Core::EmuThreadHandle::InvalidHandle(); + scope_lock = 1; + inner_lock.unlock(); + // TODO(Blinkhawk): Setup the interrupts and change context on current core. } Scheduler::Scheduler(Core::System& system, Core::ARM_Interface& cpu_core, std::size_t core_id) diff --git a/src/core/hle/kernel/scheduler.h b/src/core/hle/kernel/scheduler.h index a779bb70f..1c93a838c 100644 --- a/src/core/hle/kernel/scheduler.h +++ b/src/core/hle/kernel/scheduler.h @@ -171,7 +171,7 @@ private: /// Scheduler lock mechanisms. std::mutex inner_lock{}; // TODO(Blinkhawk): Replace for a SpinLock - std::atomic scope_lock{}; + std::atomic scope_lock{}; Core::EmuThreadHandle current_owner{Core::EmuThreadHandle::InvalidHandle()}; /// Lists all thread ids that aren't deleted/etc. @@ -245,7 +245,7 @@ private: class SchedulerLock { public: - SchedulerLock(KernelCore& kernel); + explicit SchedulerLock(KernelCore& kernel); ~SchedulerLock(); protected: @@ -254,8 +254,8 @@ protected: class SchedulerLockAndSleep : public SchedulerLock { public: - SchedulerLockAndSleep(KernelCore& kernel, Handle& event_handle, Thread* time_task, - s64 nanoseconds); + explicit SchedulerLockAndSleep(KernelCore& kernel, Handle& event_handle, Thread* time_task, + s64 nanoseconds); ~SchedulerLockAndSleep(); void CancelSleep() { diff --git a/src/core/hle/kernel/time_manager.cpp b/src/core/hle/kernel/time_manager.cpp index 0b3e464d0..21b290468 100644 --- a/src/core/hle/kernel/time_manager.cpp +++ b/src/core/hle/kernel/time_manager.cpp @@ -2,6 +2,7 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include "common/assert.h" #include "core/core.h" #include "core/core_timing.h" #include "core/core_timing_util.h" @@ -34,9 +35,10 @@ void TimeManager::ScheduleTimeEvent(Handle& event_handle, Thread* timetask, s64 } void TimeManager::UnscheduleTimeEvent(Handle event_handle) { - if (event_handle != InvalidHandle) { - system.CoreTiming().UnscheduleEvent(time_manager_event_type, event_handle); + if (event_handle == InvalidHandle) { + return; } + system.CoreTiming().UnscheduleEvent(time_manager_event_type, event_handle); } } // namespace Kernel diff --git a/src/core/hle/kernel/time_manager.h b/src/core/hle/kernel/time_manager.h index b760311f1..eaec486d1 100644 --- a/src/core/hle/kernel/time_manager.h +++ b/src/core/hle/kernel/time_manager.h @@ -20,12 +20,19 @@ namespace Kernel { class Thread; +/** + * The `TimeManager` takes care of scheduling time events on threads and executes their TimeUp + * method when the event is triggered. + */ class TimeManager { public: - TimeManager(Core::System& system); + explicit TimeManager(Core::System& system); + /// Schedule a time event on `timetask` thread that will expire in 'nanoseconds' + /// returns a non-invalid handle in `event_handle` if correctly scheduled void ScheduleTimeEvent(Handle& event_handle, Thread* timetask, s64 nanoseconds); + /// Unschedule an existing time event void UnscheduleTimeEvent(Handle event_handle); private: