[PATCH vkd3d 4/4] vkd3d: Remove the descriptor mutexes.
Conor McCarthy
cmccarthy at codeweavers.com
Thu May 12 10:11:08 CDT 2022
Descriptor read/write race conditions in SotTR were a result of deficiencies
in the fence implementation.
Signed-off-by: Conor McCarthy <cmccarthy at codeweavers.com>
---
libs/vkd3d/device.c | 50 ++++++++------------
libs/vkd3d/resource.c | 97 ++++++++++++--------------------------
libs/vkd3d/vkd3d_private.h | 17 +------
3 files changed, 51 insertions(+), 113 deletions(-)
diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c
index bdc80ba3..e2533598 100644
--- a/libs/vkd3d/device.c
+++ b/libs/vkd3d/device.c
@@ -2615,7 +2615,6 @@ static ULONG STDMETHODCALLTYPE d3d12_device_Release(ID3D12Device *iface)
{
struct d3d12_device *device = impl_from_ID3D12Device(iface);
ULONG refcount = InterlockedDecrement(&device->refcount);
- size_t i;
TRACE("%p decreasing refcount to %u.\n", device, refcount);
@@ -2635,8 +2634,6 @@ static ULONG STDMETHODCALLTYPE d3d12_device_Release(ID3D12Device *iface)
vkd3d_render_pass_cache_cleanup(&device->render_pass_cache, device);
d3d12_device_destroy_pipeline_cache(device);
d3d12_device_destroy_vkd3d_queues(device);
- for (i = 0; i < ARRAY_SIZE(device->desc_mutex); ++i)
- vkd3d_mutex_destroy(&device->desc_mutex[i]);
VK_CALL(vkDestroyDevice(device->vk_device, NULL));
if (device->parent)
IUnknown_Release(device->parent);
@@ -3457,42 +3454,45 @@ static HRESULT STDMETHODCALLTYPE d3d12_device_CreateRootSignature(ID3D12Device *
static void STDMETHODCALLTYPE d3d12_device_CreateConstantBufferView(ID3D12Device *iface,
const D3D12_CONSTANT_BUFFER_VIEW_DESC *desc, D3D12_CPU_DESCRIPTOR_HANDLE descriptor)
{
+ struct d3d12_desc *dst = d3d12_desc_from_cpu_handle(descriptor);
struct d3d12_device *device = impl_from_ID3D12Device(iface);
- struct d3d12_desc tmp = {0};
TRACE("iface %p, desc %p, descriptor %#lx.\n", iface, desc, descriptor.ptr);
- d3d12_desc_create_cbv(&tmp, device, desc);
- d3d12_desc_write_atomic(d3d12_desc_from_cpu_handle(descriptor), &tmp, device);
+ d3d12_desc_create_cbv(dst, device, desc);
+ if (device->use_vk_heaps && dst->magic)
+ d3d12_desc_write_vk_heap(dst, device);
}
static void STDMETHODCALLTYPE d3d12_device_CreateShaderResourceView(ID3D12Device *iface,
ID3D12Resource *resource, const D3D12_SHADER_RESOURCE_VIEW_DESC *desc,
D3D12_CPU_DESCRIPTOR_HANDLE descriptor)
{
+ struct d3d12_desc *dst = d3d12_desc_from_cpu_handle(descriptor);
struct d3d12_device *device = impl_from_ID3D12Device(iface);
- struct d3d12_desc tmp = {0};
TRACE("iface %p, resource %p, desc %p, descriptor %#lx.\n",
iface, resource, desc, descriptor.ptr);
- d3d12_desc_create_srv(&tmp, device, unsafe_impl_from_ID3D12Resource(resource), desc);
- d3d12_desc_write_atomic(d3d12_desc_from_cpu_handle(descriptor), &tmp, device);
+ d3d12_desc_create_srv(dst, device, unsafe_impl_from_ID3D12Resource(resource), desc);
+ if (device->use_vk_heaps && dst->magic)
+ d3d12_desc_write_vk_heap(dst, device);
}
static void STDMETHODCALLTYPE d3d12_device_CreateUnorderedAccessView(ID3D12Device *iface,
ID3D12Resource *resource, ID3D12Resource *counter_resource,
const D3D12_UNORDERED_ACCESS_VIEW_DESC *desc, D3D12_CPU_DESCRIPTOR_HANDLE descriptor)
{
+ struct d3d12_desc *dst = d3d12_desc_from_cpu_handle(descriptor);
struct d3d12_device *device = impl_from_ID3D12Device(iface);
- struct d3d12_desc tmp = {0};
TRACE("iface %p, resource %p, counter_resource %p, desc %p, descriptor %#lx.\n",
iface, resource, counter_resource, desc, descriptor.ptr);
- d3d12_desc_create_uav(&tmp, device, unsafe_impl_from_ID3D12Resource(resource),
+ d3d12_desc_create_uav(dst, device, unsafe_impl_from_ID3D12Resource(resource),
unsafe_impl_from_ID3D12Resource(counter_resource), desc);
- d3d12_desc_write_atomic(d3d12_desc_from_cpu_handle(descriptor), &tmp, device);
+ if (device->use_vk_heaps && dst->magic)
+ d3d12_desc_write_vk_heap(dst, device);
}
static void STDMETHODCALLTYPE d3d12_device_CreateRenderTargetView(ID3D12Device *iface,
@@ -3520,13 +3520,14 @@ static void STDMETHODCALLTYPE d3d12_device_CreateDepthStencilView(ID3D12Device *
static void STDMETHODCALLTYPE d3d12_device_CreateSampler(ID3D12Device *iface,
const D3D12_SAMPLER_DESC *desc, D3D12_CPU_DESCRIPTOR_HANDLE descriptor)
{
+ struct d3d12_desc *sampler = d3d12_desc_from_cpu_handle(descriptor);
struct d3d12_device *device = impl_from_ID3D12Device(iface);
- struct d3d12_desc tmp = {0};
TRACE("iface %p, desc %p, descriptor %#lx.\n", iface, desc, descriptor.ptr);
- d3d12_desc_create_sampler(&tmp, device, desc);
- d3d12_desc_write_atomic(d3d12_desc_from_cpu_handle(descriptor), &tmp, device);
+ d3d12_desc_create_sampler(sampler, device, desc);
+ if (device->use_vk_heaps && sampler->magic)
+ d3d12_desc_write_vk_heap(sampler, device);
}
static void flush_desc_writes(struct d3d12_desc_copy_location locations[][VKD3D_DESCRIPTOR_WRITE_BUFFER_SIZE],
@@ -3543,23 +3544,16 @@ static void flush_desc_writes(struct d3d12_desc_copy_location locations[][VKD3D_
}
}
-static void d3d12_desc_buffered_copy_atomic(struct d3d12_desc *dst, const struct d3d12_desc *src,
+static void d3d12_desc_buffered_copy(struct d3d12_desc *dst, const struct d3d12_desc *src,
struct d3d12_desc_copy_location locations[][VKD3D_DESCRIPTOR_WRITE_BUFFER_SIZE],
struct d3d12_desc_copy_info *infos, struct d3d12_descriptor_heap *descriptor_heap, struct d3d12_device *device)
{
struct d3d12_desc_copy_location *location;
enum vkd3d_vk_descriptor_set_index set;
- struct vkd3d_mutex *mutex;
-
- mutex = d3d12_device_get_descriptor_mutex(device, src);
- vkd3d_mutex_lock(mutex);
if (src->magic == VKD3D_DESCRIPTOR_MAGIC_FREE)
{
- /* Source must be unlocked first, and therefore can't be used as a null source. */
- static const struct d3d12_desc null = {0};
- vkd3d_mutex_unlock(mutex);
- d3d12_desc_write_atomic(dst, &null, device);
+ d3d12_desc_destroy(dst, device);
return;
}
@@ -3571,8 +3565,6 @@ static void d3d12_desc_buffered_copy_atomic(struct d3d12_desc *dst, const struct
if (location->src.magic & VKD3D_DESCRIPTOR_MAGIC_HAS_VIEW)
vkd3d_view_incref(location->src.u.view_info.view);
- vkd3d_mutex_unlock(mutex);
-
infos[set].uav_counter |= (location->src.magic == VKD3D_DESCRIPTOR_MAGIC_UAV)
&& !!location->src.u.view_info.view->vk_counter_view;
location->dst = dst;
@@ -3637,7 +3629,7 @@ static void d3d12_device_vk_heaps_copy_descriptors(struct d3d12_device *device,
if (dst[dst_idx].magic == src[src_idx].magic && (dst[dst_idx].magic & VKD3D_DESCRIPTOR_MAGIC_HAS_VIEW)
&& dst[dst_idx].u.view_info.written_serial_id == src[src_idx].u.view_info.view->serial_id)
continue;
- d3d12_desc_buffered_copy_atomic(&dst[dst_idx], &src[src_idx], locations, infos, descriptor_heap, device);
+ d3d12_desc_buffered_copy(&dst[dst_idx], &src[src_idx], locations, infos, descriptor_heap, device);
}
if (dst_idx >= dst_range_size)
@@ -4244,7 +4236,6 @@ static HRESULT d3d12_device_init(struct d3d12_device *device,
const struct vkd3d_vk_device_procs *vk_procs;
VkResult vr;
HRESULT hr;
- size_t i;
device->ID3D12Device_iface.lpVtbl = &d3d12_device_vtbl;
device->refcount = 1;
@@ -4296,9 +4287,6 @@ static HRESULT d3d12_device_init(struct d3d12_device *device,
device->blocked_queue_count = 0;
- for (i = 0; i < ARRAY_SIZE(device->desc_mutex); ++i)
- vkd3d_mutex_init(&device->desc_mutex[i]);
-
vkd3d_init_descriptor_pool_sizes(device->vk_pool_sizes, &device->vk_info.descriptor_limits);
if ((device->parent = create_info->parent))
diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c
index 68c28cd1..df633afd 100644
--- a/libs/vkd3d/resource.c
+++ b/libs/vkd3d/resource.c
@@ -2098,9 +2098,13 @@ void vkd3d_view_incref(struct vkd3d_view *view)
InterlockedIncrement(&view->refcount);
}
-static void vkd3d_view_destroy(struct vkd3d_view *view, struct d3d12_device *device)
+static void vkd3d_view_decref_descriptor(struct vkd3d_view *view, struct d3d12_device *device)
{
const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs;
+ ULONG refcount = InterlockedDecrement(&view->refcount);
+
+ if (refcount)
+ return;
TRACE("Destroying view %p.\n", view);
@@ -2127,8 +2131,7 @@ static void vkd3d_view_destroy(struct vkd3d_view *view, struct d3d12_device *dev
void vkd3d_view_decref(struct vkd3d_view *view, struct d3d12_device *device)
{
- if (!InterlockedDecrement(&view->refcount))
- vkd3d_view_destroy(view, device);
+ vkd3d_view_decref_descriptor(view, device);
}
/* TODO: write null descriptors to all applicable sets (invalid behaviour workaround). */
@@ -2220,24 +2223,21 @@ static void d3d12_desc_write_vk_heap_null_descriptor(struct d3d12_descriptor_hea
}
}
-/* dst and src contain the same data unless another thread overwrites dst. The array index is
- * calculated from dst, and src is thread safe. */
-static void d3d12_desc_write_vk_heap(const struct d3d12_desc *dst, const struct d3d12_desc *src,
- struct d3d12_device *device)
+void d3d12_desc_write_vk_heap(const struct d3d12_desc *src, struct d3d12_device *device)
{
struct d3d12_descriptor_heap_vk_set *descriptor_set;
struct d3d12_descriptor_heap *descriptor_heap;
const struct vkd3d_vk_device_procs *vk_procs;
bool is_null = false;
- descriptor_heap = vkd3d_gpu_descriptor_allocator_heap_from_descriptor(&device->gpu_descriptor_allocator, dst);
+ descriptor_heap = vkd3d_gpu_descriptor_allocator_heap_from_descriptor(&device->gpu_descriptor_allocator, src);
descriptor_set = &descriptor_heap->vk_descriptor_sets[vkd3d_vk_descriptor_set_index_from_vk_descriptor_type(
src->vk_descriptor_type)];
vk_procs = &device->vk_procs;
vkd3d_mutex_lock(&descriptor_heap->vk_sets_mutex);
- descriptor_set->vk_descriptor_writes[0].dstArrayElement = dst
+ descriptor_set->vk_descriptor_writes[0].dstArrayElement = src
- (const struct d3d12_desc *)descriptor_heap->descriptors;
descriptor_set->vk_descriptor_writes[0].descriptorCount = 1;
switch (src->vk_descriptor_type)
@@ -2275,7 +2275,7 @@ static void d3d12_desc_write_vk_heap(const struct d3d12_desc *dst, const struct
if (src->magic == VKD3D_DESCRIPTOR_MAGIC_UAV && src->u.view_info.view->vk_counter_view)
{
descriptor_set = &descriptor_heap->vk_descriptor_sets[VKD3D_SET_INDEX_UAV_COUNTER];
- descriptor_set->vk_descriptor_writes[0].dstArrayElement = dst
+ descriptor_set->vk_descriptor_writes[0].dstArrayElement = src
- (const struct d3d12_desc *)descriptor_heap->descriptors;
descriptor_set->vk_descriptor_writes[0].descriptorCount = 1;
descriptor_set->vk_descriptor_writes[0].pTexelBufferView = &src->u.view_info.view->vk_counter_view;
@@ -2285,60 +2285,22 @@ static void d3d12_desc_write_vk_heap(const struct d3d12_desc *dst, const struct
vkd3d_mutex_unlock(&descriptor_heap->vk_sets_mutex);
}
-static void d3d12_desc_write_atomic_d3d12_only(struct d3d12_desc *dst, const struct d3d12_desc *src, struct d3d12_device *device)
-{
- struct vkd3d_view *defunct_view;
- struct vkd3d_mutex *mutex;
-
- mutex = d3d12_device_get_descriptor_mutex(device, dst);
- vkd3d_mutex_lock(mutex);
-
- if (!(dst->magic & VKD3D_DESCRIPTOR_MAGIC_HAS_VIEW) || InterlockedDecrement(&dst->u.view_info.view->refcount))
- {
- *dst = *src;
- vkd3d_mutex_unlock(mutex);
- return;
- }
-
- defunct_view = dst->u.view_info.view;
- *dst = *src;
- vkd3d_mutex_unlock(mutex);
-
- /* Destroy the view after unlocking to reduce wait time. */
- vkd3d_view_destroy(defunct_view, device);
-}
-
-void d3d12_desc_write_atomic(struct d3d12_desc *dst, const struct d3d12_desc *src,
+static void d3d12_desc_copy_d3d12_only(struct d3d12_desc *dst, const struct d3d12_desc *src,
struct d3d12_device *device)
{
- struct vkd3d_view *defunct_view = NULL;
- struct vkd3d_mutex *mutex;
-
- mutex = d3d12_device_get_descriptor_mutex(device, dst);
- vkd3d_mutex_lock(mutex);
+ assert(dst != src);
- /* Nothing to do for VKD3D_DESCRIPTOR_MAGIC_CBV. */
- if ((dst->magic & VKD3D_DESCRIPTOR_MAGIC_HAS_VIEW)
- && !InterlockedDecrement(&dst->u.view_info.view->refcount))
- defunct_view = dst->u.view_info.view;
+ d3d12_desc_destroy(dst, device);
*dst = *src;
-
- vkd3d_mutex_unlock(mutex);
-
- /* Destroy the view after unlocking to reduce wait time. */
- if (defunct_view)
- vkd3d_view_destroy(defunct_view, device);
-
- if (device->use_vk_heaps && dst->magic)
- d3d12_desc_write_vk_heap(dst, src, device);
}
-static void d3d12_desc_destroy(struct d3d12_desc *descriptor, struct d3d12_device *device)
+void d3d12_desc_destroy(struct d3d12_desc *descriptor, struct d3d12_device *device)
{
- static const struct d3d12_desc null_desc = {0};
+ if (descriptor->magic & VKD3D_DESCRIPTOR_MAGIC_HAS_VIEW)
+ vkd3d_view_decref_descriptor(descriptor->u.view_info.view, device);
- d3d12_desc_write_atomic(descriptor, &null_desc, device);
+ memset(descriptor, 0, sizeof(*descriptor));
}
void d3d12_desc_copy_vk_heap_range(struct d3d12_desc_copy_location *locations, const struct d3d12_desc_copy_info *info,
@@ -2353,7 +2315,7 @@ void d3d12_desc_copy_vk_heap_range(struct d3d12_desc_copy_location *locations, c
for (i = 0, write_count = 0; i < info->count; ++i)
{
- d3d12_desc_write_atomic_d3d12_only(locations[i].dst, &locations[i].src, device);
+ d3d12_desc_copy_d3d12_only(locations[i].dst, &locations[i].src, device);
if (i && locations[i].dst == locations[i - 1].dst + 1)
{
@@ -2394,24 +2356,17 @@ done:
void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src,
struct d3d12_device *device)
{
- struct d3d12_desc tmp;
- struct vkd3d_mutex *mutex;
-
assert(dst != src);
- /* Shadow of the Tomb Raider and possibly other titles sometimes destroy
- * and rewrite a descriptor in another thread while it is being copied. */
- mutex = d3d12_device_get_descriptor_mutex(device, src);
- vkd3d_mutex_lock(mutex);
+ d3d12_desc_destroy(dst, device);
if (src->magic & VKD3D_DESCRIPTOR_MAGIC_HAS_VIEW)
vkd3d_view_incref(src->u.view_info.view);
- tmp = *src;
-
- vkd3d_mutex_unlock(mutex);
+ *dst = *src;
- d3d12_desc_write_atomic(dst, &tmp, device);
+ if (device->use_vk_heaps && dst->magic)
+ d3d12_desc_write_vk_heap(dst, device);
}
static VkDeviceSize vkd3d_get_required_texel_buffer_alignment(const struct d3d12_device *device,
@@ -2813,6 +2768,8 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor,
struct VkDescriptorBufferInfo *buffer_info;
struct d3d12_resource *resource;
+ d3d12_desc_destroy(descriptor, device);
+
if (!desc)
{
WARN("Constant buffer desc is NULL.\n");
@@ -2988,6 +2945,8 @@ void d3d12_desc_create_srv(struct d3d12_desc *descriptor,
struct vkd3d_texture_view_desc vkd3d_desc;
struct vkd3d_view *view;
+ d3d12_desc_destroy(descriptor, device);
+
if (!resource)
{
vkd3d_create_null_srv(descriptor, device, desc);
@@ -3286,6 +3245,8 @@ void d3d12_desc_create_uav(struct d3d12_desc *descriptor, struct d3d12_device *d
struct d3d12_resource *resource, struct d3d12_resource *counter_resource,
const D3D12_UNORDERED_ACCESS_VIEW_DESC *desc)
{
+ d3d12_desc_destroy(descriptor, device);
+
if (!resource)
{
if (counter_resource)
@@ -3416,6 +3377,8 @@ void d3d12_desc_create_sampler(struct d3d12_desc *sampler,
{
struct vkd3d_view *view;
+ d3d12_desc_destroy(sampler, device);
+
if (!desc)
{
WARN("NULL sampler desc.\n");
diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h
index 37bac159..e2ab8eac 100644
--- a/libs/vkd3d/vkd3d_private.h
+++ b/libs/vkd3d/vkd3d_private.h
@@ -746,7 +746,8 @@ void d3d12_desc_create_uav(struct d3d12_desc *descriptor, struct d3d12_device *d
struct d3d12_resource *resource, struct d3d12_resource *counter_resource,
const D3D12_UNORDERED_ACCESS_VIEW_DESC *desc);
void d3d12_desc_create_sampler(struct d3d12_desc *sampler, struct d3d12_device *device, const D3D12_SAMPLER_DESC *desc);
-void d3d12_desc_write_atomic(struct d3d12_desc *dst, const struct d3d12_desc *src, struct d3d12_device *device);
+void d3d12_desc_write_vk_heap(const struct d3d12_desc *src, struct d3d12_device *device);
+void d3d12_desc_destroy(struct d3d12_desc *descriptor, struct d3d12_device *device);
bool vkd3d_create_raw_buffer_view(struct d3d12_device *device,
D3D12_GPU_VIRTUAL_ADDRESS gpu_address, VkBufferView *vk_buffer_view);
@@ -1468,7 +1469,6 @@ struct d3d12_device
struct vkd3d_gpu_va_allocator gpu_va_allocator;
struct vkd3d_mutex mutex;
- struct vkd3d_mutex desc_mutex[8];
struct vkd3d_render_pass_cache render_pass_cache;
VkPipelineCache vk_pipeline_cache;
@@ -1544,19 +1544,6 @@ static inline unsigned int d3d12_device_get_descriptor_handle_increment_size(str
return ID3D12Device_GetDescriptorHandleIncrementSize(&device->ID3D12Device_iface, descriptor_type);
}
-static inline struct vkd3d_mutex *d3d12_device_get_descriptor_mutex(struct d3d12_device *device,
- const struct d3d12_desc *descriptor)
-{
- STATIC_ASSERT(!(ARRAY_SIZE(device->desc_mutex) & (ARRAY_SIZE(device->desc_mutex) - 1)));
- uintptr_t idx = (uintptr_t)descriptor;
-
- idx ^= idx >> 12;
- idx ^= idx >> 6;
- idx ^= idx >> 3;
-
- return &device->desc_mutex[idx & (ARRAY_SIZE(device->desc_mutex) - 1)];
-}
-
/* utils */
enum vkd3d_format_type
{
--
2.35.1
More information about the wine-devel
mailing list