From e81a2080ebf9712231dd29c081141780ffd46cfb Mon Sep 17 00:00:00 2001 From: Subv Date: Fri, 20 Apr 2018 12:01:14 -0500 Subject: Kernel: Corrected the implementation of svcArbitrateLock and svcArbitrateUnlock. Switch mutexes are no longer kernel objects, they are managed in userland and only use the kernel to handle the contention case. Mutex addresses store a special flag value (0x40000000) to notify the guest code that there are still some threads waiting for the mutex to be released. This flag is updated when a thread calls ArbitrateUnlock. TODO: * Fix svcWaitProcessWideKey * Fix svcSignalProcessWideKey * Remove the Mutex class. --- src/core/hle/kernel/thread.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'src/core/hle/kernel/thread.cpp') diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index f3a8aa4aa..0a0ad7cfb 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -126,6 +126,14 @@ static void ThreadWakeupCallback(u64 thread_handle, int cycles_late) { resume = thread->wakeup_callback(ThreadWakeupReason::Timeout, thread, nullptr, 0); } + if (thread->mutex_wait_address != 0 || thread->condvar_wait_address != 0 || + thread->wait_handle) { + ASSERT(thread->status == THREADSTATUS_WAIT_MUTEX); + thread->mutex_wait_address = 0; + thread->condvar_wait_address = 0; + thread->wait_handle = 0; + } + if (resume) thread->ResumeFromWait(); } @@ -151,6 +159,7 @@ void Thread::ResumeFromWait() { case THREADSTATUS_WAIT_HLE_EVENT: case THREADSTATUS_WAIT_SLEEP: case THREADSTATUS_WAIT_IPC: + case THREADSTATUS_WAIT_MUTEX: break; case THREADSTATUS_READY: @@ -256,7 +265,9 @@ ResultVal> Thread::Create(std::string name, VAddr entry_point, thread->last_running_ticks = CoreTiming::GetTicks(); thread->processor_id = processor_id; thread->wait_objects.clear(); - thread->wait_address = 0; + thread->mutex_wait_address = 0; + thread->condvar_wait_address = 0; + thread->wait_handle = 0; thread->name = std::move(name); thread->callback_handle = wakeup_callback_handle_table.Create(thread).Unwrap(); thread->owner_process = owner_process; -- cgit v1.2.3 From 5fdfbfe25adafd2734a19fe94cccc58993cb12e7 Mon Sep 17 00:00:00 2001 From: Subv Date: Fri, 20 Apr 2018 14:42:29 -0500 Subject: Kernel: Remove old and unused Mutex code. --- src/core/hle/kernel/thread.cpp | 3 --- 1 file changed, 3 deletions(-) (limited to 'src/core/hle/kernel/thread.cpp') diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 0a0ad7cfb..8093c4496 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -77,9 +77,6 @@ void Thread::Stop() { } wait_objects.clear(); - // Release all the mutexes that this thread holds - ReleaseThreadMutexes(this); - // Mark the TLS slot in the thread's page as free. u64 tls_page = (tls_address - Memory::TLS_AREA_VADDR) / Memory::PAGE_SIZE; u64 tls_slot = -- cgit v1.2.3 From be155f4d9d410886853c25ffa032ce41a7428337 Mon Sep 17 00:00:00 2001 From: Subv Date: Fri, 20 Apr 2018 14:45:52 -0500 Subject: Kernel: Remove unused ConditionVariable class. --- src/core/hle/kernel/thread.cpp | 9 --------- 1 file changed, 9 deletions(-) (limited to 'src/core/hle/kernel/thread.cpp') diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 8093c4496..16d9b9e36 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -329,15 +329,6 @@ void Thread::SetPriority(u32 priority) { nominal_priority = current_priority = priority; } -void Thread::UpdatePriority() { - u32 best_priority = nominal_priority; - for (auto& mutex : held_mutexes) { - if (mutex->priority < best_priority) - best_priority = mutex->priority; - } - BoostPriority(best_priority); -} - void Thread::BoostPriority(u32 priority) { Core::System::GetInstance().Scheduler().SetThreadPriority(this, priority); current_priority = priority; -- cgit v1.2.3 From 46572d027dc9620ed2b2a50277e6afd2a115ab81 Mon Sep 17 00:00:00 2001 From: Subv Date: Fri, 20 Apr 2018 20:15:16 -0500 Subject: Kernel: Implemented mutex priority inheritance. Verified with a hwtest and implemented based on reverse engineering. Thread A's priority will get bumped to the highest priority among all the threads that are waiting for a mutex that A holds. Once A releases the mutex and ownership is transferred to B, A's priority will return to normal and B's priority will be bumped. --- src/core/hle/kernel/thread.cpp | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) (limited to 'src/core/hle/kernel/thread.cpp') diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 16d9b9e36..36222d45f 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -129,6 +129,11 @@ static void ThreadWakeupCallback(u64 thread_handle, int cycles_late) { thread->mutex_wait_address = 0; thread->condvar_wait_address = 0; thread->wait_handle = 0; + + auto lock_owner = thread->lock_owner; + // Threads waking up by timeout from WaitProcessWideKey do not perform priority inheritance + // and don't have a lock owner. + ASSERT(lock_owner == nullptr); } if (resume) @@ -325,8 +330,8 @@ ResultVal> Thread::Create(std::string name, VAddr entry_point, void Thread::SetPriority(u32 priority) { ASSERT_MSG(priority <= THREADPRIO_LOWEST && priority >= THREADPRIO_HIGHEST, "Invalid priority value."); - Core::System::GetInstance().Scheduler().SetThreadPriority(this, priority); - nominal_priority = current_priority = priority; + nominal_priority = priority; + UpdatePriority(); } void Thread::BoostPriority(u32 priority) { @@ -376,6 +381,38 @@ VAddr Thread::GetCommandBufferAddress() const { return GetTLSAddress() + CommandHeaderOffset; } +void Thread::AddMutexWaiter(SharedPtr thread) { + thread->lock_owner = this; + wait_mutex_threads.emplace_back(std::move(thread)); + UpdatePriority(); +} + +void Thread::RemoveMutexWaiter(SharedPtr thread) { + boost::remove_erase(wait_mutex_threads, thread); + thread->lock_owner = nullptr; + UpdatePriority(); +} + +void Thread::UpdatePriority() { + // Find the highest priority among all the threads that are waiting for this thread's lock + u32 new_priority = nominal_priority; + for (const auto& thread : wait_mutex_threads) { + if (thread->nominal_priority < new_priority) + new_priority = thread->nominal_priority; + } + + if (new_priority == current_priority) + return; + + Core::System::GetInstance().Scheduler().SetThreadPriority(this, new_priority); + + current_priority = new_priority; + + // Recursively update the priority of the thread that depends on the priority of this one. + if (lock_owner) + lock_owner->UpdatePriority(); +} + //////////////////////////////////////////////////////////////////////////////////////////////////// /** -- cgit v1.2.3 From 40dee76c57e76438d06fc282d1f8a632933d5816 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 25 Apr 2018 19:11:22 -0400 Subject: kernel: Migrate logging macros to fmt-compatible ones --- src/core/hle/kernel/thread.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'src/core/hle/kernel/thread.cpp') diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 36222d45f..4cd57ab25 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -101,9 +101,10 @@ void ExitCurrentThread() { * @param cycles_late The number of CPU cycles that have passed since the desired wakeup time */ static void ThreadWakeupCallback(u64 thread_handle, int cycles_late) { - SharedPtr thread = wakeup_callback_handle_table.Get((Handle)thread_handle); + const auto proper_handle = static_cast(thread_handle); + SharedPtr thread = wakeup_callback_handle_table.Get(proper_handle); if (thread == nullptr) { - LOG_CRITICAL(Kernel, "Callback fired for invalid thread %08X", (Handle)thread_handle); + NGLOG_CRITICAL(Kernel, "Callback fired for invalid thread {:08X}", proper_handle); return; } @@ -238,19 +239,19 @@ ResultVal> Thread::Create(std::string name, VAddr entry_point, SharedPtr owner_process) { // Check if priority is in ranged. Lowest priority -> highest priority id. if (priority > THREADPRIO_LOWEST) { - LOG_ERROR(Kernel_SVC, "Invalid thread priority: %u", priority); + NGLOG_ERROR(Kernel_SVC, "Invalid thread priority: {}", priority); return ERR_OUT_OF_RANGE; } if (processor_id > THREADPROCESSORID_MAX) { - LOG_ERROR(Kernel_SVC, "Invalid processor id: %d", processor_id); + NGLOG_ERROR(Kernel_SVC, "Invalid processor id: {}", processor_id); return ERR_OUT_OF_RANGE_KERNEL; } // TODO(yuriks): Other checks, returning 0xD9001BEA if (!Memory::IsValidVirtualAddress(*owner_process, entry_point)) { - LOG_ERROR(Kernel_SVC, "(name=%s): invalid entry %016" PRIx64, name.c_str(), entry_point); + NGLOG_ERROR(Kernel_SVC, "(name={}): invalid entry {:016X}", name, entry_point); // TODO (bunnei): Find the correct error code to use here return ResultCode(-1); } @@ -289,8 +290,8 @@ ResultVal> Thread::Create(std::string name, VAddr entry_point, auto& linheap_memory = memory_region->linear_heap_memory; if (linheap_memory->size() + Memory::PAGE_SIZE > memory_region->size) { - LOG_ERROR(Kernel_SVC, - "Not enough space in region to allocate a new TLS page for thread"); + NGLOG_ERROR(Kernel_SVC, + "Not enough space in region to allocate a new TLS page for thread"); return ERR_OUT_OF_MEMORY; } -- cgit v1.2.3