From f40602cc599f281904ba70a4af49e3c272792cd3 Mon Sep 17 00:00:00 2001 From: Eladash <18193363+elad335@users.noreply.github.com> Date: Thu, 4 Jan 2024 09:30:11 +0200 Subject: [PATCH] cellGame: Fix PPU deadlocks on concurrent execution --- rpcs3/Emu/Cell/Modules/cellGame.cpp | 24 +++-- rpcs3/Emu/Cell/lv2/lv2.cpp | 56 ++++++++++ rpcs3/util/init_mutex.hpp | 153 ++++++++++++++++++++-------- 3 files changed, 178 insertions(+), 55 deletions(-) diff --git a/rpcs3/Emu/Cell/Modules/cellGame.cpp b/rpcs3/Emu/Cell/Modules/cellGame.cpp index a8817ae323..7aacb6b116 100644 --- a/rpcs3/Emu/Cell/Modules/cellGame.cpp +++ b/rpcs3/Emu/Cell/Modules/cellGame.cpp @@ -28,6 +28,10 @@ vm::gvar g_stat_set; vm::gvar g_file_param; vm::gvar g_cb_result; +stx::init_lock acquire_lock(stx::init_mutex& mtx, ppu_thread* ppu = nullptr); +stx::access_lock acquire_access_lock(stx::init_mutex& mtx, ppu_thread* ppu = nullptr); +stx::reset_lock acquire_reset_lock(stx::init_mutex& mtx, ppu_thread* ppu = nullptr); + template<> void fmt_class_string::format(std::string& out, u64 arg) { @@ -705,7 +709,7 @@ error_code cellGameBootCheck(vm::ptr type, vm::ptr attributes, vm::ptr lv2_sleep(500); - const auto init = perm.init.init(); + const auto init = acquire_lock(perm.init); if (!init) { @@ -789,7 +793,7 @@ error_code cellGamePatchCheck(vm::ptr size, vm::ptr r auto& perm = g_fxo->get(); - const auto init = perm.init.init(); + const auto init = acquire_lock(perm.init); if (!init) { @@ -836,7 +840,7 @@ error_code cellGameDataCheck(u32 type, vm::cptr dirName, vm::ptrget(); - auto init = perm.init.init(); + auto init = acquire_lock(perm.init); if (!init) { @@ -904,7 +908,7 @@ error_code cellGameContentPermit(ppu_thread& ppu, vm::ptrget(); - const auto init = perm.init.reset(); + const auto init = acquire_reset_lock(perm.init); if (!init) { @@ -1204,7 +1208,7 @@ error_code cellGameCreateGameData(vm::ptr init, vm::ptrget(); - const auto _init = perm.init.access(); + const auto _init = acquire_access_lock(perm.init); lv2_sleep(2000); @@ -1319,7 +1323,7 @@ error_code cellGameDeleteGameData(vm::cptr dirName) if (!_init) { // Or access it - if (auto access = perm.init.access(); access) + if (auto access = acquire_access_lock(perm.init); access) { // Cannot remove it when it is accessed by cellGameDataCheck // If it is HG data then resort to remove_gd for ERROR_BROKEN @@ -1356,7 +1360,7 @@ error_code cellGameGetParamInt(s32 id, vm::ptr value) auto& perm = g_fxo->get(); - const auto init = perm.init.access(); + const auto init = acquire_access_lock(perm.init); if (!init) { @@ -1484,7 +1488,7 @@ error_code cellGameGetParamString(s32 id, vm::ptr buf, u32 bufsize) lv2_sleep(2000); - const auto init = perm.init.access(); + const auto init = acquire_access_lock(perm.init); if (!init || perm.mode == content_permission::check_mode::not_set) { @@ -1530,7 +1534,7 @@ error_code cellGameSetParamString(s32 id, vm::cptr buf) auto& perm = g_fxo->get(); - const auto init = perm.init.access(); + const auto init = acquire_access_lock(perm.init); if (!init || perm.mode == content_permission::check_mode::not_set) { @@ -1569,7 +1573,7 @@ error_code cellGameGetSizeKB(ppu_thread& ppu, vm::ptr size) auto& perm = g_fxo->get(); - const auto init = perm.init.access(); + const auto init = acquire_access_lock(perm.init); if (!init) { diff --git a/rpcs3/Emu/Cell/lv2/lv2.cpp b/rpcs3/Emu/Cell/lv2/lv2.cpp index 8ad00079cc..1bade5b7ce 100644 --- a/rpcs3/Emu/Cell/lv2/lv2.cpp +++ b/rpcs3/Emu/Cell/lv2/lv2.cpp @@ -55,6 +55,7 @@ #include #include "util/tsc.hpp" #include "util/sysinfo.hpp" +#include "util/init_mutex.hpp" #if defined(ARCH_X64) #ifdef _MSC_VER @@ -1108,6 +1109,61 @@ void fmt_class_string::format(std::string& out, u64 arg) }); } +stx::init_lock acquire_lock(stx::init_mutex& mtx, ppu_thread* ppu) +{ + if (!ppu) + { + ppu = ensure(cpu_thread::get_current()); + } + + return mtx.init([](int invoke_count, const stx::init_lock&, ppu_thread* ppu) + { + if (!invoke_count) + { + // Sleep before waiting on lock + lv2_obj::sleep(*ppu); + } + else + { + // Wake up after acquistion or failure to acquire + ppu->check_state(); + } + }, ppu); +} + +stx::access_lock acquire_access_lock(stx::init_mutex& mtx, ppu_thread* ppu) +{ + if (!ppu) + { + ppu = ensure(cpu_thread::get_current()); + } + + // TODO: Check if needs to wait + return mtx.access(); +} + +stx::reset_lock acquire_reset_lock(stx::init_mutex& mtx, ppu_thread* ppu) +{ + if (!ppu) + { + ppu = ensure(cpu_thread::get_current()); + } + + return mtx.reset([](int invoke_count, const stx::init_lock&, ppu_thread* ppu) + { + if (!invoke_count) + { + // Sleep before waiting on lock + lv2_obj::sleep(*ppu); + } + else + { + // Wake up after acquistion or failure to acquire + ppu->check_state(); + } + }, ppu); +} + class ppu_syscall_usage { // Internal buffer diff --git a/rpcs3/util/init_mutex.hpp b/rpcs3/util/init_mutex.hpp index 1d73bb0f5d..19ff2aedb7 100644 --- a/rpcs3/util/init_mutex.hpp +++ b/rpcs3/util/init_mutex.hpp @@ -20,11 +20,19 @@ namespace stx { init_mutex* _this; + template + void invoke_callback(int invoke_count, Func&& func, Args&&... args) const + { + std::invoke(func, invoke_count, *this, std::forward(args)...); + } + public: - template - explicit init_lock(init_mutex& mtx, FAndArgs&&... args) noexcept + template + explicit init_lock(init_mutex& mtx, Forced&&, FAndArgs&&... args) noexcept : _this(&mtx) { + bool invoked_func = false; + while (true) { auto [val, ok] = _this->m_state.fetch_op([](u32& value) @@ -35,7 +43,7 @@ namespace stx return true; } - if constexpr (sizeof...(FAndArgs)) + if constexpr (Forced()()) { if (value & c_init_bit) { @@ -50,12 +58,21 @@ namespace stx if (val == 0) { // Success: obtained "init lock" + + if constexpr (sizeof...(FAndArgs)) + { + if (invoked_func) + { + invoke_callback(1, std::forward(args)...); + } + } + break; } if (val & c_init_bit) { - if constexpr (sizeof...(FAndArgs)) + if constexpr (Forced()()) { // Forced reset val -= c_init_bit - 1; @@ -67,22 +84,43 @@ namespace stx val = _this->m_state; } - // Call specified reset function - std::invoke(std::forward(args)...); break; } // Failure _this = nullptr; + + if constexpr (sizeof...(FAndArgs)) + { + if (invoked_func) + { + invoke_callback(1, std::forward(args)...); + } + } + break; } + if constexpr (sizeof...(FAndArgs)) + { + if (!invoked_func) + { + invoke_callback(0, std::forward(args)...); + invoked_func = true; + } + } + _this->m_state.wait(val); } } init_lock(const init_lock&) = delete; + init_lock(init_lock&& lock) noexcept + : _this(std::exchange(lock._this, nullptr)) + { + } + init_lock& operator=(const init_lock&) = delete; ~init_lock() @@ -102,6 +140,11 @@ namespace stx explicit operator bool() && = delete; + void force_lock(init_mutex* mtx) + { + _this = mtx; + } + void cancel() & noexcept { if (_this) @@ -112,54 +155,43 @@ namespace stx _this = nullptr; } } + + init_mutex* get_mutex() + { + return _this; + } }; // Obtain exclusive lock to initialize protected resource. Waits for ongoing initialization or finalization. Fails if already initialized. - [[nodiscard]] init_lock init() noexcept + template + [[nodiscard]] init_lock init(FAndArgs&&... args) noexcept { - return init_lock(*this); + return init_lock(*this, std::false_type{}, std::forward(args)...); } // Same as init, but never fails, and executes provided `on_reset` function if already initialized. template [[nodiscard]] init_lock init_always(F on_reset, Args&&... args) noexcept { - return init_lock(*this, std::move(on_reset), std::forward(args)...); + init_lock lock(*this, std::true_type{}); + + if (!lock) + { + lock.force_lock(this); + std::invoke(std::forward(on_reset), std::forward(args)...); + } + + return lock; } class reset_lock final { - init_mutex* _this; + init_lock _lock; public: - explicit reset_lock(init_mutex& mtx) noexcept - : _this(&mtx) + explicit reset_lock(init_lock&& lock) noexcept + : _lock(std::move(lock)) { - auto [val, ok] = _this->m_state.fetch_op([](u32& value) - { - if (value & c_init_bit) - { - // Remove initialized state and add "init lock" - value -= c_init_bit - 1; - return true; - } - - return false; - }); - - if (!ok) - { - // Failure: not initialized - _this = nullptr; - return; - } - - while (val != 1) - { - // Wait for other users to finish their work - _this->m_state.wait(val); - val = _this->m_state; - } } reset_lock(const reset_lock&) = delete; @@ -168,36 +200,63 @@ namespace stx ~reset_lock() { - if (_this) + if (_lock) { // Set uninitialized state and remove "init lock" - _this->m_state -= 1; - _this->m_state.notify_all(); + _lock.cancel(); } } explicit operator bool() const& noexcept { - return _this != nullptr; + return !!_lock; } explicit operator bool() && = delete; void set_init() & noexcept { - if (_this) + if (_lock) { // Set initialized state (TODO?) - _this->m_state |= c_init_bit; - _this->m_state.notify_all(); + _lock.get_mutex()->m_state |= c_init_bit; + _lock.get_mutex()->m_state.notify_all(); } } }; // Obtain exclusive lock to finalize protected resource. Waits for ongoing use. Fails if not initialized. + template + [[nodiscard]] reset_lock reset(F on_reset, Args&&... args) noexcept + { + init_lock lock(*this, std::true_type{}, std::forward(on_reset), std::forward(args)...); + + if (!lock) + { + lock.force_lock(this); + } + else + { + lock.cancel(); + } + + return reset_lock(std::move(lock)); + } + [[nodiscard]] reset_lock reset() noexcept { - return reset_lock(*this); + init_lock lock(*this, std::true_type{}); + + if (!lock) + { + lock.force_lock(this); + } + else + { + lock.cancel(); + } + + return reset_lock(std::move(lock)); } class access_lock final @@ -276,4 +335,8 @@ namespace stx m_state.wait(state); } }; + + using init_lock = init_mutex::init_lock; + using reset_lock = init_mutex::reset_lock; + using access_lock = init_mutex::access_lock; }