From 9f9b64cda2079a1aebf2c4a12fc20c24891c23c9 Mon Sep 17 00:00:00 2001 From: Liam Date: Wed, 22 Feb 2023 21:40:53 -0500 Subject: kernel: document previous location of interrupt disables in arbiter/condvar --- src/core/hle/kernel/k_condition_variable.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/core/hle/kernel/k_condition_variable.cpp') diff --git a/src/core/hle/kernel/k_condition_variable.cpp b/src/core/hle/kernel/k_condition_variable.cpp index 3f0be1c3f..50a805296 100644 --- a/src/core/hle/kernel/k_condition_variable.cpp +++ b/src/core/hle/kernel/k_condition_variable.cpp @@ -198,7 +198,9 @@ void KConditionVariable::SignalImpl(KThread* thread) { u32 prev_tag{}; bool can_access{}; { - // TODO(bunnei): We should disable interrupts here via KScopedInterruptDisable. + // NOTE: If scheduler lock is not held here, interrupt disable is required. + // KScopedInterruptDisable di; + // TODO(bunnei): We should call CanAccessAtomic(..) here. can_access = true; if (can_access) [[likely]] { -- cgit v1.2.3 From 367e89f984e635ae6680e6c640fe3d1259fb692e Mon Sep 17 00:00:00 2001 From: Liam Date: Wed, 22 Feb 2023 21:46:06 -0500 Subject: kernel: barrier memory before condition variable write --- src/core/hle/kernel/k_condition_variable.cpp | 30 ++++++++++++++-------------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'src/core/hle/kernel/k_condition_variable.cpp') diff --git a/src/core/hle/kernel/k_condition_variable.cpp b/src/core/hle/kernel/k_condition_variable.cpp index 50a805296..c6a088942 100644 --- a/src/core/hle/kernel/k_condition_variable.cpp +++ b/src/core/hle/kernel/k_condition_variable.cpp @@ -112,7 +112,7 @@ Result KConditionVariable::SignalToAddress(VAddr addr) { // Remove waiter thread. s32 num_waiters{}; - KThread* next_owner_thread = + KThread* const next_owner_thread = owner_thread->RemoveWaiterByKey(std::addressof(num_waiters), addr); // Determine the next tag. @@ -122,25 +122,25 @@ Result KConditionVariable::SignalToAddress(VAddr addr) { if (num_waiters > 1) { next_value |= Svc::HandleWaitMask; } + } - // Write the value to userspace. - Result result{ResultSuccess}; - if (WriteToUser(system, addr, std::addressof(next_value))) [[likely]] { - result = ResultSuccess; - } else { - result = ResultInvalidCurrentMemory; - } + // Synchronize memory before proceeding. + std::atomic_thread_fence(std::memory_order_seq_cst); - // Signal the next owner thread. - next_owner_thread->EndWait(result); - return result; + // Write the value to userspace. + Result result{ResultSuccess}; + if (WriteToUser(system, addr, std::addressof(next_value))) [[likely]] { + result = ResultSuccess; } else { - // Just write the value to userspace. - R_UNLESS(WriteToUser(system, addr, std::addressof(next_value)), - ResultInvalidCurrentMemory); + result = ResultInvalidCurrentMemory; + } - return ResultSuccess; + // If necessary, signal the next owner thread. + if (next_owner_thread != nullptr) { + next_owner_thread->EndWait(result); } + + R_RETURN(result); } } -- cgit v1.2.3 From c4ba088a5df13ff4b4d8853216231d690f2c79c0 Mon Sep 17 00:00:00 2001 From: Liam Date: Thu, 23 Feb 2023 15:49:42 -0500 Subject: kernel: refactor priority inheritance to represent locks as C++ objects --- src/core/hle/kernel/k_condition_variable.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'src/core/hle/kernel/k_condition_variable.cpp') diff --git a/src/core/hle/kernel/k_condition_variable.cpp b/src/core/hle/kernel/k_condition_variable.cpp index c6a088942..8dae78397 100644 --- a/src/core/hle/kernel/k_condition_variable.cpp +++ b/src/core/hle/kernel/k_condition_variable.cpp @@ -111,15 +111,15 @@ Result KConditionVariable::SignalToAddress(VAddr addr) { KScopedSchedulerLock sl(kernel); // Remove waiter thread. - s32 num_waiters{}; + bool has_waiters{}; KThread* const next_owner_thread = - owner_thread->RemoveWaiterByKey(std::addressof(num_waiters), addr); + owner_thread->RemoveWaiterByKey(std::addressof(has_waiters), addr); // Determine the next tag. u32 next_value{}; if (next_owner_thread != nullptr) { next_value = next_owner_thread->GetAddressKeyValue(); - if (num_waiters > 1) { + if (has_waiters) { next_value |= Svc::HandleWaitMask; } } @@ -247,9 +247,11 @@ void KConditionVariable::Signal(u64 cv_key, s32 count) { (it->GetConditionVariableKey() == cv_key)) { KThread* target_thread = std::addressof(*it); - this->SignalImpl(target_thread); it = thread_tree.erase(it); target_thread->ClearConditionVariable(); + + this->SignalImpl(target_thread); + ++num_waiters; } @@ -279,16 +281,16 @@ Result KConditionVariable::Wait(VAddr addr, u64 key, u32 value, s64 timeout) { // Update the value and process for the next owner. { // Remove waiter thread. - s32 num_waiters{}; + bool has_waiters{}; KThread* next_owner_thread = - cur_thread->RemoveWaiterByKey(std::addressof(num_waiters), addr); + cur_thread->RemoveWaiterByKey(std::addressof(has_waiters), addr); // Update for the next owner thread. u32 next_value{}; if (next_owner_thread != nullptr) { // Get the next tag value. next_value = next_owner_thread->GetAddressKeyValue(); - if (num_waiters > 1) { + if (has_waiters) { next_value |= Svc::HandleWaitMask; } -- cgit v1.2.3 From 97f7f7bad59cdd42bf5f504089e5cecd441da3ce Mon Sep 17 00:00:00 2001 From: Liam Date: Thu, 23 Feb 2023 20:32:03 -0500 Subject: kernel: be more careful about kernel address keys --- src/core/hle/kernel/k_condition_variable.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/core/hle/kernel/k_condition_variable.cpp') diff --git a/src/core/hle/kernel/k_condition_variable.cpp b/src/core/hle/kernel/k_condition_variable.cpp index 8dae78397..f40cf92b1 100644 --- a/src/core/hle/kernel/k_condition_variable.cpp +++ b/src/core/hle/kernel/k_condition_variable.cpp @@ -113,7 +113,7 @@ Result KConditionVariable::SignalToAddress(VAddr addr) { // Remove waiter thread. bool has_waiters{}; KThread* const next_owner_thread = - owner_thread->RemoveWaiterByKey(std::addressof(has_waiters), addr); + owner_thread->RemoveUserWaiterByKey(std::addressof(has_waiters), addr); // Determine the next tag. u32 next_value{}; @@ -283,7 +283,7 @@ Result KConditionVariable::Wait(VAddr addr, u64 key, u32 value, s64 timeout) { // Remove waiter thread. bool has_waiters{}; KThread* next_owner_thread = - cur_thread->RemoveWaiterByKey(std::addressof(has_waiters), addr); + cur_thread->RemoveUserWaiterByKey(std::addressof(has_waiters), addr); // Update for the next owner thread. u32 next_value{}; -- cgit v1.2.3