[PATCH v2 1/2] vkd3d: On apple, implement pthread_spinlock_t with os_unfair_lock.

newer
[PATCH v3 2/2] msscript: Partially...

older
[PATCH] winmm: Avoid a couple of...

Derek Lesho

Sept. 12, 2019

3:24 p.m.

Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- include/private/vkd3d_common.h | 36 ++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/include/private/vkd3d_common.h b/include/private/vkd3d_common.h index 1ac8a63..c6a0946 100644 --- a/include/private/vkd3d_common.h +++ b/include/private/vkd3d_common.h @@ -168,4 +168,40 @@ static inline void vkd3d_parse_version(const char *version, int *major, int *min *minor = atoi(version); } +#ifdef __APPLE__ +#include <os/lock.h> + +typedef os_unfair_lock pthread_spinlock_t; + +inline int pthread_spin_init(pthread_spinlock_t *lock, int pshared) +{ + if (pshared != PTHREAD_PROCESS_PRIVATE) return E_INVALIDARG; + *lock = OS_UNFAIR_LOCK_INIT; + return 0; +} + +inline int pthread_spin_lock(pthread_spinlock_t *lock) +{ + os_unfair_lock_lock(lock); + return 0; +} + +inline int pthread_spin_trylock(pthread_spinlock_t *lock) +{ + if (!(os_unfair_lock_trylock(lock))) return EBUSY; + return 0; +} + +inline int pthread_spin_unlock(pthread_spinlock_t *lock) +{ + os_unfair_lock_unlock(lock); + return 0; +} + +inline int pthread_spin_destroy(pthread_spinlock_t *lock) +{ + return 0; +} +#endif + #endif /* __VKD3D_COMMON_H */ -- 2.23.0

Show replies by date

Derek Lesho

September 2019

3:25 p.m.

New subject: [PATCH v2 2/2] vkd3d: Don't allow concurrent writes to descriptors.

In D3D12 it is legal to concurrently overwrite descriptors, we can't handle that. This patch locks descriptors from being overwritten by two threads at the same time. Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v2: - Only lock when overwriting a descriptor, not when creating the vulkan view. - Add locking for depth-stencil views, even though SOTTR doesn't require it. --- libs/vkd3d/resource.c | 203 ++++++++++++++++++++++++++----------- libs/vkd3d/vkd3d_private.h | 3 + 2 files changed, 146 insertions(+), 60 deletions(-) diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index 463f373..848fb4d 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -1947,8 +1947,13 @@ static void d3d12_desc_destroy(struct d3d12_desc *descriptor, void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src, struct d3d12_device *device) { + pthread_spinlock_t *descriptor_lock; + assert(dst != src); + pthread_spin_lock(dst->lock); + descriptor_lock = dst->lock; + d3d12_desc_destroy(dst, device); *dst = *src; @@ -1959,6 +1964,8 @@ void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src, { vkd3d_view_incref(src->u.view); } + + pthread_spin_unlock(descriptor_lock); } static VkDeviceSize vkd3d_get_required_texel_buffer_alignment(const struct d3d12_device *device, @@ -2322,22 +2329,22 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor, { struct VkDescriptorBufferInfo *buffer_info; struct d3d12_resource *resource; - - d3d12_desc_destroy(descriptor, device); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!desc) { WARN("Constant buffer desc is NULL.\n"); - return; + goto done; } if (desc->SizeInBytes & (D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT - 1)) { WARN("Size is not %u bytes aligned.\n", D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT); - return; + goto done; } - buffer_info = &descriptor->u.vk_cbv_info; + buffer_info = &new_desc.u.vk_cbv_info; if (desc->BufferLocation) { resource = vkd3d_gpu_va_allocator_dereference(&device->gpu_va_allocator, desc->BufferLocation); @@ -2353,8 +2360,18 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor, buffer_info->range = VKD3D_NULL_BUFFER_SIZE; } - descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_CBV; - descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + new_desc.magic = VKD3D_DESCRIPTOR_MAGIC_CBV; + new_desc.vk_descriptor_type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } static unsigned int vkd3d_view_flags_from_d3d12_buffer_srv_flags(D3D12_BUFFER_SRV_FLAGS flags) @@ -2466,23 +2483,23 @@ 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); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { - vkd3d_create_null_srv(descriptor, device, desc); - return; + vkd3d_create_null_srv(&new_desc, device, desc); + goto done; } if (d3d12_resource_is_buffer(resource)) { - vkd3d_create_buffer_srv(descriptor, device, resource, desc); - return; + vkd3d_create_buffer_srv(&new_desc, device, resource, desc); + goto done; } if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done; vkd3d_desc.miplevel_count = VK_REMAINING_MIP_LEVELS; vkd3d_desc.allowed_swizzle = true; @@ -2559,11 +2576,21 @@ void d3d12_desc_create_srv(struct d3d12_desc *descriptor, } if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done; - descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_SRV; - descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; - descriptor->u.view = view; + new_desc.magic = VKD3D_DESCRIPTOR_MAGIC_SRV; + new_desc.vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; + new_desc.u.view = view; + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } static unsigned int vkd3d_view_flags_from_d3d12_buffer_uav_flags(D3D12_BUFFER_UAV_FLAGS flags) @@ -2761,26 +2788,37 @@ 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); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { if (counter_resource) FIXME("Ignoring counter resource %p.\n", counter_resource); - vkd3d_create_null_uav(descriptor, device, desc); - return; + vkd3d_create_null_uav(&new_desc, device, desc); + goto done; } if (d3d12_resource_is_buffer(resource)) { - vkd3d_create_buffer_uav(descriptor, device, resource, counter_resource, desc); + vkd3d_create_buffer_uav(&new_desc, device, resource, counter_resource, desc); } else { if (counter_resource) FIXME("Unexpected counter resource for texture view.\n"); - vkd3d_create_texture_uav(descriptor, device, resource, desc); + vkd3d_create_texture_uav(&new_desc, device, resource, desc); } + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } bool vkd3d_create_raw_buffer_view(struct d3d12_device *device, @@ -2888,13 +2926,13 @@ void d3d12_desc_create_sampler(struct d3d12_desc *sampler, struct d3d12_device *device, const D3D12_SAMPLER_DESC *desc) { struct vkd3d_view *view; - - d3d12_desc_destroy(sampler, device); + struct d3d12_desc new_sampler; + pthread_spinlock_t *descriptor_lock; if (!desc) { WARN("NULL sampler desc.\n"); - return; + goto done; } if (desc->AddressU == D3D12_TEXTURE_ADDRESS_MODE_BORDER @@ -2904,19 +2942,29 @@ void d3d12_desc_create_sampler(struct d3d12_desc *sampler, desc->BorderColor[0], desc->BorderColor[1], desc->BorderColor[2], desc->BorderColor[3]); if (!(view = vkd3d_view_create())) - return; + goto done; if (d3d12_create_sampler(device, desc->Filter, desc->AddressU, desc->AddressV, desc->AddressW, desc->MipLODBias, desc->MaxAnisotropy, desc->ComparisonFunc, desc->MinLOD, desc->MaxLOD, &view->u.vk_sampler) < 0) { vkd3d_free(view); - return; + goto done; } - sampler->magic = VKD3D_DESCRIPTOR_MAGIC_SAMPLER; - sampler->vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLER; - sampler->u.view = view; + new_sampler.magic = VKD3D_DESCRIPTOR_MAGIC_SAMPLER; + new_sampler.vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLER; + new_sampler.u.view = view; + + done: + pthread_spin_lock(sampler->lock); + descriptor_lock = sampler->lock; + + d3d12_desc_destroy(sampler, device); + *sampler = new_sampler; + + sampler->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } HRESULT vkd3d_create_static_sampler(struct d3d12_device *device, @@ -2950,22 +2998,22 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev { struct vkd3d_texture_view_desc vkd3d_desc; struct vkd3d_view *view; - - d3d12_rtv_desc_destroy(rtv_desc, device); + struct d3d12_rtv_desc new_rtv_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { FIXME("NULL resource RTV not implemented.\n"); - return; + goto done; } if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done; if (vkd3d_desc.format->vk_aspect_mask != VK_IMAGE_ASPECT_COLOR_BIT) { WARN("Trying to create RTV for depth/stencil format %#x.\n", vkd3d_desc.format->dxgi_format); - return; + goto done; } if (desc) @@ -3013,16 +3061,26 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev assert(d3d12_resource_is_texture(resource)); if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done; + + new_rtv_desc.magic = VKD3D_DESCRIPTOR_MAGIC_RTV; + new_rtv_desc.sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); + new_rtv_desc.format = vkd3d_desc.format; + new_rtv_desc.width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); + new_rtv_desc.height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); + new_rtv_desc.layer_count = vkd3d_desc.layer_count; + new_rtv_desc.view = view; + new_rtv_desc.resource = resource; + + done: + pthread_spin_lock(rtv_desc->lock); + descriptor_lock = rtv_desc->lock; + + d3d12_rtv_desc_destroy(rtv_desc, device); + *rtv_desc = new_rtv_desc; - rtv_desc->magic = VKD3D_DESCRIPTOR_MAGIC_RTV; - rtv_desc->sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); - rtv_desc->format = vkd3d_desc.format; - rtv_desc->width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); - rtv_desc->height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); - rtv_desc->layer_count = vkd3d_desc.layer_count; - rtv_desc->view = view; - rtv_desc->resource = resource; + rtv_desc->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } /* DSVs */ @@ -3040,28 +3098,28 @@ void d3d12_dsv_desc_create_dsv(struct d3d12_dsv_desc *dsv_desc, struct d3d12_dev { struct vkd3d_texture_view_desc vkd3d_desc; struct vkd3d_view *view; - - d3d12_dsv_desc_destroy(dsv_desc, device); + struct d3d12_dsv_desc new_dsv_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { FIXME("NULL resource DSV not implemented.\n"); - return; + goto done; } if (resource->desc.Dimension == D3D12_RESOURCE_DIMENSION_TEXTURE3D) { WARN("Cannot create DSV for 3D texture.\n"); - return; + goto done; } if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done; if (!(vkd3d_desc.format->vk_aspect_mask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT))) { WARN("Trying to create DSV for format %#x.\n", vkd3d_desc.format->dxgi_format); - return; + goto done; } if (desc) @@ -3096,16 +3154,26 @@ void d3d12_dsv_desc_create_dsv(struct d3d12_dsv_desc *dsv_desc, struct d3d12_dev assert(d3d12_resource_is_texture(resource)); if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done; + + new_dsv_desc.magic = VKD3D_DESCRIPTOR_MAGIC_DSV; + new_dsv_desc.sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); + new_dsv_desc.format = vkd3d_desc.format; + new_dsv_desc.width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); + new_dsv_desc.height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); + new_dsv_desc.layer_count = vkd3d_desc.layer_count; + new_dsv_desc.view = view; + new_dsv_desc.resource = resource; + + done: + pthread_spin_lock(dsv_desc->lock); + descriptor_lock = dsv_desc->lock; + + d3d12_dsv_desc_destroy(dsv_desc, device); + *dsv_desc = new_dsv_desc; - dsv_desc->magic = VKD3D_DESCRIPTOR_MAGIC_DSV; - dsv_desc->sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); - dsv_desc->format = vkd3d_desc.format; - dsv_desc->width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); - dsv_desc->height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); - dsv_desc->layer_count = vkd3d_desc.layer_count; - dsv_desc->view = view; - dsv_desc->resource = resource; + dsv_desc->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } /* ID3D12DescriptorHeap */ @@ -3170,6 +3238,8 @@ static ULONG STDMETHODCALLTYPE d3d12_descriptor_heap_Release(ID3D12DescriptorHea for (i = 0; i < heap->desc.NumDescriptors; ++i) { d3d12_desc_destroy(&descriptors[i], device); + pthread_spin_destroy(descriptors[i].lock); + vkd3d_free((void*)descriptors[i].lock); } break; } @@ -3181,6 +3251,8 @@ static ULONG STDMETHODCALLTYPE d3d12_descriptor_heap_Release(ID3D12DescriptorHea for (i = 0; i < heap->desc.NumDescriptors; ++i) { d3d12_rtv_desc_destroy(&rtvs[i], device); + pthread_spin_destroy(rtvs[i].lock); + vkd3d_free((void*)rtvs[i].lock); } break; } @@ -3192,6 +3264,8 @@ static ULONG STDMETHODCALLTYPE d3d12_descriptor_heap_Release(ID3D12DescriptorHea for (i = 0; i < heap->desc.NumDescriptors; ++i) { d3d12_dsv_desc_destroy(&dsvs[i], device); + pthread_spin_destroy(dsvs[i].lock); + vkd3d_free((void*)dsvs[i].lock); } break; } @@ -3367,6 +3441,15 @@ HRESULT d3d12_descriptor_heap_create(struct d3d12_device *device, memset(object->descriptors, 0, descriptor_size * desc->NumDescriptors); + for (unsigned int i = 0; i < desc->NumDescriptors; i++) + { + struct d3d12_desc *cur_desc = (struct d3d12_desc *) (object->descriptors + (i * descriptor_size)); + pthread_spinlock_t *lock = vkd3d_malloc(sizeof(pthread_spinlock_t)); + + pthread_spin_init(lock, PTHREAD_PROCESS_PRIVATE); + cur_desc->lock = lock; + } + TRACE("Created descriptor heap %p.\n", object); *descriptor_heap = object; diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index bd9670e..6f1a957 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -466,6 +466,7 @@ void vkd3d_view_incref(struct vkd3d_view *view) DECLSPEC_HIDDEN; struct d3d12_desc { uint32_t magic; + pthread_spinlock_t *lock; VkDescriptorType vk_descriptor_type; union { @@ -521,6 +522,7 @@ HRESULT vkd3d_create_static_sampler(struct d3d12_device *device, struct d3d12_rtv_desc { uint32_t magic; + pthread_spinlock_t *lock; VkSampleCountFlagBits sample_count; const struct vkd3d_format *format; uint64_t width; @@ -541,6 +543,7 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev struct d3d12_dsv_desc { uint32_t magic; + pthread_spinlock_t *lock; VkSampleCountFlagBits sample_count; const struct vkd3d_format *format; uint64_t width; -- 2.23.0

Derek Lesho

2:19 p.m.

New subject: [PATCH v4 2/2] vkd3d: Don't allow concurrent writes to descriptors.

In D3D12 it is legal to concurrently overwrite descriptors, we can't handle that. This patch locks descriptors from being overwritten by two threads at the same time. Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v4: Allocate the descriptor locks for a heap in a single array to avoid unecessary allocations. --- libs/vkd3d/resource.c | 207 ++++++++++++++++++++++++++----------- libs/vkd3d/vkd3d_private.h | 5 + 2 files changed, 152 insertions(+), 60 deletions(-) diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index 463f373..e6d3b51 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -1947,8 +1947,13 @@ static void d3d12_desc_destroy(struct d3d12_desc *descriptor, void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src, struct d3d12_device *device) { + pthread_spinlock_t *descriptor_lock; + assert(dst != src); + pthread_spin_lock(dst->lock); + descriptor_lock = dst->lock; + d3d12_desc_destroy(dst, device); *dst = *src; @@ -1959,6 +1964,8 @@ void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src, { vkd3d_view_incref(src->u.view); } + + pthread_spin_unlock(descriptor_lock); } static VkDeviceSize vkd3d_get_required_texel_buffer_alignment(const struct d3d12_device *device, @@ -2322,22 +2329,22 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor, { struct VkDescriptorBufferInfo *buffer_info; struct d3d12_resource *resource; - - d3d12_desc_destroy(descriptor, device); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!desc) { WARN("Constant buffer desc is NULL.\n"); - return; + goto done; } if (desc->SizeInBytes & (D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT - 1)) { WARN("Size is not %u bytes aligned.\n", D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT); - return; + goto done; } - buffer_info = &descriptor->u.vk_cbv_info; + buffer_info = &new_desc.u.vk_cbv_info; if (desc->BufferLocation) { resource = vkd3d_gpu_va_allocator_dereference(&device->gpu_va_allocator, desc->BufferLocation); @@ -2353,8 +2360,18 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor, buffer_info->range = VKD3D_NULL_BUFFER_SIZE; } - descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_CBV; - descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + new_desc.magic = VKD3D_DESCRIPTOR_MAGIC_CBV; + new_desc.vk_descriptor_type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } static unsigned int vkd3d_view_flags_from_d3d12_buffer_srv_flags(D3D12_BUFFER_SRV_FLAGS flags) @@ -2466,23 +2483,23 @@ 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); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { - vkd3d_create_null_srv(descriptor, device, desc); - return; + vkd3d_create_null_srv(&new_desc, device, desc); + goto done; } if (d3d12_resource_is_buffer(resource)) { - vkd3d_create_buffer_srv(descriptor, device, resource, desc); - return; + vkd3d_create_buffer_srv(&new_desc, device, resource, desc); + goto done; } if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done; vkd3d_desc.miplevel_count = VK_REMAINING_MIP_LEVELS; vkd3d_desc.allowed_swizzle = true; @@ -2559,11 +2576,21 @@ void d3d12_desc_create_srv(struct d3d12_desc *descriptor, } if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done; - descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_SRV; - descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; - descriptor->u.view = view; + new_desc.magic = VKD3D_DESCRIPTOR_MAGIC_SRV; + new_desc.vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; + new_desc.u.view = view; + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } static unsigned int vkd3d_view_flags_from_d3d12_buffer_uav_flags(D3D12_BUFFER_UAV_FLAGS flags) @@ -2761,26 +2788,37 @@ 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); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { if (counter_resource) FIXME("Ignoring counter resource %p.\n", counter_resource); - vkd3d_create_null_uav(descriptor, device, desc); - return; + vkd3d_create_null_uav(&new_desc, device, desc); + goto done; } if (d3d12_resource_is_buffer(resource)) { - vkd3d_create_buffer_uav(descriptor, device, resource, counter_resource, desc); + vkd3d_create_buffer_uav(&new_desc, device, resource, counter_resource, desc); } else { if (counter_resource) FIXME("Unexpected counter resource for texture view.\n"); - vkd3d_create_texture_uav(descriptor, device, resource, desc); + vkd3d_create_texture_uav(&new_desc, device, resource, desc); } + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } bool vkd3d_create_raw_buffer_view(struct d3d12_device *device, @@ -2888,13 +2926,13 @@ void d3d12_desc_create_sampler(struct d3d12_desc *sampler, struct d3d12_device *device, const D3D12_SAMPLER_DESC *desc) { struct vkd3d_view *view; - - d3d12_desc_destroy(sampler, device); + struct d3d12_desc new_sampler; + pthread_spinlock_t *descriptor_lock; if (!desc) { WARN("NULL sampler desc.\n"); - return; + goto done; } if (desc->AddressU == D3D12_TEXTURE_ADDRESS_MODE_BORDER @@ -2904,19 +2942,29 @@ void d3d12_desc_create_sampler(struct d3d12_desc *sampler, desc->BorderColor[0], desc->BorderColor[1], desc->BorderColor[2], desc->BorderColor[3]); if (!(view = vkd3d_view_create())) - return; + goto done; if (d3d12_create_sampler(device, desc->Filter, desc->AddressU, desc->AddressV, desc->AddressW, desc->MipLODBias, desc->MaxAnisotropy, desc->ComparisonFunc, desc->MinLOD, desc->MaxLOD, &view->u.vk_sampler) < 0) { vkd3d_free(view); - return; + goto done; } - sampler->magic = VKD3D_DESCRIPTOR_MAGIC_SAMPLER; - sampler->vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLER; - sampler->u.view = view; + new_sampler.magic = VKD3D_DESCRIPTOR_MAGIC_SAMPLER; + new_sampler.vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLER; + new_sampler.u.view = view; + + done: + pthread_spin_lock(sampler->lock); + descriptor_lock = sampler->lock; + + d3d12_desc_destroy(sampler, device); + *sampler = new_sampler; + + sampler->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } HRESULT vkd3d_create_static_sampler(struct d3d12_device *device, @@ -2950,22 +2998,22 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev { struct vkd3d_texture_view_desc vkd3d_desc; struct vkd3d_view *view; - - d3d12_rtv_desc_destroy(rtv_desc, device); + struct d3d12_rtv_desc new_rtv_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { FIXME("NULL resource RTV not implemented.\n"); - return; + goto done; } if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done; if (vkd3d_desc.format->vk_aspect_mask != VK_IMAGE_ASPECT_COLOR_BIT) { WARN("Trying to create RTV for depth/stencil format %#x.\n", vkd3d_desc.format->dxgi_format); - return; + goto done; } if (desc) @@ -3013,16 +3061,26 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev assert(d3d12_resource_is_texture(resource)); if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done; - rtv_desc->magic = VKD3D_DESCRIPTOR_MAGIC_RTV; - rtv_desc->sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); - rtv_desc->format = vkd3d_desc.format; - rtv_desc->width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); - rtv_desc->height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); - rtv_desc->layer_count = vkd3d_desc.layer_count; - rtv_desc->view = view; - rtv_desc->resource = resource; + new_rtv_desc.magic = VKD3D_DESCRIPTOR_MAGIC_RTV; + new_rtv_desc.sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); + new_rtv_desc.format = vkd3d_desc.format; + new_rtv_desc.width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); + new_rtv_desc.height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); + new_rtv_desc.layer_count = vkd3d_desc.layer_count; + new_rtv_desc.view = view; + new_rtv_desc.resource = resource; + + done: + pthread_spin_lock(rtv_desc->lock); + descriptor_lock = rtv_desc->lock; + + d3d12_rtv_desc_destroy(rtv_desc, device); + *rtv_desc = new_rtv_desc; + + rtv_desc->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } /* DSVs */ @@ -3040,28 +3098,28 @@ void d3d12_dsv_desc_create_dsv(struct d3d12_dsv_desc *dsv_desc, struct d3d12_dev { struct vkd3d_texture_view_desc vkd3d_desc; struct vkd3d_view *view; - - d3d12_dsv_desc_destroy(dsv_desc, device); + struct d3d12_dsv_desc new_dsv_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { FIXME("NULL resource DSV not implemented.\n"); - return; + goto done; } if (resource->desc.Dimension == D3D12_RESOURCE_DIMENSION_TEXTURE3D) { WARN("Cannot create DSV for 3D texture.\n"); - return; + goto done; } if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done; if (!(vkd3d_desc.format->vk_aspect_mask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT))) { WARN("Trying to create DSV for format %#x.\n", vkd3d_desc.format->dxgi_format); - return; + goto done; } if (desc) @@ -3096,16 +3154,26 @@ void d3d12_dsv_desc_create_dsv(struct d3d12_dsv_desc *dsv_desc, struct d3d12_dev assert(d3d12_resource_is_texture(resource)); if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done; + + new_dsv_desc.magic = VKD3D_DESCRIPTOR_MAGIC_DSV; + new_dsv_desc.sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); + new_dsv_desc.format = vkd3d_desc.format; + new_dsv_desc.width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); + new_dsv_desc.height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); + new_dsv_desc.layer_count = vkd3d_desc.layer_count; + new_dsv_desc.view = view; + new_dsv_desc.resource = resource; + + done: + pthread_spin_lock(dsv_desc->lock); + descriptor_lock = dsv_desc->lock; + + d3d12_dsv_desc_destroy(dsv_desc, device); + *dsv_desc = new_dsv_desc; - dsv_desc->magic = VKD3D_DESCRIPTOR_MAGIC_DSV; - dsv_desc->sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); - dsv_desc->format = vkd3d_desc.format; - dsv_desc->width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); - dsv_desc->height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); - dsv_desc->layer_count = vkd3d_desc.layer_count; - dsv_desc->view = view; - dsv_desc->resource = resource; + dsv_desc->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } /* ID3D12DescriptorHeap */ @@ -3200,6 +3268,13 @@ static ULONG STDMETHODCALLTYPE d3d12_descriptor_heap_Release(ID3D12DescriptorHea break; } + for (i = 0; i < heap->desc.NumDescriptors; i++) + { + pthread_spin_destroy(&heap->locks[i]); + } + + vkd3d_free((void*)heap->locks); + vkd3d_free(heap); d3d12_device_release(device); @@ -3323,6 +3398,9 @@ static HRESULT d3d12_descriptor_heap_init(struct d3d12_descriptor_heap *descript if (FAILED(hr = vkd3d_private_store_init(&descriptor_heap->private_store))) return hr; + if (!(descriptor_heap->locks = vkd3d_malloc(sizeof(pthread_spinlock_t) * desc->NumDescriptors))) + return E_OUTOFMEMORY; + d3d12_device_add_ref(descriptor_heap->device = device); return S_OK; @@ -3367,6 +3445,15 @@ HRESULT d3d12_descriptor_heap_create(struct d3d12_device *device, memset(object->descriptors, 0, descriptor_size * desc->NumDescriptors); + for (unsigned int i = 0; i < desc->NumDescriptors; i++) + { + struct d3d12_desc *cur_desc = (struct d3d12_desc *) (object->descriptors + (i * descriptor_size)); + pthread_spinlock_t *lock = &object->locks[i]; + + pthread_spin_init(lock, PTHREAD_PROCESS_PRIVATE); + cur_desc->lock = lock; + } + TRACE("Created descriptor heap %p.\n", object); *descriptor_heap = object; diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index bd9670e..732ded5 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -466,6 +466,7 @@ void vkd3d_view_incref(struct vkd3d_view *view) DECLSPEC_HIDDEN; struct d3d12_desc { uint32_t magic; + pthread_spinlock_t *lock; VkDescriptorType vk_descriptor_type; union { @@ -521,6 +522,7 @@ HRESULT vkd3d_create_static_sampler(struct d3d12_device *device, struct d3d12_rtv_desc { uint32_t magic; + pthread_spinlock_t *lock; VkSampleCountFlagBits sample_count; const struct vkd3d_format *format; uint64_t width; @@ -541,6 +543,7 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev struct d3d12_dsv_desc { uint32_t magic; + pthread_spinlock_t *lock; VkSampleCountFlagBits sample_count; const struct vkd3d_format *format; uint64_t width; @@ -570,6 +573,8 @@ struct d3d12_descriptor_heap struct vkd3d_private_store private_store; + pthread_spinlock_t *locks; + BYTE descriptors[]; }; -- 2.23.0

Sveinar Søpler

3:59 p.m.

New subject: [PATCH v4 2/2] vkd3d: Don't allow concurrent writes to descriptors.

Would this by any chance fix something regarding this bug: https://bugs.winehq.org/show_bug.cgi?id=46410 Sveinar ----- On Sep 13, 2019, at 4:19 PM, Derek Lesho dlesho(a)codeweavers.com wrote:

...

In D3D12 it is legal to concurrently overwrite descriptors, we can't handle that. This patch locks descriptors from being overwritten by two threads at the same time.

Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v4: Allocate the descriptor locks for a heap in a single array to avoid unecessary allocations. --- libs/vkd3d/resource.c | 207 ++++++++++++++++++++++++++----------- libs/vkd3d/vkd3d_private.h | 5 + 2 files changed, 152 insertions(+), 60 deletions(-)

diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index 463f373..e6d3b51 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -1947,8 +1947,13 @@ static void d3d12_desc_destroy(struct d3d12_desc *descriptor, void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src, struct d3d12_device *device) { + pthread_spinlock_t *descriptor_lock; + assert(dst != src);

+ pthread_spin_lock(dst->lock); + descriptor_lock = dst->lock; + d3d12_desc_destroy(dst, device);

*dst = *src; @@ -1959,6 +1964,8 @@ void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src, { vkd3d_view_incref(src->u.view); } + + pthread_spin_unlock(descriptor_lock); }

static VkDeviceSize vkd3d_get_required_texel_buffer_alignment(const struct d3d12_device *device, @@ -2322,22 +2329,22 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor, { struct VkDescriptorBufferInfo *buffer_info; struct d3d12_resource *resource; - - d3d12_desc_destroy(descriptor, device); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock;

if (!desc) { WARN("Constant buffer desc is NULL.\n"); - return; + goto done; }

if (desc->SizeInBytes & (D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT - 1)) { WARN("Size is not %u bytes aligned.\n", D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT); - return; + goto done; }

- buffer_info = &descriptor->u.vk_cbv_info; + buffer_info = &new_desc.u.vk_cbv_info; if (desc->BufferLocation) { resource = vkd3d_gpu_va_allocator_dereference(&device->gpu_va_allocator, desc->BufferLocation); @@ -2353,8 +2360,18 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor, buffer_info->range = VKD3D_NULL_BUFFER_SIZE; }

- descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_CBV; - descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + new_desc.magic = VKD3D_DESCRIPTOR_MAGIC_CBV; + new_desc.vk_descriptor_type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

static unsigned int vkd3d_view_flags_from_d3d12_buffer_srv_flags(D3D12_BUFFER_SRV_FLAGS flags) @@ -2466,23 +2483,23 @@ 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); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock;

if (!resource) { - vkd3d_create_null_srv(descriptor, device, desc); - return; + vkd3d_create_null_srv(&new_desc, device, desc); + goto done; }

if (d3d12_resource_is_buffer(resource)) { - vkd3d_create_buffer_srv(descriptor, device, resource, desc); - return; + vkd3d_create_buffer_srv(&new_desc, device, resource, desc); + goto done; }

if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done;

vkd3d_desc.miplevel_count = VK_REMAINING_MIP_LEVELS; vkd3d_desc.allowed_swizzle = true; @@ -2559,11 +2576,21 @@ void d3d12_desc_create_srv(struct d3d12_desc *descriptor, }

if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done;

- descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_SRV; - descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; - descriptor->u.view = view; + new_desc.magic = VKD3D_DESCRIPTOR_MAGIC_SRV; + new_desc.vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; + new_desc.u.view = view; + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

static unsigned int vkd3d_view_flags_from_d3d12_buffer_uav_flags(D3D12_BUFFER_UAV_FLAGS flags) @@ -2761,26 +2788,37 @@ 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); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock;

if (!resource) { if (counter_resource) FIXME("Ignoring counter resource %p.\n", counter_resource); - vkd3d_create_null_uav(descriptor, device, desc); - return; + vkd3d_create_null_uav(&new_desc, device, desc); + goto done; }

if (d3d12_resource_is_buffer(resource)) { - vkd3d_create_buffer_uav(descriptor, device, resource, counter_resource, desc); + vkd3d_create_buffer_uav(&new_desc, device, resource, counter_resource, desc); } else { if (counter_resource) FIXME("Unexpected counter resource for texture view.\n"); - vkd3d_create_texture_uav(descriptor, device, resource, desc); + vkd3d_create_texture_uav(&new_desc, device, resource, desc); } + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

bool vkd3d_create_raw_buffer_view(struct d3d12_device *device, @@ -2888,13 +2926,13 @@ void d3d12_desc_create_sampler(struct d3d12_desc *sampler, struct d3d12_device *device, const D3D12_SAMPLER_DESC *desc) { struct vkd3d_view *view; - - d3d12_desc_destroy(sampler, device); + struct d3d12_desc new_sampler; + pthread_spinlock_t *descriptor_lock;

if (!desc) { WARN("NULL sampler desc.\n"); - return; + goto done; }

if (desc->AddressU == D3D12_TEXTURE_ADDRESS_MODE_BORDER @@ -2904,19 +2942,29 @@ void d3d12_desc_create_sampler(struct d3d12_desc *sampler, desc->BorderColor[0], desc->BorderColor[1], desc->BorderColor[2], desc->BorderColor[3]);

if (!(view = vkd3d_view_create())) - return; + goto done;

if (d3d12_create_sampler(device, desc->Filter, desc->AddressU, desc->AddressV, desc->AddressW, desc->MipLODBias, desc->MaxAnisotropy, desc->ComparisonFunc, desc->MinLOD, desc->MaxLOD, &view->u.vk_sampler) < 0) { vkd3d_free(view); - return; + goto done; }

- sampler->magic = VKD3D_DESCRIPTOR_MAGIC_SAMPLER; - sampler->vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLER; - sampler->u.view = view; + new_sampler.magic = VKD3D_DESCRIPTOR_MAGIC_SAMPLER; + new_sampler.vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLER; + new_sampler.u.view = view; + + done: + pthread_spin_lock(sampler->lock); + descriptor_lock = sampler->lock; + + d3d12_desc_destroy(sampler, device); + *sampler = new_sampler; + + sampler->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

HRESULT vkd3d_create_static_sampler(struct d3d12_device *device, @@ -2950,22 +2998,22 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev { struct vkd3d_texture_view_desc vkd3d_desc; struct vkd3d_view *view; - - d3d12_rtv_desc_destroy(rtv_desc, device); + struct d3d12_rtv_desc new_rtv_desc = {}; + pthread_spinlock_t *descriptor_lock;

if (!resource) { FIXME("NULL resource RTV not implemented.\n"); - return; + goto done; }

if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done;

if (vkd3d_desc.format->vk_aspect_mask != VK_IMAGE_ASPECT_COLOR_BIT) { WARN("Trying to create RTV for depth/stencil format %#x.\n", vkd3d_desc.format->dxgi_format); - return; + goto done; }

if (desc) @@ -3013,16 +3061,26 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev assert(d3d12_resource_is_texture(resource));

if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done;

- rtv_desc->magic = VKD3D_DESCRIPTOR_MAGIC_RTV; - rtv_desc->sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); - rtv_desc->format = vkd3d_desc.format; - rtv_desc->width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); - rtv_desc->height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); - rtv_desc->layer_count = vkd3d_desc.layer_count; - rtv_desc->view = view; - rtv_desc->resource = resource; + new_rtv_desc.magic = VKD3D_DESCRIPTOR_MAGIC_RTV; + new_rtv_desc.sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); + new_rtv_desc.format = vkd3d_desc.format; + new_rtv_desc.width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); + new_rtv_desc.height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); + new_rtv_desc.layer_count = vkd3d_desc.layer_count; + new_rtv_desc.view = view; + new_rtv_desc.resource = resource; + + done: + pthread_spin_lock(rtv_desc->lock); + descriptor_lock = rtv_desc->lock; + + d3d12_rtv_desc_destroy(rtv_desc, device); + *rtv_desc = new_rtv_desc; + + rtv_desc->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

/* DSVs */ @@ -3040,28 +3098,28 @@ void d3d12_dsv_desc_create_dsv(struct d3d12_dsv_desc *dsv_desc, struct d3d12_dev { struct vkd3d_texture_view_desc vkd3d_desc; struct vkd3d_view *view; - - d3d12_dsv_desc_destroy(dsv_desc, device); + struct d3d12_dsv_desc new_dsv_desc = {}; + pthread_spinlock_t *descriptor_lock;

if (!resource) { FIXME("NULL resource DSV not implemented.\n"); - return; + goto done; }

if (resource->desc.Dimension == D3D12_RESOURCE_DIMENSION_TEXTURE3D) { WARN("Cannot create DSV for 3D texture.\n"); - return; + goto done; }

if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done;

if (!(vkd3d_desc.format->vk_aspect_mask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT))) { WARN("Trying to create DSV for format %#x.\n", vkd3d_desc.format->dxgi_format); - return; + goto done; }

if (desc) @@ -3096,16 +3154,26 @@ void d3d12_dsv_desc_create_dsv(struct d3d12_dsv_desc *dsv_desc, struct d3d12_dev assert(d3d12_resource_is_texture(resource));

if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done; + + new_dsv_desc.magic = VKD3D_DESCRIPTOR_MAGIC_DSV; + new_dsv_desc.sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); + new_dsv_desc.format = vkd3d_desc.format; + new_dsv_desc.width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); + new_dsv_desc.height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); + new_dsv_desc.layer_count = vkd3d_desc.layer_count; + new_dsv_desc.view = view; + new_dsv_desc.resource = resource; + + done: + pthread_spin_lock(dsv_desc->lock); + descriptor_lock = dsv_desc->lock; + + d3d12_dsv_desc_destroy(dsv_desc, device); + *dsv_desc = new_dsv_desc;

- dsv_desc->magic = VKD3D_DESCRIPTOR_MAGIC_DSV; - dsv_desc->sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); - dsv_desc->format = vkd3d_desc.format; - dsv_desc->width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); - dsv_desc->height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); - dsv_desc->layer_count = vkd3d_desc.layer_count; - dsv_desc->view = view; - dsv_desc->resource = resource; + dsv_desc->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

/* ID3D12DescriptorHeap */ @@ -3200,6 +3268,13 @@ static ULONG STDMETHODCALLTYPE d3d12_descriptor_heap_Release(ID3D12DescriptorHea break; }

+ for (i = 0; i < heap->desc.NumDescriptors; i++) + { + pthread_spin_destroy(&heap->locks[i]); + } + + vkd3d_free((void*)heap->locks); + vkd3d_free(heap);

d3d12_device_release(device); @@ -3323,6 +3398,9 @@ static HRESULT d3d12_descriptor_heap_init(struct d3d12_descriptor_heap *descript if (FAILED(hr = vkd3d_private_store_init(&descriptor_heap->private_store))) return hr;

+ if (!(descriptor_heap->locks = vkd3d_malloc(sizeof(pthread_spinlock_t) * desc->NumDescriptors))) + return E_OUTOFMEMORY; + d3d12_device_add_ref(descriptor_heap->device = device);

return S_OK; @@ -3367,6 +3445,15 @@ HRESULT d3d12_descriptor_heap_create(struct d3d12_device *device,

memset(object->descriptors, 0, descriptor_size * desc->NumDescriptors);

+ for (unsigned int i = 0; i < desc->NumDescriptors; i++) + { + struct d3d12_desc *cur_desc = (struct d3d12_desc *) (object->descriptors + (i * descriptor_size)); + pthread_spinlock_t *lock = &object->locks[i]; + + pthread_spin_init(lock, PTHREAD_PROCESS_PRIVATE); + cur_desc->lock = lock; + } + TRACE("Created descriptor heap %p.\n", object);

*descriptor_heap = object; diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index bd9670e..732ded5 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -466,6 +466,7 @@ void vkd3d_view_incref(struct vkd3d_view *view) DECLSPEC_HIDDEN; struct d3d12_desc { uint32_t magic; + pthread_spinlock_t *lock; VkDescriptorType vk_descriptor_type; union { @@ -521,6 +522,7 @@ HRESULT vkd3d_create_static_sampler(struct d3d12_device *device, struct d3d12_rtv_desc { uint32_t magic; + pthread_spinlock_t *lock; VkSampleCountFlagBits sample_count; const struct vkd3d_format *format; uint64_t width; @@ -541,6 +543,7 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev struct d3d12_dsv_desc { uint32_t magic; + pthread_spinlock_t *lock; VkSampleCountFlagBits sample_count; const struct vkd3d_format *format; uint64_t width; @@ -570,6 +573,8 @@ struct d3d12_descriptor_heap

struct vkd3d_private_store private_store;

+ pthread_spinlock_t *locks; + BYTE descriptors[]; };

-- 2.23.0

Derek Lesho

4:03 p.m.

New subject: [PATCH v4 2/2] vkd3d: Don't allow concurrent writes to descriptors.

On 9/13/19 10:59 AM, Sveinar Søpler wrote:

...

Would this by any chance fix something regarding this bug: https://bugs.winehq.org/show_bug.cgi?id=46410

Sveinar

----- On Sep 13, 2019, at 4:19 PM, Derek Lesho dlesho(a)codeweavers.com wrote:

...
In D3D12 it is legal to concurrently overwrite descriptors, we can't handle that. This patch locks descriptors from being overwritten by two threads at the same time.

Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v4: Allocate the descriptor locks for a heap in a single array to avoid unecessary allocations. --- libs/vkd3d/resource.c | 207 ++++++++++++++++++++++++++----------- libs/vkd3d/vkd3d_private.h | 5 + 2 files changed, 152 insertions(+), 60 deletions(-)

diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index 463f373..e6d3b51 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -1947,8 +1947,13 @@ static void d3d12_desc_destroy(struct d3d12_desc *descriptor, void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src, struct d3d12_device *device) { + pthread_spinlock_t *descriptor_lock; + assert(dst != src);

+ pthread_spin_lock(dst->lock); + descriptor_lock = dst->lock; + d3d12_desc_destroy(dst, device);

*dst = *src; @@ -1959,6 +1964,8 @@ void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src, { vkd3d_view_incref(src->u.view); } + + pthread_spin_unlock(descriptor_lock); }

static VkDeviceSize vkd3d_get_required_texel_buffer_alignment(const struct d3d12_device *device, @@ -2322,22 +2329,22 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor, { struct VkDescriptorBufferInfo *buffer_info; struct d3d12_resource *resource; - - d3d12_desc_destroy(descriptor, device); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock;

if (!desc) { WARN("Constant buffer desc is NULL.\n"); - return; + goto done; }

if (desc->SizeInBytes & (D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT - 1)) { WARN("Size is not %u bytes aligned.\n", D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT); - return; + goto done; }

- buffer_info = &descriptor->u.vk_cbv_info; + buffer_info = &new_desc.u.vk_cbv_info; if (desc->BufferLocation) { resource = vkd3d_gpu_va_allocator_dereference(&device->gpu_va_allocator, desc->BufferLocation); @@ -2353,8 +2360,18 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor, buffer_info->range = VKD3D_NULL_BUFFER_SIZE; }

- descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_CBV; - descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + new_desc.magic = VKD3D_DESCRIPTOR_MAGIC_CBV; + new_desc.vk_descriptor_type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

static unsigned int vkd3d_view_flags_from_d3d12_buffer_srv_flags(D3D12_BUFFER_SRV_FLAGS flags) @@ -2466,23 +2483,23 @@ 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); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock;

if (!resource) { - vkd3d_create_null_srv(descriptor, device, desc); - return; + vkd3d_create_null_srv(&new_desc, device, desc); + goto done; }

if (d3d12_resource_is_buffer(resource)) { - vkd3d_create_buffer_srv(descriptor, device, resource, desc); - return; + vkd3d_create_buffer_srv(&new_desc, device, resource, desc); + goto done; }

if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done;

vkd3d_desc.miplevel_count = VK_REMAINING_MIP_LEVELS; vkd3d_desc.allowed_swizzle = true; @@ -2559,11 +2576,21 @@ void d3d12_desc_create_srv(struct d3d12_desc *descriptor, }

if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done;

- descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_SRV; - descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; - descriptor->u.view = view; + new_desc.magic = VKD3D_DESCRIPTOR_MAGIC_SRV; + new_desc.vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; + new_desc.u.view = view; + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

static unsigned int vkd3d_view_flags_from_d3d12_buffer_uav_flags(D3D12_BUFFER_UAV_FLAGS flags) @@ -2761,26 +2788,37 @@ 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); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock;

if (!resource) { if (counter_resource) FIXME("Ignoring counter resource %p.\n", counter_resource); - vkd3d_create_null_uav(descriptor, device, desc); - return; + vkd3d_create_null_uav(&new_desc, device, desc); + goto done; }

if (d3d12_resource_is_buffer(resource)) { - vkd3d_create_buffer_uav(descriptor, device, resource, counter_resource, desc); + vkd3d_create_buffer_uav(&new_desc, device, resource, counter_resource, desc); } else { if (counter_resource) FIXME("Unexpected counter resource for texture view.\n"); - vkd3d_create_texture_uav(descriptor, device, resource, desc); + vkd3d_create_texture_uav(&new_desc, device, resource, desc); } + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

bool vkd3d_create_raw_buffer_view(struct d3d12_device *device, @@ -2888,13 +2926,13 @@ void d3d12_desc_create_sampler(struct d3d12_desc *sampler, struct d3d12_device *device, const D3D12_SAMPLER_DESC *desc) { struct vkd3d_view *view; - - d3d12_desc_destroy(sampler, device); + struct d3d12_desc new_sampler; + pthread_spinlock_t *descriptor_lock;

if (!desc) { WARN("NULL sampler desc.\n"); - return; + goto done; }

if (desc->AddressU == D3D12_TEXTURE_ADDRESS_MODE_BORDER @@ -2904,19 +2942,29 @@ void d3d12_desc_create_sampler(struct d3d12_desc *sampler, desc->BorderColor[0], desc->BorderColor[1], desc->BorderColor[2], desc->BorderColor[3]);

if (!(view = vkd3d_view_create())) - return; + goto done;

if (d3d12_create_sampler(device, desc->Filter, desc->AddressU, desc->AddressV, desc->AddressW, desc->MipLODBias, desc->MaxAnisotropy, desc->ComparisonFunc, desc->MinLOD, desc->MaxLOD, &view->u.vk_sampler) < 0) { vkd3d_free(view); - return; + goto done; }

- sampler->magic = VKD3D_DESCRIPTOR_MAGIC_SAMPLER; - sampler->vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLER; - sampler->u.view = view; + new_sampler.magic = VKD3D_DESCRIPTOR_MAGIC_SAMPLER; + new_sampler.vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLER; + new_sampler.u.view = view; + + done: + pthread_spin_lock(sampler->lock); + descriptor_lock = sampler->lock; + + d3d12_desc_destroy(sampler, device); + *sampler = new_sampler; + + sampler->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

HRESULT vkd3d_create_static_sampler(struct d3d12_device *device, @@ -2950,22 +2998,22 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev { struct vkd3d_texture_view_desc vkd3d_desc; struct vkd3d_view *view; - - d3d12_rtv_desc_destroy(rtv_desc, device); + struct d3d12_rtv_desc new_rtv_desc = {}; + pthread_spinlock_t *descriptor_lock;

if (!resource) { FIXME("NULL resource RTV not implemented.\n"); - return; + goto done; }

if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done;

if (vkd3d_desc.format->vk_aspect_mask != VK_IMAGE_ASPECT_COLOR_BIT) { WARN("Trying to create RTV for depth/stencil format %#x.\n", vkd3d_desc.format->dxgi_format); - return; + goto done; }

if (desc) @@ -3013,16 +3061,26 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev assert(d3d12_resource_is_texture(resource));

if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done;

- rtv_desc->magic = VKD3D_DESCRIPTOR_MAGIC_RTV; - rtv_desc->sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); - rtv_desc->format = vkd3d_desc.format; - rtv_desc->width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); - rtv_desc->height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); - rtv_desc->layer_count = vkd3d_desc.layer_count; - rtv_desc->view = view; - rtv_desc->resource = resource; + new_rtv_desc.magic = VKD3D_DESCRIPTOR_MAGIC_RTV; + new_rtv_desc.sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); + new_rtv_desc.format = vkd3d_desc.format; + new_rtv_desc.width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); + new_rtv_desc.height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); + new_rtv_desc.layer_count = vkd3d_desc.layer_count; + new_rtv_desc.view = view; + new_rtv_desc.resource = resource; + + done: + pthread_spin_lock(rtv_desc->lock); + descriptor_lock = rtv_desc->lock; + + d3d12_rtv_desc_destroy(rtv_desc, device); + *rtv_desc = new_rtv_desc; + + rtv_desc->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

/* DSVs */ @@ -3040,28 +3098,28 @@ void d3d12_dsv_desc_create_dsv(struct d3d12_dsv_desc *dsv_desc, struct d3d12_dev { struct vkd3d_texture_view_desc vkd3d_desc; struct vkd3d_view *view; - - d3d12_dsv_desc_destroy(dsv_desc, device); + struct d3d12_dsv_desc new_dsv_desc = {}; + pthread_spinlock_t *descriptor_lock;

if (!resource) { FIXME("NULL resource DSV not implemented.\n"); - return; + goto done; }

if (resource->desc.Dimension == D3D12_RESOURCE_DIMENSION_TEXTURE3D) { WARN("Cannot create DSV for 3D texture.\n"); - return; + goto done; }

if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done;

if (!(vkd3d_desc.format->vk_aspect_mask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT))) { WARN("Trying to create DSV for format %#x.\n", vkd3d_desc.format->dxgi_format); - return; + goto done; }

if (desc) @@ -3096,16 +3154,26 @@ void d3d12_dsv_desc_create_dsv(struct d3d12_dsv_desc *dsv_desc, struct d3d12_dev assert(d3d12_resource_is_texture(resource));

if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done; + + new_dsv_desc.magic = VKD3D_DESCRIPTOR_MAGIC_DSV; + new_dsv_desc.sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); + new_dsv_desc.format = vkd3d_desc.format; + new_dsv_desc.width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); + new_dsv_desc.height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); + new_dsv_desc.layer_count = vkd3d_desc.layer_count; + new_dsv_desc.view = view; + new_dsv_desc.resource = resource; + + done: + pthread_spin_lock(dsv_desc->lock); + descriptor_lock = dsv_desc->lock; + + d3d12_dsv_desc_destroy(dsv_desc, device); + *dsv_desc = new_dsv_desc;

- dsv_desc->magic = VKD3D_DESCRIPTOR_MAGIC_DSV; - dsv_desc->sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); - dsv_desc->format = vkd3d_desc.format; - dsv_desc->width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); - dsv_desc->height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); - dsv_desc->layer_count = vkd3d_desc.layer_count; - dsv_desc->view = view; - dsv_desc->resource = resource; + dsv_desc->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); }

/* ID3D12DescriptorHeap */ @@ -3200,6 +3268,13 @@ static ULONG STDMETHODCALLTYPE d3d12_descriptor_heap_Release(ID3D12DescriptorHea break; }

+ for (i = 0; i < heap->desc.NumDescriptors; i++) + { + pthread_spin_destroy(&heap->locks[i]); + } + + vkd3d_free((void*)heap->locks); + vkd3d_free(heap);

d3d12_device_release(device); @@ -3323,6 +3398,9 @@ static HRESULT d3d12_descriptor_heap_init(struct d3d12_descriptor_heap *descript if (FAILED(hr = vkd3d_private_store_init(&descriptor_heap->private_store))) return hr;

+ if (!(descriptor_heap->locks = vkd3d_malloc(sizeof(pthread_spinlock_t) * desc->NumDescriptors))) + return E_OUTOFMEMORY; + d3d12_device_add_ref(descriptor_heap->device = device);

return S_OK; @@ -3367,6 +3445,15 @@ HRESULT d3d12_descriptor_heap_create(struct d3d12_device *device,

memset(object->descriptors, 0, descriptor_size * desc->NumDescriptors);

+ for (unsigned int i = 0; i < desc->NumDescriptors; i++) + { + struct d3d12_desc *cur_desc = (struct d3d12_desc *) (object->descriptors + (i * descriptor_size)); + pthread_spinlock_t *lock = &object->locks[i]; + + pthread_spin_init(lock, PTHREAD_PROCESS_PRIVATE); + cur_desc->lock = lock; + } + TRACE("Created descriptor heap %p.\n", object);

*descriptor_heap = object; diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index bd9670e..732ded5 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -466,6 +466,7 @@ void vkd3d_view_incref(struct vkd3d_view *view) DECLSPEC_HIDDEN; struct d3d12_desc { uint32_t magic; + pthread_spinlock_t *lock; VkDescriptorType vk_descriptor_type; union { @@ -521,6 +522,7 @@ HRESULT vkd3d_create_static_sampler(struct d3d12_device *device, struct d3d12_rtv_desc { uint32_t magic; + pthread_spinlock_t *lock; VkSampleCountFlagBits sample_count; const struct vkd3d_format *format; uint64_t width; @@ -541,6 +543,7 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev struct d3d12_dsv_desc { uint32_t magic; + pthread_spinlock_t *lock; VkSampleCountFlagBits sample_count; const struct vkd3d_format *format; uint64_t width; @@ -570,6 +573,8 @@ struct d3d12_descriptor_heap

struct vkd3d_private_store private_store;

+ pthread_spinlock_t *locks; + BYTE descriptors[]; };

-- 2.23.0

No, this patch is completely unrelated to that bug. Just so you're aware though, on discord this patch(https://github.com/GloriousEggroll/proton-ge-custom/blob/proton-ge-4.15/game...) was mentioned, which supposedly fixes that bug, if that's what you're looking for. -Derek

Sveinar Søpler

7:10 p.m.

New subject: [PATCH v4 2/2] vkd3d: Don't allow concurrent writes to descriptors.

Yeah, Mr. Kucia posted that patch in december -18 here: https://bugs.winehq.org/attachment.cgi?id=63022&action=diff I was just hoping this 9+ months old fix would be properly upstreamed instead of a "hack-patch" existing on random sites on the interweb. Sveinar ----- On Sep 13, 2019, at 6:03 PM, Derek Lesho dlesho(a)codeweavers.com wrote:

...

No, this patch is completely unrelated to that bug.

Just so you're aware though, on discord this patch(https://github.com/GloriousEggroll/proton-ge-custom/blob/proton-ge-4.15/game...) was mentioned, which supposedly fixes that bug, if that's what you're looking for.

-Derek

Conor McCarthy

3:22 a.m.

New subject: [PATCH v4 2/2] vkd3d: Don't allow concurrent writes to descriptors.

On Sat, Sep 14, 2019 at 12:20 AM Derek Lesho <dlesho(a)codeweavers.com> wrote:

...

+ pthread_spin_lock(descriptor->lock);

I think this isn't thread safe. If a thread tries to lock descriptor->lock when another thread has memset the descriptor to 0, it will crash if pthread_spin_lock() doesn't check for null. For performance reasons I think we need to test the possibility that any thread which meets a locked descriptor can skip the operation entirely. This would save spinning on the lock and an extra descriptor write. If two threads are writing the same descriptor simultaneously, this implies it's essentially random which thread sets what data, so maybe it will work if only the first thread writes the descriptor. Skipping can be done with pthread_spin_trylock(), but InterlockedCompareExchange() would be simpler.

Derek Lesho

2:07 p.m.

New subject: [PATCH v4 2/2] vkd3d: Don't allow concurrent writes to descriptors.

On 9/15/19 10:22 PM, Conor McCarthy wrote:

...

On Sat, Sep 14, 2019 at 12:20 AM Derek Lesho <dlesho(a)codeweavers.com <mailto:dlesho(a)codeweavers.com>> wrote:

+ pthread_spin_lock(descriptor->lock);

I think this isn't thread safe. If a thread tries to lock descriptor->lock when another thread has memset the descriptor to 0, it will crash if pthread_spin_lock() doesn't check for null.

For performance reasons I think we need to test the possibility that any thread which meets a locked descriptor can skip the operation entirely. This would save spinning on the lock and an extra descriptor write. If two threads are writing the same descriptor simultaneously, this implies it's essentially random which thread sets what data, so maybe it will work if only the first thread writes the descriptor. Skipping can be done with pthread_spin_trylock(), but InterlockedCompareExchange() would be simpler. You're right, I did overlook that (descriptor->lock being NULL). I'll try your idea to discard writes when one is already in progress with SOTTR, that solution should be faster.

Chip Davis

6:04 p.m.

September 12, 2019 10:25 AM, "Derek Lesho" <dlesho(a)codeweavers.com> wrote:

...

diff --git a/include/private/vkd3d_common.h b/include/private/vkd3d_common.h index 1ac8a63..c6a0946 100644 --- a/include/private/vkd3d_common.h +++ b/include/private/vkd3d_common.h @@ -168,4 +168,40 @@ static inline void vkd3d_parse_version(const char *version, int *major, int *min *minor = atoi(version); }

+#ifdef __APPLE__ +#include <os/lock.h> + +typedef os_unfair_lock pthread_spinlock_t; + +inline int pthread_spin_init(pthread_spinlock_t *lock, int pshared) +{ + if (pshared != PTHREAD_PROCESS_PRIVATE) return E_INVALIDARG;

I think you meant EINVAL here. Methinks you've been writing too many COM methods :). Chip

Derek Lesho

6:34 p.m.

New subject: [PATCH v3 1/2] vkd3d: On apple, implement pthread_spinlock_t with os_unfair_lock.

Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v3: Don't return a COM error in a POSIX function, thanks CHIP (; --- include/private/vkd3d_common.h | 36 ++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/include/private/vkd3d_common.h b/include/private/vkd3d_common.h index 1ac8a63..03f242d 100644 --- a/include/private/vkd3d_common.h +++ b/include/private/vkd3d_common.h @@ -168,4 +168,40 @@ static inline void vkd3d_parse_version(const char *version, int *major, int *min *minor = atoi(version); } +#ifdef __APPLE__ +#include <os/lock.h> + +typedef os_unfair_lock pthread_spinlock_t; + +inline int pthread_spin_init(pthread_spinlock_t *lock, int pshared) +{ + if (pshared != PTHREAD_PROCESS_PRIVATE) return E_INVAL; + *lock = OS_UNFAIR_LOCK_INIT; + return 0; +} + +inline int pthread_spin_lock(pthread_spinlock_t *lock) +{ + os_unfair_lock_lock(lock); + return 0; +} + +inline int pthread_spin_trylock(pthread_spinlock_t *lock) +{ + if (!(os_unfair_lock_trylock(lock))) return EBUSY; + return 0; +} + +inline int pthread_spin_unlock(pthread_spinlock_t *lock) +{ + os_unfair_lock_unlock(lock); + return 0; +} + +inline int pthread_spin_destroy(pthread_spinlock_t *lock) +{ + return 0; +} +#endif + #endif /* __VKD3D_COMMON_H */ -- 2.23.0

Derek Lesho

6:34 p.m.

New subject: [PATCH v3 2/2] vkd3d: Don't allow concurrent writes to descriptors.

In D3D12 it is legal to concurrently overwrite descriptors, we can't handle that. This patch locks descriptors from being overwritten by two threads at the same time. Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- libs/vkd3d/resource.c | 203 ++++++++++++++++++++++++++----------- libs/vkd3d/vkd3d_private.h | 3 + 2 files changed, 146 insertions(+), 60 deletions(-) diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index 463f373..848fb4d 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -1947,8 +1947,13 @@ static void d3d12_desc_destroy(struct d3d12_desc *descriptor, void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src, struct d3d12_device *device) { + pthread_spinlock_t *descriptor_lock; + assert(dst != src); + pthread_spin_lock(dst->lock); + descriptor_lock = dst->lock; + d3d12_desc_destroy(dst, device); *dst = *src; @@ -1959,6 +1964,8 @@ void d3d12_desc_copy(struct d3d12_desc *dst, const struct d3d12_desc *src, { vkd3d_view_incref(src->u.view); } + + pthread_spin_unlock(descriptor_lock); } static VkDeviceSize vkd3d_get_required_texel_buffer_alignment(const struct d3d12_device *device, @@ -2322,22 +2329,22 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor, { struct VkDescriptorBufferInfo *buffer_info; struct d3d12_resource *resource; - - d3d12_desc_destroy(descriptor, device); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!desc) { WARN("Constant buffer desc is NULL.\n"); - return; + goto done; } if (desc->SizeInBytes & (D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT - 1)) { WARN("Size is not %u bytes aligned.\n", D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT); - return; + goto done; } - buffer_info = &descriptor->u.vk_cbv_info; + buffer_info = &new_desc.u.vk_cbv_info; if (desc->BufferLocation) { resource = vkd3d_gpu_va_allocator_dereference(&device->gpu_va_allocator, desc->BufferLocation); @@ -2353,8 +2360,18 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor, buffer_info->range = VKD3D_NULL_BUFFER_SIZE; } - descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_CBV; - descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + new_desc.magic = VKD3D_DESCRIPTOR_MAGIC_CBV; + new_desc.vk_descriptor_type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } static unsigned int vkd3d_view_flags_from_d3d12_buffer_srv_flags(D3D12_BUFFER_SRV_FLAGS flags) @@ -2466,23 +2483,23 @@ 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); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { - vkd3d_create_null_srv(descriptor, device, desc); - return; + vkd3d_create_null_srv(&new_desc, device, desc); + goto done; } if (d3d12_resource_is_buffer(resource)) { - vkd3d_create_buffer_srv(descriptor, device, resource, desc); - return; + vkd3d_create_buffer_srv(&new_desc, device, resource, desc); + goto done; } if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done; vkd3d_desc.miplevel_count = VK_REMAINING_MIP_LEVELS; vkd3d_desc.allowed_swizzle = true; @@ -2559,11 +2576,21 @@ void d3d12_desc_create_srv(struct d3d12_desc *descriptor, } if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done; - descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_SRV; - descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; - descriptor->u.view = view; + new_desc.magic = VKD3D_DESCRIPTOR_MAGIC_SRV; + new_desc.vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; + new_desc.u.view = view; + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } static unsigned int vkd3d_view_flags_from_d3d12_buffer_uav_flags(D3D12_BUFFER_UAV_FLAGS flags) @@ -2761,26 +2788,37 @@ 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); + struct d3d12_desc new_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { if (counter_resource) FIXME("Ignoring counter resource %p.\n", counter_resource); - vkd3d_create_null_uav(descriptor, device, desc); - return; + vkd3d_create_null_uav(&new_desc, device, desc); + goto done; } if (d3d12_resource_is_buffer(resource)) { - vkd3d_create_buffer_uav(descriptor, device, resource, counter_resource, desc); + vkd3d_create_buffer_uav(&new_desc, device, resource, counter_resource, desc); } else { if (counter_resource) FIXME("Unexpected counter resource for texture view.\n"); - vkd3d_create_texture_uav(descriptor, device, resource, desc); + vkd3d_create_texture_uav(&new_desc, device, resource, desc); } + + done: + pthread_spin_lock(descriptor->lock); + descriptor_lock = descriptor->lock; + + d3d12_desc_destroy(descriptor, device); + *descriptor = new_desc; + + descriptor->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } bool vkd3d_create_raw_buffer_view(struct d3d12_device *device, @@ -2888,13 +2926,13 @@ void d3d12_desc_create_sampler(struct d3d12_desc *sampler, struct d3d12_device *device, const D3D12_SAMPLER_DESC *desc) { struct vkd3d_view *view; - - d3d12_desc_destroy(sampler, device); + struct d3d12_desc new_sampler; + pthread_spinlock_t *descriptor_lock; if (!desc) { WARN("NULL sampler desc.\n"); - return; + goto done; } if (desc->AddressU == D3D12_TEXTURE_ADDRESS_MODE_BORDER @@ -2904,19 +2942,29 @@ void d3d12_desc_create_sampler(struct d3d12_desc *sampler, desc->BorderColor[0], desc->BorderColor[1], desc->BorderColor[2], desc->BorderColor[3]); if (!(view = vkd3d_view_create())) - return; + goto done; if (d3d12_create_sampler(device, desc->Filter, desc->AddressU, desc->AddressV, desc->AddressW, desc->MipLODBias, desc->MaxAnisotropy, desc->ComparisonFunc, desc->MinLOD, desc->MaxLOD, &view->u.vk_sampler) < 0) { vkd3d_free(view); - return; + goto done; } - sampler->magic = VKD3D_DESCRIPTOR_MAGIC_SAMPLER; - sampler->vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLER; - sampler->u.view = view; + new_sampler.magic = VKD3D_DESCRIPTOR_MAGIC_SAMPLER; + new_sampler.vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLER; + new_sampler.u.view = view; + + done: + pthread_spin_lock(sampler->lock); + descriptor_lock = sampler->lock; + + d3d12_desc_destroy(sampler, device); + *sampler = new_sampler; + + sampler->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } HRESULT vkd3d_create_static_sampler(struct d3d12_device *device, @@ -2950,22 +2998,22 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev { struct vkd3d_texture_view_desc vkd3d_desc; struct vkd3d_view *view; - - d3d12_rtv_desc_destroy(rtv_desc, device); + struct d3d12_rtv_desc new_rtv_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { FIXME("NULL resource RTV not implemented.\n"); - return; + goto done; } if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done; if (vkd3d_desc.format->vk_aspect_mask != VK_IMAGE_ASPECT_COLOR_BIT) { WARN("Trying to create RTV for depth/stencil format %#x.\n", vkd3d_desc.format->dxgi_format); - return; + goto done; } if (desc) @@ -3013,16 +3061,26 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev assert(d3d12_resource_is_texture(resource)); if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done; + + new_rtv_desc.magic = VKD3D_DESCRIPTOR_MAGIC_RTV; + new_rtv_desc.sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); + new_rtv_desc.format = vkd3d_desc.format; + new_rtv_desc.width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); + new_rtv_desc.height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); + new_rtv_desc.layer_count = vkd3d_desc.layer_count; + new_rtv_desc.view = view; + new_rtv_desc.resource = resource; + + done: + pthread_spin_lock(rtv_desc->lock); + descriptor_lock = rtv_desc->lock; + + d3d12_rtv_desc_destroy(rtv_desc, device); + *rtv_desc = new_rtv_desc; - rtv_desc->magic = VKD3D_DESCRIPTOR_MAGIC_RTV; - rtv_desc->sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); - rtv_desc->format = vkd3d_desc.format; - rtv_desc->width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); - rtv_desc->height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); - rtv_desc->layer_count = vkd3d_desc.layer_count; - rtv_desc->view = view; - rtv_desc->resource = resource; + rtv_desc->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } /* DSVs */ @@ -3040,28 +3098,28 @@ void d3d12_dsv_desc_create_dsv(struct d3d12_dsv_desc *dsv_desc, struct d3d12_dev { struct vkd3d_texture_view_desc vkd3d_desc; struct vkd3d_view *view; - - d3d12_dsv_desc_destroy(dsv_desc, device); + struct d3d12_dsv_desc new_dsv_desc = {}; + pthread_spinlock_t *descriptor_lock; if (!resource) { FIXME("NULL resource DSV not implemented.\n"); - return; + goto done; } if (resource->desc.Dimension == D3D12_RESOURCE_DIMENSION_TEXTURE3D) { WARN("Cannot create DSV for 3D texture.\n"); - return; + goto done; } if (!init_default_texture_view_desc(&vkd3d_desc, resource, desc ? desc->Format : 0)) - return; + goto done; if (!(vkd3d_desc.format->vk_aspect_mask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT))) { WARN("Trying to create DSV for format %#x.\n", vkd3d_desc.format->dxgi_format); - return; + goto done; } if (desc) @@ -3096,16 +3154,26 @@ void d3d12_dsv_desc_create_dsv(struct d3d12_dsv_desc *dsv_desc, struct d3d12_dev assert(d3d12_resource_is_texture(resource)); if (!vkd3d_create_texture_view(device, resource->u.vk_image, &vkd3d_desc, &view)) - return; + goto done; + + new_dsv_desc.magic = VKD3D_DESCRIPTOR_MAGIC_DSV; + new_dsv_desc.sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); + new_dsv_desc.format = vkd3d_desc.format; + new_dsv_desc.width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); + new_dsv_desc.height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); + new_dsv_desc.layer_count = vkd3d_desc.layer_count; + new_dsv_desc.view = view; + new_dsv_desc.resource = resource; + + done: + pthread_spin_lock(dsv_desc->lock); + descriptor_lock = dsv_desc->lock; + + d3d12_dsv_desc_destroy(dsv_desc, device); + *dsv_desc = new_dsv_desc; - dsv_desc->magic = VKD3D_DESCRIPTOR_MAGIC_DSV; - dsv_desc->sample_count = vk_samples_from_dxgi_sample_desc(&resource->desc.SampleDesc); - dsv_desc->format = vkd3d_desc.format; - dsv_desc->width = d3d12_resource_desc_get_width(&resource->desc, vkd3d_desc.miplevel_idx); - dsv_desc->height = d3d12_resource_desc_get_height(&resource->desc, vkd3d_desc.miplevel_idx); - dsv_desc->layer_count = vkd3d_desc.layer_count; - dsv_desc->view = view; - dsv_desc->resource = resource; + dsv_desc->lock = descriptor_lock; + pthread_spin_unlock(descriptor_lock); } /* ID3D12DescriptorHeap */ @@ -3170,6 +3238,8 @@ static ULONG STDMETHODCALLTYPE d3d12_descriptor_heap_Release(ID3D12DescriptorHea for (i = 0; i < heap->desc.NumDescriptors; ++i) { d3d12_desc_destroy(&descriptors[i], device); + pthread_spin_destroy(descriptors[i].lock); + vkd3d_free((void*)descriptors[i].lock); } break; } @@ -3181,6 +3251,8 @@ static ULONG STDMETHODCALLTYPE d3d12_descriptor_heap_Release(ID3D12DescriptorHea for (i = 0; i < heap->desc.NumDescriptors; ++i) { d3d12_rtv_desc_destroy(&rtvs[i], device); + pthread_spin_destroy(rtvs[i].lock); + vkd3d_free((void*)rtvs[i].lock); } break; } @@ -3192,6 +3264,8 @@ static ULONG STDMETHODCALLTYPE d3d12_descriptor_heap_Release(ID3D12DescriptorHea for (i = 0; i < heap->desc.NumDescriptors; ++i) { d3d12_dsv_desc_destroy(&dsvs[i], device); + pthread_spin_destroy(dsvs[i].lock); + vkd3d_free((void*)dsvs[i].lock); } break; } @@ -3367,6 +3441,15 @@ HRESULT d3d12_descriptor_heap_create(struct d3d12_device *device, memset(object->descriptors, 0, descriptor_size * desc->NumDescriptors); + for (unsigned int i = 0; i < desc->NumDescriptors; i++) + { + struct d3d12_desc *cur_desc = (struct d3d12_desc *) (object->descriptors + (i * descriptor_size)); + pthread_spinlock_t *lock = vkd3d_malloc(sizeof(pthread_spinlock_t)); + + pthread_spin_init(lock, PTHREAD_PROCESS_PRIVATE); + cur_desc->lock = lock; + } + TRACE("Created descriptor heap %p.\n", object); *descriptor_heap = object; diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index bd9670e..6f1a957 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -466,6 +466,7 @@ void vkd3d_view_incref(struct vkd3d_view *view) DECLSPEC_HIDDEN; struct d3d12_desc { uint32_t magic; + pthread_spinlock_t *lock; VkDescriptorType vk_descriptor_type; union { @@ -521,6 +522,7 @@ HRESULT vkd3d_create_static_sampler(struct d3d12_device *device, struct d3d12_rtv_desc { uint32_t magic; + pthread_spinlock_t *lock; VkSampleCountFlagBits sample_count; const struct vkd3d_format *format; uint64_t width; @@ -541,6 +543,7 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev struct d3d12_dsv_desc { uint32_t magic; + pthread_spinlock_t *lock; VkSampleCountFlagBits sample_count; const struct vkd3d_format *format; uint64_t width; -- 2.23.0

2384

Age (days ago)

2388

Last active (days ago)

List overview

10 comments

4 participants

participants (4)

Chip Davis
Conor McCarthy
Derek Lesho
Sveinar Søpler