From 8bbda3dedb87b26f9047362a56cb9f5a3ed9a032 Mon Sep 17 00:00:00 2001 From: kd-11 Date: Mon, 13 Jan 2020 20:46:58 +0300 Subject: [PATCH] vk: Restructure command queue flushing behavior to avoid deadlock - Queueing commands on the offloader is a good idea but unfortunately page faults can still happen causing a cyclic dependency and eventual deadlock. Characterized by a vk::wait_for_event timed out error accompanied by severe hitching. - Drain the fault-able commands before pushing a submit operation to the queue. If a fault is in progress, bypass the queue system and submit raw. Technically this is incorrect but there isn't much that can be done about it right now. --- rpcs3/Emu/RSX/RSXOffload.cpp | 8 +++++--- rpcs3/Emu/RSX/RSXOffload.h | 2 +- rpcs3/Emu/RSX/VK/VKCommandStream.cpp | 2 +- rpcs3/Emu/RSX/VK/VKGSRender.cpp | 18 ++++++++++++------ rpcs3/Emu/RSX/VK/VKHelpers.h | 17 +++++++++++------ rpcs3/Emu/RSX/VK/VKTextureCache.h | 4 ++-- 6 files changed, 32 insertions(+), 19 deletions(-) diff --git a/rpcs3/Emu/RSX/RSXOffload.cpp b/rpcs3/Emu/RSX/RSXOffload.cpp index 2f2d1ff41e..83aec187d2 100644 --- a/rpcs3/Emu/RSX/RSXOffload.cpp +++ b/rpcs3/Emu/RSX/RSXOffload.cpp @@ -137,12 +137,12 @@ namespace rsx return (std::this_thread::get_id() == m_thread_id); } - void dma_manager::sync() + bool dma_manager::sync() { if (LIKELY(m_enqueued_count.load() == m_processed_count)) { // Nothing to do - return; + return true; } if (auto rsxthr = get_current_renderer(); rsxthr->is_current_thread()) @@ -150,7 +150,7 @@ namespace rsx if (m_mem_fault_flag) { // Abort if offloader is in recovery mode - return; + return false; } while (m_enqueued_count.load() != m_processed_count) @@ -164,6 +164,8 @@ namespace rsx while (m_enqueued_count.load() != m_processed_count) _mm_pause(); } + + return true; } void dma_manager::join() diff --git a/rpcs3/Emu/RSX/RSXOffload.h b/rpcs3/Emu/RSX/RSXOffload.h index f436971f07..27b8e7b7bf 100644 --- a/rpcs3/Emu/RSX/RSXOffload.h +++ b/rpcs3/Emu/RSX/RSXOffload.h @@ -77,7 +77,7 @@ namespace rsx // Synchronization bool is_current_thread() const; - void sync(); + bool sync(); void join(); void set_mem_fault_flag(); void clear_mem_fault_flag(); diff --git a/rpcs3/Emu/RSX/VK/VKCommandStream.cpp b/rpcs3/Emu/RSX/VK/VKCommandStream.cpp index a333126bc9..ef72e0e716 100644 --- a/rpcs3/Emu/RSX/VK/VKCommandStream.cpp +++ b/rpcs3/Emu/RSX/VK/VKCommandStream.cpp @@ -30,7 +30,7 @@ namespace vk release_global_submit_lock(); // Signal fence - pfence->flushed = true; + pfence->signal_flushed(); } } } diff --git a/rpcs3/Emu/RSX/VK/VKGSRender.cpp b/rpcs3/Emu/RSX/VK/VKGSRender.cpp index 423b381adb..b8f554afca 100644 --- a/rpcs3/Emu/RSX/VK/VKGSRender.cpp +++ b/rpcs3/Emu/RSX/VK/VKGSRender.cpp @@ -2418,7 +2418,6 @@ void VKGSRender::do_local_task(rsx::FIFO_state state) // Clear offloader deadlock // NOTE: It is not possible to handle regular flush requests before this is cleared // NOTE: This may cause graphics corruption due to unsynchronized modification - flush_command_queue(); on_invalidate_memory_range(m_offloader_fault_range, m_offloader_fault_cause); m_queue_status.clear(flush_queue_state::deadlock); } @@ -2825,11 +2824,13 @@ void VKGSRender::init_buffers(rsx::framebuffer_creation_context context, bool) void VKGSRender::close_and_submit_command_buffer(vk::fence* pFence, VkSemaphore wait_semaphore, VkSemaphore signal_semaphore, VkPipelineStageFlags pipeline_stage_flags) { - // NOTE: There is no need to wait for dma sync. When MTRSX is enabled, the commands are submitted in order anyway due to CSMT + // Workaround for deadlock occuring during RSX offloader fault + // TODO: Restructure command submission infrastructure to avoid this condition + const bool sync_success = rsx::g_dma_manager.sync(); + const VkBool32 force_flush = !sync_success; + if (vk::test_status_interrupt(vk::heap_dirty)) { - rsx::g_dma_manager.sync(); - if (m_attrib_ring_info.dirty() || m_fragment_env_ring_info.dirty() || m_vertex_env_ring_info.dirty() || @@ -2856,7 +2857,7 @@ void VKGSRender::close_and_submit_command_buffer(vk::fence* pFence, VkSemaphore m_secondary_command_buffer.end(); m_secondary_command_buffer.submit(m_swapchain->get_graphics_queue(), - VK_NULL_HANDLE, VK_NULL_HANDLE, VK_NULL_HANDLE, VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT); + VK_NULL_HANDLE, VK_NULL_HANDLE, VK_NULL_HANDLE, VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT, force_flush); } vk::clear_status_interrupt(vk::heap_dirty); @@ -2888,7 +2889,12 @@ void VKGSRender::close_and_submit_command_buffer(vk::fence* pFence, VkSemaphore m_current_command_buffer->tag(); m_current_command_buffer->submit(m_swapchain->get_graphics_queue(), - wait_semaphore, signal_semaphore, pFence, pipeline_stage_flags); + wait_semaphore, signal_semaphore, pFence, pipeline_stage_flags, force_flush); + + if (force_flush) + { + verify(HERE), m_current_command_buffer->submit_fence->flushed; + } } void VKGSRender::open_command_buffer() diff --git a/rpcs3/Emu/RSX/VK/VKHelpers.h b/rpcs3/Emu/RSX/VK/VKHelpers.h index 044e25a488..cadc86ec89 100644 --- a/rpcs3/Emu/RSX/VK/VKHelpers.h +++ b/rpcs3/Emu/RSX/VK/VKHelpers.h @@ -1087,9 +1087,9 @@ private: struct fence { - volatile bool flushed = false; - VkFence handle = VK_NULL_HANDLE; - VkDevice owner = VK_NULL_HANDLE; + atomic_t flushed = false; + VkFence handle = VK_NULL_HANDLE; + VkDevice owner = VK_NULL_HANDLE; fence(VkDevice dev) { @@ -1111,7 +1111,12 @@ private: void reset() { vkResetFences(owner, 1, &handle); - flushed = false; + flushed.release(false); + } + + void signal_flushed() + { + flushed.release(true); } void wait_flush() @@ -1254,7 +1259,7 @@ private: is_open = false; } - void submit(VkQueue queue, VkSemaphore wait_semaphore, VkSemaphore signal_semaphore, fence* pfence, VkPipelineStageFlags pipeline_stage_flags) + void submit(VkQueue queue, VkSemaphore wait_semaphore, VkSemaphore signal_semaphore, fence* pfence, VkPipelineStageFlags pipeline_stage_flags, VkBool32 flush = VK_FALSE) { if (is_open) { @@ -1289,7 +1294,7 @@ private: infos.pSignalSemaphores = &signal_semaphore; } - queue_submit(queue, &infos, pfence); + queue_submit(queue, &infos, pfence, flush); clear_flags(); } }; diff --git a/rpcs3/Emu/RSX/VK/VKTextureCache.h b/rpcs3/Emu/RSX/VK/VKTextureCache.h index da9b952383..c3679daa24 100644 --- a/rpcs3/Emu/RSX/VK/VKTextureCache.h +++ b/rpcs3/Emu/RSX/VK/VKTextureCache.h @@ -1337,7 +1337,7 @@ namespace vk { // Primary access command queue, must restart it after vk::fence submit_fence(*m_device); - cmd.submit(m_submit_queue, VK_NULL_HANDLE, VK_NULL_HANDLE, &submit_fence, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT); + cmd.submit(m_submit_queue, VK_NULL_HANDLE, VK_NULL_HANDLE, &submit_fence, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_TRUE); vk::wait_for_fence(&submit_fence, GENERAL_WAIT_TIMEOUT); @@ -1347,7 +1347,7 @@ namespace vk else { // Auxilliary command queue with auto-restart capability - cmd.submit(m_submit_queue, VK_NULL_HANDLE, VK_NULL_HANDLE, VK_NULL_HANDLE, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT); + cmd.submit(m_submit_queue, VK_NULL_HANDLE, VK_NULL_HANDLE, VK_NULL_HANDLE, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_TRUE); } verify(HERE), cmd.flags == 0;