From 63629429288274438eba32d7ab8b209fcc4ae68a Mon Sep 17 00:00:00 2001 From: kd-11 Date: Tue, 29 May 2018 14:53:16 +0300 Subject: [PATCH] rsx: Avoid semaphore acquire deadlock --- rpcs3/Emu/RSX/D3D12/D3D12GSRender.cpp | 11 +++++-- rpcs3/Emu/RSX/D3D12/D3D12GSRender.h | 2 +- rpcs3/Emu/RSX/GL/GLGSRender.cpp | 18 ++++++++---- rpcs3/Emu/RSX/GL/GLGSRender.h | 2 +- rpcs3/Emu/RSX/RSXThread.cpp | 6 ++-- rpcs3/Emu/RSX/RSXThread.h | 9 ++++-- rpcs3/Emu/RSX/VK/VKGSRender.cpp | 27 +++++------------ rpcs3/Emu/RSX/VK/VKGSRender.h | 3 +- rpcs3/Emu/RSX/rsx_methods.cpp | 42 ++++++++++++--------------- 9 files changed, 59 insertions(+), 61 deletions(-) diff --git a/rpcs3/Emu/RSX/D3D12/D3D12GSRender.cpp b/rpcs3/Emu/RSX/D3D12/D3D12GSRender.cpp index adb6af3593..297580e766 100644 --- a/rpcs3/Emu/RSX/D3D12/D3D12GSRender.cpp +++ b/rpcs3/Emu/RSX/D3D12/D3D12GSRender.cpp @@ -301,10 +301,15 @@ void D3D12GSRender::on_exit() return GSRender::on_exit(); } -void D3D12GSRender::do_local_task(bool) +void D3D12GSRender::do_local_task(rsx::FIFO_state state) { - //TODO - m_frame->clear_wm_events(); + if (state != rsx::FIFO_state::lock_wait) + { + //TODO + m_frame->clear_wm_events(); + } + + rsx::thread::do_local_task(state); } bool D3D12GSRender::do_method(u32 cmd, u32 arg) diff --git a/rpcs3/Emu/RSX/D3D12/D3D12GSRender.h b/rpcs3/Emu/RSX/D3D12/D3D12GSRender.h index 7674cd13f5..b91bff22ab 100644 --- a/rpcs3/Emu/RSX/D3D12/D3D12GSRender.h +++ b/rpcs3/Emu/RSX/D3D12/D3D12GSRender.h @@ -173,7 +173,7 @@ private: protected: virtual void on_init_thread() override; virtual void on_exit() override; - virtual void do_local_task(bool idle) override; + virtual void do_local_task(rsx::FIFO_state state) override; virtual bool do_method(u32 cmd, u32 arg) override; virtual void end() override; virtual void flip(int buffer) override; diff --git a/rpcs3/Emu/RSX/GL/GLGSRender.cpp b/rpcs3/Emu/RSX/GL/GLGSRender.cpp index d052e98eaa..87073d6bab 100644 --- a/rpcs3/Emu/RSX/GL/GLGSRender.cpp +++ b/rpcs3/Emu/RSX/GL/GLGSRender.cpp @@ -1582,10 +1582,8 @@ void GLGSRender::on_invalidate_memory_range(u32 address_base, u32 size) } } -void GLGSRender::do_local_task(bool idle) +void GLGSRender::do_local_task(rsx::FIFO_state state) { - m_frame->clear_wm_events(); - if (!work_queue.empty()) { std::lock_guard lock(queue_guard); @@ -1605,13 +1603,23 @@ void GLGSRender::do_local_task(bool idle) q.cv.notify_one(); } } - else if (!in_begin_end) + else if (!in_begin_end && state != rsx::FIFO_state::lock_wait) { //This will re-engage locks and break the texture cache if another thread is waiting in access violation handler! //Only call when there are no waiters m_gl_texture_cache.do_update(); } + rsx::thread::do_local_task(state); + + if (state == rsx::FIFO_state::lock_wait) + { + // Critical check finished + return; + } + + m_frame->clear_wm_events(); + if (m_overlay_manager) { if (!in_begin_end && native_ui_flip_request.load()) @@ -1620,8 +1628,6 @@ void GLGSRender::do_local_task(bool idle) flip((s32)current_display_buffer); } } - - rsx::thread::do_local_task(idle); } work_item& GLGSRender::post_flush_request(u32 address, gl::texture_cache::thrashed_set& flush_data) diff --git a/rpcs3/Emu/RSX/GL/GLGSRender.h b/rpcs3/Emu/RSX/GL/GLGSRender.h index 0ea8d08d95..ebef600d4c 100644 --- a/rpcs3/Emu/RSX/GL/GLGSRender.h +++ b/rpcs3/Emu/RSX/GL/GLGSRender.h @@ -381,7 +381,7 @@ protected: bool do_method(u32 id, u32 arg) override; void flip(int buffer) override; - void do_local_task(bool idle) override; + void do_local_task(rsx::FIFO_state state) override; bool on_access_violation(u32 address, bool is_writing) override; void on_invalidate_memory_range(u32 address_base, u32 size) override; diff --git a/rpcs3/Emu/RSX/RSXThread.cpp b/rpcs3/Emu/RSX/RSXThread.cpp index 0a89936bd3..20e18a3271 100644 --- a/rpcs3/Emu/RSX/RSXThread.cpp +++ b/rpcs3/Emu/RSX/RSXThread.cpp @@ -513,7 +513,7 @@ namespace rsx } //Execute backend-local tasks first - do_local_task(performance_counters.state != FIFO_state::running); + do_local_task(performance_counters.state); //Update sub-units zcull_ctrl->update(this); @@ -1290,9 +1290,9 @@ namespace rsx } } - void thread::do_local_task(bool /*idle*/) + void thread::do_local_task(FIFO_state state) { - if (!in_begin_end) + if (!in_begin_end && state != FIFO_state::lock_wait) { if (!m_invalidated_memory_ranges.empty()) { diff --git a/rpcs3/Emu/RSX/RSXThread.h b/rpcs3/Emu/RSX/RSXThread.h index d9b81ae550..3725706669 100644 --- a/rpcs3/Emu/RSX/RSXThread.h +++ b/rpcs3/Emu/RSX/RSXThread.h @@ -87,6 +87,7 @@ namespace rsx empty = 1, // PUT == GET spinning = 2, // Puller continuously jumps to self addr (synchronization technique) nop = 3, // Puller is processing a NOP command + lock_wait = 4 // Puller is processing a lock acquire }; u32 get_vertex_type_size_on_host(vertex_base_type type, u32 size); @@ -417,9 +418,8 @@ namespace rsx /** * Execute a backend local task queue - * Idle argument checks that the FIFO queue is in an idle state */ - virtual void do_local_task(bool idle); + virtual void do_local_task(FIFO_state state); public: virtual std::string get_name() const override; @@ -545,6 +545,11 @@ namespace rsx */ void on_notify_memory_unmapped(u32 address_base, u32 size); + /** + * Notify to check internal state during semaphore wait + */ + void on_semaphore_acquire_wait() { do_local_task(FIFO_state::lock_wait); } + /** * Copy rtt values to buffer. * TODO: It's more efficient to combine multiple call of this function into one. diff --git a/rpcs3/Emu/RSX/VK/VKGSRender.cpp b/rpcs3/Emu/RSX/VK/VKGSRender.cpp index 6f9631027f..53e8be533e 100644 --- a/rpcs3/Emu/RSX/VK/VKGSRender.cpp +++ b/rpcs3/Emu/RSX/VK/VKGSRender.cpp @@ -857,9 +857,6 @@ bool VKGSRender::on_access_violation(u32 address, bool is_writing) if (target_cb) target_cb->wait(); - - if (is_rsxthr) - m_last_flushable_cb = -1; } if (has_queue_ref) @@ -1858,7 +1855,6 @@ void VKGSRender::copy_render_targets_to_dma_location() vk::leave_uninterruptible(); - m_last_flushable_cb = m_current_cb_index; flush_command_queue(); m_flush_draw_buffers = false; @@ -1884,7 +1880,6 @@ void VKGSRender::flush_command_queue(bool hard_sync) cb.poke(); } - m_last_flushable_cb = -1; m_flush_requests.clear_pending_flag(); } else @@ -1904,9 +1899,6 @@ void VKGSRender::flush_command_queue(bool hard_sync) } m_current_command_buffer->reset(); - - if (m_last_flushable_cb == m_current_cb_index) - m_last_flushable_cb = -1; } open_command_buffer(); @@ -2089,7 +2081,7 @@ void VKGSRender::process_swap_request(frame_context_t *ctx, bool free_resources) ctx->swap_command_buffer = nullptr; } -void VKGSRender::do_local_task(bool idle) +void VKGSRender::do_local_task(rsx::FIFO_state state) { if (m_flush_requests.pending()) { @@ -2102,22 +2094,19 @@ void VKGSRender::do_local_task(bool idle) m_flush_requests.clear_pending_flag(); m_flush_requests.consumer_wait(); } - else if (!in_begin_end) + else if (!in_begin_end && state != rsx::FIFO_state::lock_wait) { //This will re-engage locks and break the texture cache if another thread is waiting in access violation handler! //Only call when there are no waiters m_texture_cache.do_update(); } - if (m_last_flushable_cb > -1) + rsx::thread::do_local_task(state); + + if (state == rsx::FIFO_state::lock_wait) { - auto cb = &m_primary_cb_list[m_last_flushable_cb]; - - if (cb->pending) - cb->poke(); - - if (!cb->pending) - m_last_flushable_cb = -1; + // Critical check finished + return; } #ifdef _WIN32 @@ -2208,8 +2197,6 @@ void VKGSRender::do_local_task(bool idle) flip((s32)current_display_buffer); } } - - rsx::thread::do_local_task(idle); } bool VKGSRender::do_method(u32 cmd, u32 arg) diff --git a/rpcs3/Emu/RSX/VK/VKGSRender.h b/rpcs3/Emu/RSX/VK/VKGSRender.h index 8f6dbac4aa..c690ade806 100644 --- a/rpcs3/Emu/RSX/VK/VKGSRender.h +++ b/rpcs3/Emu/RSX/VK/VKGSRender.h @@ -351,7 +351,6 @@ private: u8 m_draw_buffers_count = 0; bool m_flush_draw_buffers = false; - std::atomic m_last_flushable_cb = {-1 }; shared_mutex m_flush_queue_mutex; flush_request_task m_flush_requests; @@ -420,7 +419,7 @@ protected: bool do_method(u32 id, u32 arg) override; void flip(int buffer) override; - void do_local_task(bool idle) override; + void do_local_task(rsx::FIFO_state state) override; bool scaled_image_from_memory(rsx::blit_src_info& src, rsx::blit_dst_info& dst, bool interpolate) override; void notify_tile_unbound(u32 tile) override; diff --git a/rpcs3/Emu/RSX/rsx_methods.cpp b/rpcs3/Emu/RSX/rsx_methods.cpp index 739ac99fa3..f44eea9c24 100644 --- a/rpcs3/Emu/RSX/rsx_methods.cpp +++ b/rpcs3/Emu/RSX/rsx_methods.cpp @@ -78,35 +78,31 @@ namespace rsx if (Emu.IsStopped()) return; - const auto tdr = (u64)g_cfg.video.driver_recovery_timeout; - if (tdr == 0) + if (const auto tdr = (u64)g_cfg.video.driver_recovery_timeout) { - //No timeout - std::this_thread::yield(); - continue; - } - - if (Emu.IsPaused()) - { - while (Emu.IsPaused()) + if (Emu.IsPaused()) { - std::this_thread::yield(); - } + while (Emu.IsPaused()) + { + std::this_thread::sleep_for(1ms); + } - //Reset - start = get_system_time(); - } - else - { - if ((get_system_time() - start) > tdr) + // Reset + start = get_system_time(); + } + else { - //If longer than driver timeout force exit - LOG_ERROR(RSX, "nv406e::semaphore_acquire has timed out. semaphore_address=0x%X", addr); - break; + if ((get_system_time() - start) > tdr) + { + // If longer than driver timeout force exit + LOG_ERROR(RSX, "nv406e::semaphore_acquire has timed out. semaphore_address=0x%X", addr); + break; + } } - - std::this_thread::yield(); } + + rsx->on_semaphore_acquire_wait(); + std::this_thread::yield(); } rsx->performance_counters.idle_time += (get_system_time() - start);