diff --git a/.pick_status.json b/.pick_status.json index c4075ea9a3a..69e0143e4f6 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -3954,7 +3954,7 @@ "description": "anv: Rework locking for sparse binding with TR-TT", "nominated": true, "nomination_type": 4, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": null, "notes": null diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c index 8774b1291be..333cf0e1762 100644 --- a/src/intel/vulkan/anv_batch_chain.c +++ b/src/intel/vulkan/anv_batch_chain.c @@ -1386,8 +1386,8 @@ can_chain_query_pools(struct anv_query_pool *p1, struct anv_query_pool *p2) } static VkResult -anv_queue_submit_sparse_bind_locked(struct anv_queue *queue, - struct vk_queue_submit *submit) +anv_queue_submit_sparse_bind(struct anv_queue *queue, + struct vk_queue_submit *submit) { struct anv_device *device = queue->device; VkResult result; @@ -1647,16 +1647,16 @@ anv_queue_submit(struct vk_queue *vk_queue, uint64_t start_ts = intel_ds_begin_submit(&queue->ds); - pthread_mutex_lock(&device->mutex); if (submit->buffer_bind_count || submit->image_opaque_bind_count || submit->image_bind_count) { - result = anv_queue_submit_sparse_bind_locked(queue, submit); + result = anv_queue_submit_sparse_bind(queue, submit); } else { + pthread_mutex_lock(&device->mutex); result = anv_queue_submit_cmd_buffers_locked(queue, submit, utrace_submit); + pthread_mutex_unlock(&device->mutex); } - pthread_mutex_unlock(&device->mutex); intel_ds_end_submit(&queue->ds, start_ts); intel_ds_device_process(&device->ds, false); diff --git a/src/intel/vulkan/anv_sparse.c b/src/intel/vulkan/anv_sparse.c index 3575c1d31a2..dc52078030a 100644 --- a/src/intel/vulkan/anv_sparse.c +++ b/src/intel/vulkan/anv_sparse.c @@ -686,11 +686,18 @@ anv_trtt_first_bind_init(struct anv_device *device) return VK_SUCCESS; } + /* We lock around execbuf because the algorithm we use for building the + * list of unique buffers isn't thread-safe. Lock the device mutex + * before the TRTT mutex for consistency with the order of other paths + * (e.g., anv_queue_submit_cmd_buffers_locked()). + */ + pthread_mutex_lock(&device->mutex); simple_mtx_lock(&trtt->mutex); /* This means we have already initialized the first bind. */ if (likely(trtt->l3_addr)) { simple_mtx_unlock(&trtt->mutex); + pthread_mutex_unlock(&device->mutex); return VK_SUCCESS; } @@ -736,6 +743,7 @@ out: trtt->l3_addr = 0; simple_mtx_unlock(&trtt->mutex); + pthread_mutex_unlock(&device->mutex); return result; } @@ -765,6 +773,12 @@ anv_sparse_bind_trtt(struct anv_device *device, if (result != VK_SUCCESS) goto out_async; + /* We lock around execbuf because the algorithm we use for building the + * list of unique buffers isn't thread-safe. Lock the device mutex + * before the TRTT mutex for consistency with the order that locking is + * done around other paths (e.g., anv_queue_submit_cmd_buffers_locked()). + */ + pthread_mutex_lock(&device->mutex); simple_mtx_lock(&trtt->mutex); /* Do this so we can avoid reallocs later. */ @@ -871,6 +885,7 @@ anv_sparse_bind_trtt(struct anv_device *device, list_addtail(&submit->link, &trtt->in_flight_batches); simple_mtx_unlock(&trtt->mutex); + pthread_mutex_unlock(&device->mutex); ANV_RMV(vm_binds, device, sparse_submit->binds, sparse_submit->binds_len); @@ -881,6 +896,7 @@ anv_sparse_bind_trtt(struct anv_device *device, util_dynarray_fini(&l3l2_binds); out_add_bind: simple_mtx_unlock(&trtt->mutex); + pthread_mutex_unlock(&device->mutex); anv_async_submit_fini(&submit->base); out_async: vk_free(&device->vk.alloc, submit);