[PATCH] vkd3d: Allow the source to be concurrently rewritten during descriptor copy.

Conor McCarthy cmccarthy at codeweavers.com
Fri Sep 20 06:27:31 CDT 2019


While copying a descriptor, the source can be rewritten with a new descriptor
by another thread without causing invalid behavior in Windows. In vkd3d this
can result in transparent freeing of a vkd3d_view object in the source
descriptor, so the copy gets a pointer to a deallocated object. Acquisition of
a reference to a view object must be protected with a mutex. This enables
SotTR to run, and causes little or no impact to performance in Metro Exodus,
which doesn't require it.

Signed-off-by: Conor McCarthy <cmccarthy at codeweavers.com>
---
 libs/vkd3d/device.c        |  3 +++
 libs/vkd3d/resource.c      | 43 ++++++++++++++++++++++++++++++++++----
 libs/vkd3d/vkd3d_private.h |  1 +
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c
index dc0fa0a..f76ae30 100644
--- a/libs/vkd3d/device.c
+++ b/libs/vkd3d/device.c
@@ -2006,6 +2006,7 @@ static ULONG STDMETHODCALLTYPE d3d12_device_Release(ID3D12Device *iface)
         vkd3d_fence_worker_stop(&device->fence_worker, device);
         d3d12_device_destroy_pipeline_cache(device);
         d3d12_device_destroy_vkd3d_queues(device);
+        pthread_mutex_destroy(&device->desc_mutex);
         VK_CALL(vkDestroyDevice(device->vk_device, NULL));
         if (device->parent)
             IUnknown_Release(device->parent);
@@ -3237,6 +3238,8 @@ static HRESULT d3d12_device_init(struct d3d12_device *device,
     vkd3d_render_pass_cache_init(&device->render_pass_cache);
     vkd3d_gpu_va_allocator_init(&device->gpu_va_allocator);
 
+    pthread_mutex_init(&device->desc_mutex, NULL);
+
     if ((device->parent = create_info->parent))
         IUnknown_AddRef(device->parent);
 
diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c
index 463f373..28f047d 100644
--- a/libs/vkd3d/resource.c
+++ b/libs/vkd3d/resource.c
@@ -1938,10 +1938,27 @@ static void d3d12_desc_destroy(struct d3d12_desc *descriptor,
             || descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV
             || descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_SAMPLER)
     {
-        vkd3d_view_decref_descriptor(descriptor->u.view, descriptor, device);
+        /* Shadow of the Tomb Raider and possibly other titles sometimes destroy
+         * and rewrite a descriptor in another thread while it is being copied. */
+        pthread_mutex_lock(&device->desc_mutex);
+        if (!descriptor->u.view)
+        {
+            /* Destroyed in another thread. */
+            pthread_mutex_unlock(&device->desc_mutex);
+        }
+        else
+        {
+            vkd3d_view_decref_descriptor(descriptor->u.view, descriptor, device);
+            /* Another thread may free the view after the mutex is unlocked, so zero the
+             * descriptor while locked. This has no measurable effect on performance. */
+            memset(descriptor, 0, sizeof(*descriptor));
+            pthread_mutex_unlock(&device->desc_mutex);
+        }
+    }
+    else
+    {
+        memset(descriptor, 0, sizeof(*descriptor));
     }
-
-    memset(descriptor, 0, sizeof(*descriptor));
 }
 
 void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src,
@@ -1951,13 +1968,31 @@ void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src,
 
     d3d12_desc_destroy(dst, device);
 
+    /* Try to do as much as possible while unlocked to allow multithreading. */
     *dst = *src;
 
     if (src->magic == VKD3D_DESCRIPTOR_MAGIC_SRV
             || src->magic == VKD3D_DESCRIPTOR_MAGIC_UAV
             || src->magic == VKD3D_DESCRIPTOR_MAGIC_SAMPLER)
     {
-        vkd3d_view_incref(src->u.view);
+        pthread_mutex_lock(&device->desc_mutex);
+        if (src->u.view == dst->u.view)
+        {
+            /* Hopefully this happens most of the time. */
+            vkd3d_view_incref(src->u.view);
+            pthread_mutex_unlock(&device->desc_mutex);
+        }
+        else
+        {
+            /* The source was changed while we waited for the lock. Get a copy of the new descriptor.
+             * (See comments in d3d12_desc_destroy()). */
+            *dst = *src;
+            if (src->magic == VKD3D_DESCRIPTOR_MAGIC_SRV
+                    || src->magic == VKD3D_DESCRIPTOR_MAGIC_UAV
+                    || src->magic == VKD3D_DESCRIPTOR_MAGIC_SAMPLER)
+                vkd3d_view_incref(src->u.view);
+            pthread_mutex_unlock(&device->desc_mutex);
+        }
     }
 }
 
diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h
index 294e677..0cfd1af 100644
--- a/libs/vkd3d/vkd3d_private.h
+++ b/libs/vkd3d/vkd3d_private.h
@@ -1052,6 +1052,7 @@ struct d3d12_device
     struct vkd3d_fence_worker fence_worker;
 
     pthread_mutex_t mutex;
+    pthread_mutex_t desc_mutex;
     struct vkd3d_render_pass_cache render_pass_cache;
     VkPipelineCache vk_pipeline_cache;
 
-- 
2.23.0




More information about the wine-devel mailing list