From: Bas Nieuwenhuizen Date: Thu, 8 Oct 2020 12:36:08 +0000 (+0200) Subject: radv/winsys: Expand scope of allbos lock. X-Git-Tag: upstream/21.0.0~4359 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=875ff8414f66d10923171178adff49521fd9f70e;p=platform%2Fupstream%2Fmesa.git radv/winsys: Expand scope of allbos lock. With us not creating a bo_list anymore, there is a problem if we delete a buffer between enumerating all buffers and doing the submission. Also changes this to a rwlock given the wider scope of the things under lock. (especialy some of the syncobj stuff is now under the lock) Reviewed-by: Samuel Pitoiset Part-of: --- diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c index 496d6cf..a9fbb92 100644 --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c @@ -297,10 +297,10 @@ static void radv_amdgpu_winsys_bo_destroy(struct radeon_winsys_bo *_bo) free(bo->ranges); } else { if (bo->ws->debug_all_bos) { - pthread_mutex_lock(&bo->ws->global_bo_list_lock); + pthread_rwlock_wrlock(&bo->ws->global_bo_list_lock); list_del(&bo->global_list_item); bo->ws->num_buffers--; - pthread_mutex_unlock(&bo->ws->global_bo_list_lock); + pthread_rwlock_unlock(&bo->ws->global_bo_list_lock); } radv_amdgpu_bo_va_op(bo->ws, bo->bo, 0, bo->size, bo->base.va, 0, 0, AMDGPU_VA_OP_UNMAP); @@ -330,10 +330,10 @@ static void radv_amdgpu_add_buffer_to_global_list(struct radv_amdgpu_winsys_bo * struct radv_amdgpu_winsys *ws = bo->ws; if (bo->ws->debug_all_bos) { - pthread_mutex_lock(&ws->global_bo_list_lock); + pthread_rwlock_wrlock(&ws->global_bo_list_lock); list_addtail(&bo->global_list_item, &ws->global_bo_list); ws->num_buffers++; - pthread_mutex_unlock(&ws->global_bo_list_lock); + pthread_rwlock_unlock(&ws->global_bo_list_lock); } } diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c index 6d5478e..7df4375 100644 --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c @@ -727,11 +727,8 @@ radv_amdgpu_get_bo_list(struct radv_amdgpu_winsys *ws, if (ws->debug_all_bos) { struct radv_amdgpu_winsys_bo *bo; - pthread_mutex_lock(&ws->global_bo_list_lock); - handles = malloc(sizeof(handles[0]) * ws->num_buffers); if (!handles) { - pthread_mutex_unlock(&ws->global_bo_list_lock); return VK_ERROR_OUT_OF_HOST_MEMORY; } @@ -741,8 +738,6 @@ radv_amdgpu_get_bo_list(struct radv_amdgpu_winsys *ws, handles[num_handles].bo_priority = bo->priority; num_handles++; } - - pthread_mutex_unlock(&ws->global_bo_list_lock); } else if (count == 1 && !num_extra_bo && !extra_cs && !radv_bo_list && !radv_amdgpu_cs(cs_array[0])->num_virtual_buffers) { struct radv_amdgpu_cs *cs = (struct radv_amdgpu_cs*)cs_array[0]; @@ -894,6 +889,7 @@ radv_amdgpu_winsys_cs_submit_chained(struct radeon_winsys_ctx *_ctx, struct radv_amdgpu_ctx *ctx = radv_amdgpu_ctx(_ctx); struct radv_amdgpu_fence *fence = (struct radv_amdgpu_fence *)_fence; struct radv_amdgpu_cs *cs0 = radv_amdgpu_cs(cs_array[0]); + struct radv_amdgpu_winsys *aws = cs0->ws; struct drm_amdgpu_bo_list_entry *handles = NULL; struct radv_amdgpu_cs_request request = {0}; struct amdgpu_cs_ib_info ibs[2]; @@ -923,12 +919,15 @@ radv_amdgpu_winsys_cs_submit_chained(struct radeon_winsys_ctx *_ctx, } } + if (aws->debug_all_bos) + pthread_rwlock_rdlock(&aws->global_bo_list_lock); + /* Get the BO list. */ result = radv_amdgpu_get_bo_list(cs0->ws, cs_array, cs_count, NULL, 0, initial_preamble_cs, radv_bo_list, &num_handles, &handles); if (result != VK_SUCCESS) - return result; + goto fail; /* Configure the CS request. */ if (initial_preamble_cs) { @@ -953,14 +952,17 @@ radv_amdgpu_winsys_cs_submit_chained(struct radeon_winsys_ctx *_ctx, free(request.handles); if (result != VK_SUCCESS) - return result; + goto fail; if (fence) radv_amdgpu_request_to_fence(ctx, fence, &request); radv_assign_last_submit(ctx, &request); - return VK_SUCCESS; +fail: + if (aws->debug_all_bos) + pthread_rwlock_unlock(&aws->global_bo_list_lock); + return result; } static VkResult @@ -980,27 +982,34 @@ radv_amdgpu_winsys_cs_submit_fallback(struct radeon_winsys_ctx *_ctx, struct radv_amdgpu_cs_request request = {}; struct amdgpu_cs_ib_info *ibs; struct radv_amdgpu_cs *cs0; + struct radv_amdgpu_winsys *aws; unsigned num_handles = 0; unsigned number_of_ibs; VkResult result; assert(cs_count); cs0 = radv_amdgpu_cs(cs_array[0]); + aws = cs0->ws; /* Compute the number of IBs for this submit. */ number_of_ibs = cs_count + !!initial_preamble_cs; + if (aws->debug_all_bos) + pthread_rwlock_rdlock(&aws->global_bo_list_lock); + /* Get the BO list. */ result = radv_amdgpu_get_bo_list(cs0->ws, &cs_array[0], cs_count, NULL, 0, initial_preamble_cs, radv_bo_list, &num_handles, &handles); - if (result != VK_SUCCESS) - return result; + if (result != VK_SUCCESS) { + goto fail; + } ibs = malloc(number_of_ibs * sizeof(*ibs)); if (!ibs) { free(handles); - return VK_ERROR_OUT_OF_HOST_MEMORY; + result = VK_ERROR_OUT_OF_HOST_MEMORY; + goto fail; } /* Configure the CS request. */ @@ -1033,14 +1042,17 @@ radv_amdgpu_winsys_cs_submit_fallback(struct radeon_winsys_ctx *_ctx, free(ibs); if (result != VK_SUCCESS) - return result; + goto fail; if (fence) radv_amdgpu_request_to_fence(ctx, fence, &request); radv_assign_last_submit(ctx, &request); - return VK_SUCCESS; +fail: + if (aws->debug_all_bos) + pthread_rwlock_unlock(&aws->global_bo_list_lock); + return result; } static VkResult @@ -1058,6 +1070,7 @@ radv_amdgpu_winsys_cs_submit_sysmem(struct radeon_winsys_ctx *_ctx, struct radv_amdgpu_fence *fence = (struct radv_amdgpu_fence *)_fence; struct radv_amdgpu_cs *cs0 = radv_amdgpu_cs(cs_array[0]); struct radeon_winsys *ws = (struct radeon_winsys*)cs0->ws; + struct radv_amdgpu_winsys *aws = cs0->ws; struct radv_amdgpu_cs_request request; uint32_t pad_word = PKT3_NOP_PAD; bool emit_signal_sem = sem_info->cs_emit_signal; @@ -1195,6 +1208,9 @@ radv_amdgpu_winsys_cs_submit_sysmem(struct radeon_winsys_ctx *_ctx, ibs[0].flags = 0; } + if (aws->debug_all_bos) + pthread_rwlock_rdlock(&aws->global_bo_list_lock); + result = radv_amdgpu_get_bo_list(cs0->ws, &cs_array[i], cnt, (struct radv_amdgpu_winsys_bo **)bos, number_of_ibs, preamble_cs, @@ -1203,6 +1219,8 @@ radv_amdgpu_winsys_cs_submit_sysmem(struct radeon_winsys_ctx *_ctx, if (result != VK_SUCCESS) { free(ibs); free(bos); + if (aws->debug_all_bos) + pthread_rwlock_unlock(&aws->global_bo_list_lock); return result; } @@ -1220,6 +1238,8 @@ radv_amdgpu_winsys_cs_submit_sysmem(struct radeon_winsys_ctx *_ctx, result = radv_amdgpu_cs_submit(ctx, &request, sem_info); free(request.handles); + if (aws->debug_all_bos) + pthread_rwlock_unlock(&aws->global_bo_list_lock); for (unsigned j = 0; j < number_of_ibs; j++) { ws->buffer_destroy(bos[j]); @@ -1290,17 +1310,17 @@ static void *radv_amdgpu_winsys_get_cpu_addr(void *_cs, uint64_t addr) } } if(cs->ws->debug_all_bos) { - pthread_mutex_lock(&cs->ws->global_bo_list_lock); + pthread_rwlock_rdlock(&cs->ws->global_bo_list_lock); list_for_each_entry(struct radv_amdgpu_winsys_bo, bo, &cs->ws->global_bo_list, global_list_item) { if (addr >= bo->base.va && addr - bo->base.va < bo->size) { if (amdgpu_bo_cpu_map(bo->bo, &ret) == 0) { - pthread_mutex_unlock(&cs->ws->global_bo_list_lock); + pthread_rwlock_unlock(&cs->ws->global_bo_list_lock); return (char *)ret + (addr - bo->base.va); } } } - pthread_mutex_unlock(&cs->ws->global_bo_list_lock); + pthread_rwlock_unlock(&cs->ws->global_bo_list_lock); } return ret; } @@ -1774,9 +1794,6 @@ radv_amdgpu_cs_submit(struct radv_amdgpu_ctx *ctx, chunks, &request->seq_no); - if (bo_list) - amdgpu_bo_list_destroy_raw(ctx->ws->dev, bo_list); - if (r) { if (r == -ENOMEM) { fprintf(stderr, "amdgpu: Not enough memory for command submission.\n"); @@ -1791,6 +1808,9 @@ radv_amdgpu_cs_submit(struct radv_amdgpu_ctx *ctx, } } + if (bo_list) + amdgpu_bo_list_destroy_raw(ctx->ws->dev, bo_list); + error_out: if (in_syncobjs) { radv_amdgpu_cache_free_syncobjs(ctx->ws, sem_info->wait.syncobj_count, in_syncobjs); diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_winsys.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_winsys.c index c6deedc..ebcd39e 100644 --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_winsys.c +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_winsys.c @@ -166,6 +166,7 @@ static void radv_amdgpu_winsys_destroy(struct radeon_winsys *rws) amdgpu_cs_destroy_syncobj(ws->dev, ws->syncobj[i]); free(ws->syncobj); + pthread_rwlock_destroy(&ws->global_bo_list_lock); ac_addrlib_destroy(ws->addrlib); amdgpu_device_deinitialize(ws->dev); FREE(rws); @@ -200,7 +201,7 @@ radv_amdgpu_winsys_create(int fd, uint64_t debug_flags, uint64_t perftest_flags) ws->zero_all_vram_allocs = debug_flags & RADV_DEBUG_ZERO_VRAM; ws->use_llvm = debug_flags & RADV_DEBUG_LLVM; list_inithead(&ws->global_bo_list); - pthread_mutex_init(&ws->global_bo_list_lock, NULL); + pthread_rwlock_init(&ws->global_bo_list_lock, NULL); pthread_mutex_init(&ws->syncobj_lock, NULL); ws->base.query_info = radv_amdgpu_winsys_query_info; ws->base.query_value = radv_amdgpu_winsys_query_value; diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_winsys.h b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_winsys.h index bb8ce88..b2c5006 100644 --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_winsys.h +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_winsys.h @@ -49,7 +49,7 @@ struct radv_amdgpu_winsys { bool use_llvm; unsigned num_buffers; - pthread_mutex_t global_bo_list_lock; + pthread_rwlock_t global_bo_list_lock; struct list_head global_bo_list; uint64_t allocated_vram;