From e5121198e395eee539d2f0d9c42e0b272972e0a9 Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 7 Jul 2021 13:48:58 +0100 Subject: [PATCH] Prevent deadlock in vulkan recursively acquiring RW lock --- renderdoc/driver/vulkan/vk_core.h | 7 +- .../driver/vulkan/wrappers/vk_queue_funcs.cpp | 4 +- .../vulkan/wrappers/vk_resource_funcs.cpp | 119 ++++++++---------- 3 files changed, 60 insertions(+), 70 deletions(-) diff --git a/renderdoc/driver/vulkan/vk_core.h b/renderdoc/driver/vulkan/vk_core.h index 6e8607291..230c367c6 100644 --- a/renderdoc/driver/vulkan/vk_core.h +++ b/renderdoc/driver/vulkan/vk_core.h @@ -715,10 +715,6 @@ private: // All IDs are original IDs, not live. VulkanRenderState m_RenderState; - // an internal structure used to differentiate forced vkFlushMappedMemoryRanges due to coherent - // map writes from real calls to vkFlushMappedMemoryRanges - VkBaseInStructure internalMemoryFlushMarker = {}; - bool InRerecordRange(ResourceId cmdid); bool HasRerecordCmdBuf(ResourceId cmdid); bool ShouldUpdateRenderState(ResourceId cmdid, bool forcePrimary = false); @@ -932,6 +928,9 @@ private: void CaptureQueueSubmit(VkQueue queue, const rdcarray &commandBuffers, VkFence fence); void ReplayQueueSubmit(VkQueue queue, VkSubmitInfo2KHR submitInfo, rdcstr basename); + void InternalFlushMemoryRange(VkDevice device, const VkMappedMemoryRange &memRange, + bool internalFlush, bool capframe); + void DoSubmit(VkQueue queue, VkSubmitInfo2KHR submitInfo); rdcarray m_ActionStack; diff --git a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp index 4721b454f..552d5146c 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp @@ -1128,12 +1128,12 @@ void WrappedVulkan::CaptureQueueSubmit(VkQueue queue, ToStr(record->GetResourceID()).c_str(), (uint64_t)diffStart, (uint64_t)diffEnd); VkMappedMemoryRange range = { VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE, - &internalMemoryFlushMarker, + NULL, (VkDeviceMemory)(uint64_t)record->Resource, state.mapOffset + diffStart, diffEnd - diffStart, }; - vkFlushMappedMemoryRanges(dev, 1, &range); + InternalFlushMemoryRange(dev, range, true, capframe); } } else diff --git a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp index 6655fad35..52ecca904 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp @@ -1152,6 +1152,57 @@ bool WrappedVulkan::Serialise_vkFlushMappedMemoryRanges(SerialiserType &ser, VkD return true; } +void WrappedVulkan::InternalFlushMemoryRange(VkDevice device, const VkMappedMemoryRange &memRange, + bool internalFlush, bool capframe) +{ + ResourceId memid = GetResID(memRange.memory); + VkResourceRecord *record = GetRecord(memRange.memory); + + MemMapState *state = record->memMapState; + + if(!internalFlush) + state->mapFlushed = true; + + if(state->mappedPtr == NULL) + { + RDCERR("Flushing memory %s that isn't currently mapped", ToStr(memid).c_str()); + return; + } + + if(capframe) + { + SCOPED_LOCK_OPTIONAL(state->mrLock, !internalFlush); + + CACHE_THREAD_SERIALISER(); + + SCOPED_SERIALISE_CHUNK(internalFlush ? VulkanChunk::CoherentMapWrite + : VulkanChunk::vkFlushMappedMemoryRanges); + Serialise_vkFlushMappedMemoryRanges(ser, device, 1, &memRange); + + m_FrameCaptureRecord->AddChunk(scope.Get()); + } + + if(capframe) + { + VkDeviceSize offs = memRange.offset; + VkDeviceSize size = memRange.size; + + // map VK_WHOLE_SIZE into a specific size + if(size == VK_WHOLE_SIZE) + size = state->mapOffset + state->mapSize - offs; + + GetResourceManager()->MarkMemoryFrameReferenced(memid, offs, size, eFrameRef_CompleteWrite); + } + else + { + FrameRefType refType = eFrameRef_PartialWrite; + if(memRange.offset == 0 && memRange.size >= record->Length) + refType = eFrameRef_CompleteWrite; + + GetResourceManager()->MarkResourceFrameReferenced(memid, refType); + } +} + VkResult WrappedVulkan::vkFlushMappedMemoryRanges(VkDevice device, uint32_t memRangeCount, const VkMappedMemoryRange *pMemRanges) { @@ -1163,28 +1214,13 @@ VkResult WrappedVulkan::vkFlushMappedMemoryRanges(VkDevice device, uint32_t memR } VkResult ret; - bool internalFlush = false; - - // don't call the driver for a fake coherent map flush - if(memRangeCount == 1 && pMemRanges->pNext == &internalMemoryFlushMarker) - { - ret = VK_SUCCESS; - internalFlush = true; - - // for simplicity we set the pNext to NULL after we've seen our internal marker. We can safely - // modify this because it's a temporary variable in the calling stackframe - VkMappedMemoryRange *range = (VkMappedMemoryRange *)pMemRanges; - range->pNext = NULL; - } - else - { - SERIALISE_TIME_CALL( - ret = ObjDisp(device)->FlushMappedMemoryRanges(Unwrap(device), memRangeCount, unwrapped)); - } + SERIALISE_TIME_CALL( + ret = ObjDisp(device)->FlushMappedMemoryRanges(Unwrap(device), memRangeCount, unwrapped)); if(IsCaptureMode(m_State)) { bool capframe = false; + { SCOPED_READLOCK(m_CapTransitionLock); capframe = IsActiveCapturing(m_State); @@ -1192,52 +1228,7 @@ VkResult WrappedVulkan::vkFlushMappedMemoryRanges(VkDevice device, uint32_t memR for(uint32_t i = 0; i < memRangeCount; i++) { - ResourceId memid = GetResID(pMemRanges[i].memory); - VkResourceRecord *record = GetRecord(pMemRanges[i].memory); - - MemMapState *state = record->memMapState; - - if(!internalFlush) - state->mapFlushed = true; - - if(state->mappedPtr == NULL) - { - RDCERR("Flushing memory %s that isn't currently mapped", ToStr(memid).c_str()); - continue; - } - - if(capframe) - { - SCOPED_LOCK_OPTIONAL(state->mrLock, !internalFlush); - - CACHE_THREAD_SERIALISER(); - - SCOPED_SERIALISE_CHUNK(internalFlush ? VulkanChunk::CoherentMapWrite - : VulkanChunk::vkFlushMappedMemoryRanges); - Serialise_vkFlushMappedMemoryRanges(ser, device, 1, pMemRanges + i); - - m_FrameCaptureRecord->AddChunk(scope.Get()); - } - - if(capframe) - { - VkDeviceSize offs = pMemRanges[i].offset; - VkDeviceSize size = pMemRanges[i].size; - - // map VK_WHOLE_SIZE into a specific size - if(size == VK_WHOLE_SIZE) - size = state->mapOffset + state->mapSize - offs; - - GetResourceManager()->MarkMemoryFrameReferenced(memid, offs, size, eFrameRef_CompleteWrite); - } - else - { - FrameRefType refType = eFrameRef_PartialWrite; - if(pMemRanges[i].offset == 0 && pMemRanges[i].size >= record->Length) - refType = eFrameRef_CompleteWrite; - - GetResourceManager()->MarkResourceFrameReferenced(memid, refType); - } + InternalFlushMemoryRange(device, pMemRanges[i], false, capframe); } }