From e87fb8c85985419d7bc751eadfe647bed7f6456b Mon Sep 17 00:00:00 2001 From: Pixeluted Date: Mon, 19 Oct 2026 04:30:27 +0200 Subject: [PATCH] kernel: hand guest mutex ownership directly to FIFO waiters --- src/kernel/pthread.cpp | 122 +++++++++++++++---------- tests/KernelFileSystemTests.cpp | 154 +++++++++++++++++++++++++++++++- 2 files changed, 227 insertions(+), 49 deletions(-) diff --git a/src/kernel/pthread.cpp b/src/kernel/pthread.cpp index 673a7dc72..bdb125ab7 100644 --- a/src/kernel/pthread.cpp +++ b/src/kernel/pthread.cpp @@ -343,14 +343,21 @@ static bool KernelClockGettimeSpecial(KernelClockid clock_id, KernelTimespec* tp #endif struct PthreadMutexPrivate { + struct Waiter { + Pthread thread = nullptr; + std::condition_variable cv; + Waiter* previous = nullptr; + Waiter* next = nullptr; + }; uint8_t reserved[256]; std::string name; std::mutex m; - std::condition_variable cv; Pthread owner = nullptr; uint32_t count = 0; int type = 1; int pprotocol = PTHREAD_PRIO_NONE; + Waiter* first_waiter = nullptr; + Waiter* last_waiter = nullptr; }; struct PthreadMutexattrPrivate { @@ -1242,6 +1249,20 @@ static int PthreadRwlockInitNamed(PthreadRwlock* rwlock, const PthreadRwlockattr const char* name); static int PthreadCondInitNamed(PthreadCond* cond, const PthreadCondattr* attr, const char* name); +static void NativeMutexRemoveWaiter(PthreadMutexPrivate* mutex, + PthreadMutexPrivate::Waiter* waiter) { + if (waiter->previous != nullptr) { + waiter->previous->next = waiter->next; + } else { + mutex->first_waiter = waiter->next; + } + if (waiter->next != nullptr) { + waiter->next->previous = waiter->previous; + } else { + mutex->last_waiter = waiter->previous; + } +} + static int NativeMutexLock(PthreadMutexPrivate* mutex, KernelUseconds* timeout_us) { EXIT_IF(mutex == nullptr); @@ -1258,54 +1279,53 @@ static int NativeMutexLock(PthreadMutexPrivate* mutex, KernelUseconds* timeout_u mutex->count++; return OK; } - if (timeout_us != nullptr) { - if (*timeout_us > 0) { - mutex->cv.wait_for(lock, std::chrono::microseconds(*timeout_us)); - } - return ETIMEDOUT; - } return EDEADLK; } - if (timeout_us == nullptr) { - while (mutex->owner != nullptr) { - mutex->cv.wait_for(lock, std::chrono::microseconds(SIGNAL_APC_POLL_MICROS)); - if (mutex->owner != nullptr) { - lock.unlock(); - KernelDispatchPendingSignalForCurrentThread(); - lock.lock(); - } - } - } else if (*timeout_us == 0) { - if (mutex->owner != nullptr) { - return ETIMEDOUT; - } - } else { - const auto deadline = - std::chrono::steady_clock::now() + std::chrono::microseconds(*timeout_us); - while (mutex->owner != nullptr) { - const auto now = std::chrono::steady_clock::now(); - if (now >= deadline) { - return ETIMEDOUT; - } - - const auto remaining = deadline - now; - const auto poll = (remaining < std::chrono::microseconds(SIGNAL_APC_POLL_MICROS) - ? remaining - : std::chrono::steady_clock::duration( - std::chrono::microseconds(SIGNAL_APC_POLL_MICROS))); - mutex->cv.wait_for(lock, poll); - if (mutex->owner != nullptr) { - lock.unlock(); - KernelDispatchPendingSignalForCurrentThread(); - lock.lock(); - } - } + if (mutex->owner == nullptr) { + mutex->owner = self; + mutex->count = 1; + return OK; + } + if (timeout_us != nullptr && *timeout_us == 0) { + return ETIMEDOUT; } - mutex->owner = self; - mutex->count = 1; - return OK; + // Guest mutex waits are not cancellation points, unlike the host condition wait. + int previous_cancel_state = 0; + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &previous_cancel_state); + PthreadMutexPrivate::Waiter waiter; + waiter.thread = self; + waiter.previous = mutex->last_waiter; + if (mutex->last_waiter != nullptr) { + mutex->last_waiter->next = &waiter; + } else { + mutex->first_waiter = &waiter; + } + mutex->last_waiter = &waiter; + const auto deadline = timeout_us != nullptr + ? std::chrono::steady_clock::now() + + std::chrono::microseconds(*timeout_us) + : std::chrono::steady_clock::time_point::max(); + int result = OK; + while (mutex->owner != self) { + const auto now = std::chrono::steady_clock::now(); + if (now >= deadline) { + NativeMutexRemoveWaiter(mutex, &waiter); + result = ETIMEDOUT; + break; + } + waiter.cv.wait_until(lock, std::min(deadline, now + + std::chrono::microseconds(SIGNAL_APC_POLL_MICROS))); + if (mutex->owner != self) { + lock.unlock(); + KernelDispatchPendingSignalForCurrentThread(); + lock.lock(); + } + } + lock.unlock(); + pthread_setcancelstate(previous_cancel_state, nullptr); + return result; } static int NativeMutexTrylock(PthreadMutexPrivate* mutex) { @@ -1358,10 +1378,16 @@ static int NativeMutexUnlock(PthreadMutexPrivate* mutex, uint32_t* recurse = nul return OK; } - mutex->owner = nullptr; - mutex->count = 0; - lock.unlock(); - mutex->cv.notify_one(); + if (auto* waiter = mutex->first_waiter) { + NativeMutexRemoveWaiter(mutex, waiter); + // Transfer ownership in FIFO order before waking the selected waiter. + mutex->owner = waiter->thread; + mutex->count = 1; + waiter->cv.notify_one(); + } else { + mutex->owner = nullptr; + mutex->count = 0; + } return OK; } diff --git a/tests/KernelFileSystemTests.cpp b/tests/KernelFileSystemTests.cpp index 035d297ca..d9b4e813c 100644 --- a/tests/KernelFileSystemTests.cpp +++ b/tests/KernelFileSystemTests.cpp @@ -6,15 +6,18 @@ #include "common/file.h" #include "ArchiveTestFixture.h" #include "common/logging/log.h" +#include "common/singleton.h" #include "common/subsystems.h" #include "common/threads.h" #include "common/stringUtils.h" #include "graphics/presentation/window/windowInternal.h" #include "kernel/eventQueue.h" #include "kernel/fileSystem.h" +#include "kernel/pthread.h" #include "libs/errno.h" #include "libs/network.h" #include "loader/additionalContent.h" +#include "loader/runtimeLinker.h" #include "loader/symbolDatabase.h" #include @@ -85,6 +88,148 @@ void Check(bool value, const char *text) { } } +template +void WaitForMutexTest(Predicate ready) { + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (!ready()) { + Check(std::chrono::steady_clock::now() < deadline, "mutex waiter made progress"); + std::this_thread::yield(); + } +} + +struct MutexWaiter { + Libs::LibKernel::PthreadMutex *mutex; + Libs::LibKernel::KernelUseconds timeout = 0; + bool cancel = false; + Libs::LibKernel::Pthread thread = nullptr; + std::vector stack = std::vector(1024 * 1024); + std::atomic parked{0}, resume{0}; + std::atomic acquired{false}, release{false}, finished{false}; + int result = 0; +}; + +thread_local MutexWaiter *current_mutex_waiter = nullptr; + +void KYTY_SYSV_ABI ParkMutexWaiter(int, void *) { + auto &waiter = *current_mutex_waiter; + const auto ticket = waiter.parked.fetch_add(1) + 1; + WaitForMutexTest([&] { return waiter.resume.load() >= ticket; }); +} + +void *KYTY_SYSV_ABI RunMutexWaiter(void *arg) { + using namespace Libs::LibKernel; + auto &waiter = *static_cast(arg); + current_mutex_waiter = &waiter; + waiter.result = waiter.timeout == 0 ? PthreadMutexLock(waiter.mutex) + : PthreadMutexTimedlock(waiter.mutex, waiter.timeout); + if (waiter.result == OK) { + waiter.acquired = true; + WaitForMutexTest([&] { return waiter.release.load(); }); + Check(PthreadMutexUnlock(waiter.mutex) == OK, "waiter unlocks owned mutex"); + } + waiter.finished = true; + if (waiter.cancel) { + PthreadTestcancel(); + Check(false, "deferred mutex waiter cancellation remains pending"); + } + return nullptr; +} + +void QueueMutexWaiter(MutexWaiter &waiter) { + using namespace Libs::LibKernel; + PthreadAttr attr = nullptr; + Check(PthreadAttrInit(&attr) == OK && + PthreadAttrSetstack(&attr, waiter.stack.data(), waiter.stack.size()) == OK && + PthreadCreate(&waiter.thread, &attr, RunMutexWaiter, &waiter, "MutexTest") == OK && + PthreadAttrDestroy(&attr) == OK, + "create guest mutex waiter"); + // Pending guest signals are dispatched inside the contended lock, after enqueueing. + // Holding that callback proves queue order without letting the waiter acquire. + PthreadQueuePendingSignal(waiter.thread, 30); + WaitForMutexTest([&] { return waiter.parked.load() == 1; }); +} + +void TestMutexHandoff() { + using namespace Libs::LibKernel; + Common::Singleton::Instance(); + Initialize(); + PthreadInitSelfForMainThread(); + Loader::SymbolDatabase symbols; + Libs::InitLibKernel_1(&symbols); + const auto *install_symbol = symbols.FindByNid("WkwEd3N7w0Y", Loader::SymbolType::Func); + const auto *remove_symbol = symbols.FindByNid("Qhv5ARAoOEc", Loader::SymbolType::Func); + Check(install_symbol != nullptr && remove_symbol != nullptr, "mutex signal exports resolve"); + const auto install = reinterpret_cast(install_symbol->vaddr); + const auto remove = reinterpret_cast(remove_symbol->vaddr); + Check(install(30, reinterpret_cast(ParkMutexWaiter)) == OK, "install mutex test signal"); + + PthreadMutexattr attr = nullptr; + Check(PthreadMutexattrInit(&attr) == OK && PthreadMutexattrSettype(&attr, 2) == OK, + "create recursive mutex attributes"); + for (unsigned timed = 0; timed < 3; ++timed) { + PthreadMutex mutex = nullptr; + Check(PthreadMutexInit(&mutex, &attr, "MutexHandoff") == OK && + PthreadMutexLock(&mutex) == OK && PthreadMutexLock(&mutex) == OK, + "lock recursive mutex twice"); + std::array waiters{{{&mutex}, {&mutex}, {&mutex}}}; + waiters[timed].timeout = 5000; + for (auto &waiter : waiters) { + QueueMutexWaiter(waiter); + } + waiters[timed].resume = 1; + Check(PthreadJoin(waiters[timed].thread, nullptr) == OK && + waiters[timed].result == KERNEL_ERROR_ETIMEDOUT, + "timed waiter unlinks from queue head, middle and tail"); + Check(PthreadMutexUnlock(&mutex) == OK && PthreadMutexTrylock(&mutex) == OK && + PthreadMutexUnlock(&mutex) == OK, + "partial recursive unlock retains ownership"); + Check(PthreadMutexUnlock(&mutex) == OK && + PthreadMutexTrylock(&mutex) == KERNEL_ERROR_EBUSY, + "unlock reserves ownership before queued waiter runs"); + for (auto &waiter : waiters) { + if (&waiter == &waiters[timed]) { + continue; + } + waiter.resume = 1; + WaitForMutexTest([&] { return waiter.acquired.load(); }); + Check(PthreadMutexTrylock(&mutex) == KERNEL_ERROR_EBUSY, "FIFO waiter owns mutex"); + waiter.release = true; + WaitForMutexTest([&] { return waiter.finished.load(); }); + Check(PthreadJoin(waiter.thread, nullptr) == OK && waiter.result == OK, + "FIFO waiter completes"); + } + Check(PthreadMutexLock(&mutex) == OK && PthreadMutexUnlock(&mutex) == OK && + PthreadMutexDestroy(&mutex) == OK, + "mutex remains usable after timed waiter removal"); + } + Check(PthreadMutexattrDestroy(&attr) == OK, "destroy recursive mutex attributes"); + + PthreadMutex mutex = nullptr; + Check(PthreadMutexInit(&mutex, nullptr, "MutexCancel") == OK && + PthreadMutexLock(&mutex) == OK && + PthreadMutexTimedlock(&mutex, 5000) == KERNEL_ERROR_EDEADLK, + "nonrecursive self timed lock reports deadlock"); + MutexWaiter waiter{&mutex}; + waiter.cancel = true; + QueueMutexWaiter(waiter); + Check(PthreadCancel(waiter.thread) == OK, "cancel queued mutex waiter"); + PthreadQueuePendingSignal(waiter.thread, 30); + waiter.resume = 1; + WaitForMutexTest([&] { return waiter.parked.load() == 2; }); + Check(PthreadMutexUnlock(&mutex) == OK && + PthreadMutexTrylock(&mutex) == KERNEL_ERROR_EBUSY, + "canceled waiter retains its queue position"); + waiter.release = true; + waiter.resume = 2; + void *result = nullptr; + Check(PthreadJoin(waiter.thread, &result) == OK && + result == reinterpret_cast(-1) && waiter.acquired && waiter.finished, + "mutex lock defers cancellation until explicit cancellation point"); + Check(PthreadMutexLock(&mutex) == OK && PthreadMutexUnlock(&mutex) == OK && + PthreadMutexDestroy(&mutex) == OK && remove(30) == OK, + "mutex remains usable after waiter cancellation"); +} + class TempDirectory { public: TempDirectory() { @@ -2260,7 +2405,7 @@ void CheckSocketWakeup() { } // namespace -int main(int, char**) { +int main(int argc, char** argv) { Common::InitializeThreads(); Common::Subsystems subsystems; subsystems.Initialize(); @@ -2269,6 +2414,13 @@ int main(int, char**) { Config::Load(options); subsystems.Initialize(); + TestMutexHandoff(); + if (argc == 2 && std::string_view(argv[1]) == "--mutex-only") { + subsystems.Destroy(); + std::printf("KernelFileSystemTests: mutex cases passed\n"); + return 0; + } + Check(SDL_InitSubSystem(SDL_INIT_VIDEO), "initialize Vulkan test video"); auto graphics = std::make_unique(); graphics->graphic_ctx.screen_width = 64;