From 44f0ca08e28c078fcf684c0e8902dcead6105f5c Mon Sep 17 00:00:00 2001 From: Eladash Date: Sat, 22 May 2021 09:02:30 +0300 Subject: [PATCH] Simplify PPU exit --- rpcs3/Emu/Cell/lv2/sys_cond.cpp | 3 +- rpcs3/Emu/Cell/lv2/sys_lwcond.cpp | 2 +- rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp | 142 ++++++++------------------ rpcs3/rpcs3qt/kernel_explorer.cpp | 7 +- 4 files changed, 49 insertions(+), 105 deletions(-) diff --git a/rpcs3/Emu/Cell/lv2/sys_cond.cpp b/rpcs3/Emu/Cell/lv2/sys_cond.cpp index 95e193ac5df9..d14843676723 100644 --- a/rpcs3/Emu/Cell/lv2/sys_cond.cpp +++ b/rpcs3/Emu/Cell/lv2/sys_cond.cpp @@ -154,8 +154,7 @@ error_code sys_cond_signal_to(ppu_thread& ppu, u32 cond_id, u32 thread_id) const auto cond = idm::check(cond_id, [&](lv2_cond& cond) -> int { - if (const auto cpu = idm::check_unlocked>(thread_id); - !cpu || cpu->joiner == ppu_join_status::exited) + if (!idm::check_unlocked>(thread_id)) { return -1; } diff --git a/rpcs3/Emu/Cell/lv2/sys_lwcond.cpp b/rpcs3/Emu/Cell/lv2/sys_lwcond.cpp index 06d07c1d3a76..67c0f6156887 100644 --- a/rpcs3/Emu/Cell/lv2/sys_lwcond.cpp +++ b/rpcs3/Emu/Cell/lv2/sys_lwcond.cpp @@ -93,7 +93,7 @@ error_code _sys_lwcond_signal(ppu_thread& ppu, u32 lwcond_id, u32 lwmutex_id, u6 { cpu = idm::check_unlocked>(static_cast(ppu_thread_id)); - if (!cpu || cpu->joiner == ppu_join_status::exited) + if (!cpu) { return -1; } diff --git a/rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp b/rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp index c38f88b2342f..ce48ebdfbcea 100644 --- a/rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp +++ b/rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp @@ -21,29 +21,18 @@ LOG_CHANNEL(sys_ppu_thread); // Simple structure to cleanup previous thread, because can't remove its own thread struct ppu_thread_cleaner { - atomic_t old_id = 0; + std::shared_ptr old; - void clean(u32 new_id) + std::shared_ptr clean(std::shared_ptr ptr) { - if (old_id || new_id) [[likely]] - { - if (u32 id = old_id.exchange(new_id)) [[likely]] - { - auto ppu = idm::get>(id); + return std::exchange(old, std::move(ptr)); + } - if (ppu) - { - // Join thread - (*ppu)(); - } + ppu_thread_cleaner() = default; - if (!ppu || !idm::remove_verify>(id, std::move(ppu))) [[unlikely]] - { - sys_ppu_thread.fatal("Failed to remove detached thread 0x%x", id); - } - } - } - } + ppu_thread_cleaner(const ppu_thread_cleaner&) = delete; + + ppu_thread_cleaner& operator=(const ppu_thread_cleaner&) = delete; }; bool ppu_thread_exit(ppu_thread& ppu) @@ -72,6 +61,9 @@ void _sys_ppu_thread_exit(ppu_thread& ppu, u64 errorcode) ppu_join_status old_status; { + // Avoid cases where cleaning causes the destructor to be called inside IDM lock scope (for performance) + std::shared_ptr old_ppu; + std::lock_guard lock(id_manager::g_mutex); // Get joiner ID @@ -93,6 +85,12 @@ void _sys_ppu_thread_exit(ppu_thread& ppu, u64 errorcode) lv2_obj::append(idm::check_unlocked>(static_cast(old_status))); } + if (old_status != ppu_join_status::joinable) + { + // Remove self ID from IDM, move owning ptr + old_ppu = g_fxo->get().clean(std::move(idm::find_unlocked>(ppu.id)->second)); + } + // Unqueue lv2_obj::sleep(ppu); @@ -100,8 +98,6 @@ void _sys_ppu_thread_exit(ppu_thread& ppu, u64 errorcode) ppu.state -= cpu_flag::suspend; } - g_fxo->get().clean(old_status == ppu_join_status::detached ? ppu.id : 0); - while (ppu.joiner == ppu_join_status::zombie && !ppu.is_stopped()) { // Wait for termination @@ -127,9 +123,6 @@ error_code sys_ppu_thread_join(ppu_thread& ppu, u32 thread_id, vm::ptr vptr sys_ppu_thread.trace("sys_ppu_thread_join(thread_id=0x%x, vptr=*0x%x)", thread_id, vptr); - // Clean some detached thread (hack) - g_fxo->get().clean(0); - auto thread = idm::get>(thread_id, [&](ppu_thread& thread) -> CellError { if (&ppu == &thread) @@ -184,16 +177,21 @@ error_code sys_ppu_thread_join(ppu_thread& ppu, u32 thread_id, vm::ptr vptr // Wait for cleanup (*thread.ptr)(); - if (ppu.test_stopped()) + if (thread->joiner != ppu_join_status::exited) { + // Thread aborted, log it later + ppu.state += cpu_flag::exit; return {}; } // Get the exit status from the register const u64 vret = thread->gpr[3]; - // Cleanup - ensure(idm::remove_verify>(thread_id, std::move(thread.ptr))); + if (thread.ret == CELL_EAGAIN) + { + // Cleanup + ensure(idm::remove_verify>(thread_id, std::move(thread.ptr))); + } if (!vptr) { @@ -210,12 +208,11 @@ error_code sys_ppu_thread_detach(ppu_thread& ppu, u32 thread_id) sys_ppu_thread.trace("sys_ppu_thread_detach(thread_id=0x%x)", thread_id); - // Clean some detached thread (hack) - g_fxo->get().clean(0); + CellError result = CELL_ESRCH; - const auto thread = idm::check>(thread_id, [&](ppu_thread& thread) -> CellError + idm::withdraw>(thread_id, [&](ppu_thread& thread) { - CellError result = thread.joiner.atomic_op([](ppu_join_status& value) -> CellError + result = thread.joiner.atomic_op([](ppu_join_status& value) -> CellError { if (value == ppu_join_status::zombie) { @@ -247,23 +244,13 @@ error_code sys_ppu_thread_detach(ppu_thread& ppu, u32 thread_id) thread.joiner.notify_one(); } - return result; - }); - - if (!thread) - { - return CELL_ESRCH; - } - - if (thread.ret && thread.ret != CELL_EAGAIN) - { - return thread.ret; - } + // Remove ID on EAGAIN + return result != CELL_EAGAIN; + }).ptr; - if (thread.ret == CELL_EAGAIN) + if (result) { - g_fxo->get().clean(thread_id); - g_fxo->get().clean(0); + return result; } return CELL_OK; @@ -293,25 +280,15 @@ error_code sys_ppu_thread_set_priority(ppu_thread& ppu, u32 thread_id, s32 prio) return CELL_EINVAL; } - // Clean some detached thread (hack) - g_fxo->get().clean(0); - const auto thread = idm::check>(thread_id, [&](ppu_thread& thread) { - if (thread.joiner == ppu_join_status::exited) - { - return false; - } - if (thread.prio != prio) { lv2_obj::set_priority(thread, prio); } - - return true; }); - if (!thread || !thread.ret) + if (!thread) { return CELL_ESRCH; } @@ -325,23 +302,14 @@ error_code sys_ppu_thread_get_priority(ppu_thread& ppu, u32 thread_id, vm::ptrget().clean(0); - u32 prio; const auto thread = idm::check>(thread_id, [&](ppu_thread& thread) { - if (thread.joiner == ppu_join_status::exited) - { - return false; - } - prio = thread.prio; - return true; }); - if (!thread || !thread.ret) + if (!thread) { return CELL_ESRCH; } @@ -371,12 +339,9 @@ error_code sys_ppu_thread_stop(ppu_thread& ppu, u32 thread_id) return CELL_ENOSYS; } - const auto thread = idm::check>(thread_id, [&](ppu_thread& thread) - { - return thread.joiner != ppu_join_status::exited; - }); + const auto thread = idm::check>(thread_id); - if (!thread || !thread.ret) + if (!thread) { return CELL_ESRCH; } @@ -426,9 +391,6 @@ error_code _sys_ppu_thread_create(ppu_thread& ppu, vm::ptr thread_id, vm::p const ppu_func_opd_t entry = param->entry.opd(); const u32 tls = param->tls; - // Clean some detached thread (hack) - g_fxo->get().clean(0); - // Compute actual stack size and allocate const u32 stack_size = utils::align(std::max(_stacksz, 4096), 4096); @@ -490,11 +452,6 @@ error_code sys_ppu_thread_start(ppu_thread& ppu, u32 thread_id) const auto thread = idm::get>(thread_id, [&](ppu_thread& thread) -> CellError { - if (thread.joiner == ppu_join_status::exited) - { - return CELL_ESRCH; - } - if (!thread.state.test_and_reset(cpu_flag::stop)) { // Already started @@ -560,12 +517,9 @@ error_code sys_ppu_thread_rename(ppu_thread& ppu, u32 thread_id, vm::cptr sys_ppu_thread.warning("sys_ppu_thread_rename(thread_id=0x%x, name=*0x%x)", thread_id, name); - const auto thread = idm::get>(thread_id, [&](ppu_thread& thread) - { - return thread.joiner != ppu_join_status::exited; - }); + const auto thread = idm::get>(thread_id); - if (!thread || !thread.ret) + if (!thread) { return CELL_ESRCH; } @@ -593,17 +547,14 @@ error_code sys_ppu_thread_recover_page_fault(ppu_thread& ppu, u32 thread_id) sys_ppu_thread.warning("sys_ppu_thread_recover_page_fault(thread_id=0x%x)", thread_id); - const auto thread = idm::get>(thread_id, [&](ppu_thread& thread) - { - return thread.joiner != ppu_join_status::exited; - }); + const auto thread = idm::get>(thread_id); - if (!thread || !thread.ret) + if (!thread) { return CELL_ESRCH; } - return mmapper_thread_recover_page_fault(thread.ptr.get()); + return mmapper_thread_recover_page_fault(thread.get()); } error_code sys_ppu_thread_get_page_fault_context(ppu_thread& ppu, u32 thread_id, vm::ptr ctxt) @@ -612,12 +563,9 @@ error_code sys_ppu_thread_get_page_fault_context(ppu_thread& ppu, u32 thread_id, sys_ppu_thread.todo("sys_ppu_thread_get_page_fault_context(thread_id=0x%x, ctxt=*0x%x)", thread_id, ctxt); - const auto thread = idm::get>(thread_id, [&](ppu_thread& thread) - { - return thread.joiner != ppu_join_status::exited; - }); + const auto thread = idm::get>(thread_id); - if (!thread || !thread.ret) + if (!thread) { return CELL_ESRCH; } @@ -626,7 +574,7 @@ error_code sys_ppu_thread_get_page_fault_context(ppu_thread& ppu, u32 thread_id, auto& pf_events = g_fxo->get(); reader_lock lock(pf_events.pf_mutex); - const auto evt = pf_events.events.find(thread.ptr.get()); + const auto evt = pf_events.events.find(thread.get()); if (evt == pf_events.events.end()) { return CELL_EINVAL; diff --git a/rpcs3/rpcs3qt/kernel_explorer.cpp b/rpcs3/rpcs3qt/kernel_explorer.cpp index e9d530c430f7..c09691335443 100644 --- a/rpcs3/rpcs3qt/kernel_explorer.cpp +++ b/rpcs3/rpcs3qt/kernel_explorer.cpp @@ -555,11 +555,8 @@ void kernel_explorer::Update() const auto func = ppu.last_function; const ppu_thread_status status = lv2_obj::ppu_state(&ppu, false); - if (status != PPU_THREAD_STATUS_DELETED) - { - add_leaf(find_node(root, additional_nodes::ppu_threads), qstr(fmt::format(u8"PPU 0x%07x: “%s”, PRIO: %d, Joiner: %s, Status: %s, State: %s, %s func: “%s”", id, *ppu.ppu_tname.load(), +ppu.prio, ppu.joiner.load(), status, ppu.state.load() - , ppu.current_function ? "In" : "Last", func ? func : ""))); - } + add_leaf(find_node(root, additional_nodes::ppu_threads), qstr(fmt::format(u8"PPU 0x%07x: “%s”, PRIO: %d, Joiner: %s, Status: %s, State: %s, %s func: “%s”", id, *ppu.ppu_tname.load(), +ppu.prio, ppu.joiner.load(), status, ppu.state.load() + , ppu.current_function ? "In" : "Last", func ? func : ""))); }); idm::select>([&](u32 /*id*/, spu_thread& spu)