From 87c0cd7dad1d038270bb1731d3b2647d3775442a Mon Sep 17 00:00:00 2001 From: DH Date: Wed, 20 Nov 2024 23:29:56 +0300 Subject: [PATCH] kernel: umtx: avoid notifying suspended processes stub sys_prepare_to_suspend_process and sys_suspend_process fix sys_osem_trywait on bad descriptor --- orbis-kernel/src/sys/sys_sce.cpp | 13 +++++-- orbis-kernel/src/umtx.cpp | 54 +++++++++++++++++++++-------- orbis-kernel/src/utils/SharedCV.cpp | 4 +-- 3 files changed, 50 insertions(+), 21 deletions(-) diff --git a/orbis-kernel/src/sys/sys_sce.cpp b/orbis-kernel/src/sys/sys_sce.cpp index 45dcace38..2d4339b52 100644 --- a/orbis-kernel/src/sys/sys_sce.cpp +++ b/orbis-kernel/src/sys/sys_sce.cpp @@ -605,7 +605,10 @@ orbis::SysResult orbis::sys_osem_wait(Thread *thread, sint id, sint need, orbis::SysResult orbis::sys_osem_trywait(Thread *thread, sint id, sint need) { ORBIS_LOG_TRACE(__FUNCTION__, thread, id, need); Ref sem = thread->tproc->semMap.get(id); - if (need < 1 || need > sem->maxValue) + if (sem == nullptr) { + return ErrorCode::BADF; + } + return ErrorCode::INVAL; std::lock_guard lock(sem->mtx); @@ -693,7 +696,9 @@ orbis::SysResult orbis::sys_debug_init(Thread *thread /* TODO */) { return ErrorCode::NOSYS; } orbis::SysResult orbis::sys_suspend_process(Thread *thread, pid_t pid) { - return ErrorCode::NOSYS; + ORBIS_LOG_FATAL(__FUNCTION__, pid); + // FIXME: implement + return {}; } orbis::SysResult orbis::sys_resume_process(Thread *thread, pid_t pid) { return ErrorCode::NOSYS; @@ -1158,7 +1163,9 @@ orbis::SysResult orbis::sys_get_resident_count(Thread *thread, pid_t pid) { } orbis::SysResult orbis::sys_prepare_to_suspend_process(Thread *thread, pid_t pid) { - return ErrorCode::NOSYS; + ORBIS_LOG_FATAL(__FUNCTION__, pid); + // FIXME:implement + return {}; } orbis::SysResult orbis::sys_get_resident_fmem_count(Thread *thread, pid_t pid) { return ErrorCode::NOSYS; diff --git a/orbis-kernel/src/umtx.cpp b/orbis-kernel/src/umtx.cpp index 308a7cca0..ffcb28dc8 100644 --- a/orbis-kernel/src/umtx.cpp +++ b/orbis-kernel/src/umtx.cpp @@ -41,6 +41,15 @@ uint UmtxChain::notify_n(const UmtxKey &key, sint count) { uint n = 0; while (count > 0) { + while (true) { + auto flags = it->second.thr->suspendFlags.load(); + if (~flags & kThreadSuspendFlag) { + break; + } + + orbis::scoped_unblock unblock; + it->second.thr->suspendFlags.wait(flags); + } it->second.thr = nullptr; it->second.cv.notify_all(mtx); it = erase(it); @@ -63,6 +72,10 @@ uint UmtxChain::notify_all(const UmtxKey &key) { } } // namespace orbis +static bool isSpuriousWakeup(orbis::ErrorCode errc) { + return errc == orbis::ErrorCode::AGAIN || errc == orbis::ErrorCode::INTR; +} + orbis::ErrorCode orbis::umtx_lock_umtx(Thread *thread, ptr umtx, ulong id, std::uint64_t ut) { ORBIS_LOG_TODO(__FUNCTION__, thread->tid, umtx, id, ut); @@ -93,7 +106,8 @@ orbis::ErrorCode orbis::umtx_wait(Thread *thread, ptr addr, ulong id, while (true) { orbis::scoped_unblock unblock; result = orbis::toErrorCode(node->second.cv.wait(chain.mtx)); - if (result != ErrorCode{} || node->second.thr != thread) + if ((result != ErrorCode{} && !isSpuriousWakeup(result)) || + node->second.thr != thread) break; } } else { @@ -112,7 +126,7 @@ orbis::ErrorCode orbis::umtx_wait(Thread *thread, ptr addr, ulong id, result = ErrorCode::TIMEDOUT; break; } - if (result != ErrorCode{}) { + if (result != ErrorCode{} && !isSpuriousWakeup(result)) { break; } } @@ -187,15 +201,17 @@ static ErrorCode do_lock_normal(Thread *thread, ptr m, uint flags, if (mode == umutex_lock_mode::try_) return ErrorCode::BUSY; - if (error != ErrorCode{}) + if (error != ErrorCode{} && !isSpuriousWakeup(error)) return error; auto [chain, key, lock] = g_context.getUmtxChain1(thread, flags, m); auto node = chain.enqueue(key, thread); if (m->owner.compare_exchange_strong(owner, owner | kUmutexContested)) { - orbis::scoped_unblock unblock; - error = orbis::toErrorCode(node->second.cv.wait(chain.mtx, ut)); - if (error == ErrorCode{} && node->second.thr == thread) { + { + orbis::scoped_unblock unblock; + error = orbis::toErrorCode(node->second.cv.wait(chain.mtx, ut)); + } + if (error == ErrorCode{} && !isSpuriousWakeup(error) && node->second.thr == thread && m->owner.load() != 0) { error = ErrorCode::TIMEDOUT; } } @@ -350,7 +366,8 @@ orbis::ErrorCode orbis::umtx_cv_wait(Thread *thread, ptr cv, while (true) { orbis::scoped_unblock unblock; result = orbis::toErrorCode(node->second.cv.wait(chain.mtx, ut)); - if (result != ErrorCode{} || node->second.thr != thread) { + if ((result != ErrorCode{} && !isSpuriousWakeup(result)) || + node->second.thr != thread) { break; } } @@ -372,7 +389,7 @@ orbis::ErrorCode orbis::umtx_cv_wait(Thread *thread, ptr cv, break; } - if (result != ErrorCode{}) { + if (result != ErrorCode{} && !isSpuriousWakeup(result)) { break; } } @@ -457,7 +474,8 @@ orbis::ErrorCode orbis::umtx_rw_rdlock(Thread *thread, ptr rwlock, while (true) { orbis::scoped_unblock unblock; result = orbis::toErrorCode(node->second.cv.wait(chain.mtx, ut)); - if (result != ErrorCode{} || node->second.thr != thread) { + if ((result != ErrorCode{} && !isSpuriousWakeup(result)) || + node->second.thr != thread) { break; } } @@ -478,7 +496,7 @@ orbis::ErrorCode orbis::umtx_rw_rdlock(Thread *thread, ptr rwlock, break; } - if (result != ErrorCode{}) { + if (result != ErrorCode{} && !isSpuriousWakeup(result)) { break; } } @@ -560,7 +578,8 @@ orbis::ErrorCode orbis::umtx_rw_wrlock(Thread *thread, ptr rwlock, while (true) { orbis::scoped_unblock unblock; error = orbis::toErrorCode(node->second.cv.wait(chain.mtx, ut)); - if (error != ErrorCode{} || node->second.thr != thread) { + if ((error != ErrorCode{} && !isSpuriousWakeup(error)) || + node->second.thr != thread) { break; } } @@ -580,7 +599,7 @@ orbis::ErrorCode orbis::umtx_rw_wrlock(Thread *thread, ptr rwlock, error = ErrorCode::TIMEDOUT; break; } - if (error != ErrorCode{}) { + if (error != ErrorCode{} && !isSpuriousWakeup(error)) { break; } } @@ -663,7 +682,11 @@ orbis::ErrorCode orbis::umtx_rw_unlock(Thread *thread, ptr rwlock) { } } - chain.notify_n(key, count); + if (flags & 1) { + chain.notify_all(key); + } else { + chain.notify_n(key, count); + } return {}; } @@ -735,7 +758,8 @@ orbis::ErrorCode orbis::umtx_sem_wait(Thread *thread, ptr sem, while (true) { orbis::scoped_unblock unblock; result = orbis::toErrorCode(node->second.cv.wait(chain.mtx, ut)); - if (result != ErrorCode{} || node->second.thr != thread) + if ((result != ErrorCode{} && !isSpuriousWakeup(result)) || + node->second.thr != thread) break; } } else { @@ -754,7 +778,7 @@ orbis::ErrorCode orbis::umtx_sem_wait(Thread *thread, ptr sem, result = ErrorCode::TIMEDOUT; break; } - if (result != ErrorCode{}) { + if (result != ErrorCode{} && !isSpuriousWakeup(result)) { break; } } diff --git a/orbis-kernel/src/utils/SharedCV.cpp b/orbis-kernel/src/utils/SharedCV.cpp index c3c53a6aa..044a19b7f 100644 --- a/orbis-kernel/src/utils/SharedCV.cpp +++ b/orbis-kernel/src/utils/SharedCV.cpp @@ -23,14 +23,12 @@ std::errc shared_cv::impl_wait(shared_mutex &mutex, unsigned _val, result = m_value.wait(_val, useTimeout ? std::chrono::microseconds(usec_timeout) : std::chrono::microseconds::max()); - bool spurious = result == std::errc::resource_unavailable_try_again || - result == std::errc::interrupted; + bool spurious = result == std::errc::resource_unavailable_try_again; // Cleanup const auto old = m_value.fetch_op([&](unsigned &value) { // Remove waiter if no signals if ((value & ~c_waiter_mask) == 0) { - if (!spurious) { value -= 1; }