From 015835fc598ac1f33f27ffd6245008535a23be4b Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Mon, 10 Feb 2025 13:54:54 +0100 Subject: [PATCH] panvk/csf: Don't free the resources twice when init_render_desc_ringbuf() fails init_queue() calls cleanup_queue() if anything fails in the middle, which means finish_render_desc_ringbuf() will be automatically called if init_render_desc_ringbuf() failed. Get rid of the the error path and return directly instead. The one exception we have is the dev_addr allocation, which needs to be explicitly freed if an error occurs between util_vma_heap_alloc() and pan_kmod_vm_bind(). Reported-by: Erik Faye-Lund Fixes: 5544d39f4420 ("panvk: Add a CSF backend for panvk_queue/cmd_buffer") Signed-off-by: Boris Brezillon Reviewed-by: Lars-Ivar Hesselberg Simonsen Part-of: (cherry picked from commit 5f3c6a0f272733ae226a9cc0f0d551b081961b0d) --- .pick_status.json | 2 +- src/panfrost/vulkan/csf/panvk_vX_queue.c | 44 ++++++++---------------- 2 files changed, 15 insertions(+), 31 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 62012118de6..150bc62ced4 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -324,7 +324,7 @@ "description": "panvk/csf: Don't free the resources twice when init_render_desc_ringbuf() fails", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "5544d39f4420da88c53aaf8dd48d86ac92bd0eaa", "notes": null diff --git a/src/panfrost/vulkan/csf/panvk_vX_queue.c b/src/panfrost/vulkan/csf/panvk_vX_queue.c index e44d311d4a7..b40219c9a0a 100644 --- a/src/panfrost/vulkan/csf/panvk_vX_queue.c +++ b/src/panfrost/vulkan/csf/panvk_vX_queue.c @@ -80,7 +80,6 @@ init_render_desc_ringbuf(struct panvk_queue *queue) uint32_t flags = panvk_device_adjust_bo_flags(dev, PAN_KMOD_BO_FLAG_NO_MMAP); struct panvk_desc_ringbuf *ringbuf = &queue->render_desc_ringbuf; uint64_t dev_addr = 0; - VkResult result; int ret; if (tracing_enabled) { @@ -103,11 +102,9 @@ init_render_desc_ringbuf(struct panvk_queue *queue) ringbuf->addr.host = pan_kmod_bo_mmap(ringbuf->bo, 0, ringbuf->size, PROT_READ | PROT_WRITE, MAP_SHARED, NULL); - if (ringbuf->addr.host == MAP_FAILED) { - result = panvk_errorf(dev, VK_ERROR_OUT_OF_HOST_MEMORY, - "Failed to CPU map ringbuf BO"); - goto err_finish_ringbuf; - } + if (ringbuf->addr.host == MAP_FAILED) + return panvk_errorf(dev, VK_ERROR_OUT_OF_HOST_MEMORY, + "Failed to CPU map ringbuf BO"); } /* We choose the alignment to guarantee that we won't ever cross a 4G @@ -118,12 +115,9 @@ init_render_desc_ringbuf(struct panvk_queue *queue) util_vma_heap_alloc(&dev->as.heap, ringbuf->size * 2, ringbuf->size * 2); simple_mtx_unlock(&dev->as.lock); - if (!dev_addr) { - result = - panvk_errorf(dev, VK_ERROR_OUT_OF_DEVICE_MEMORY, - "Failed to allocate virtual address for ringbuf BO"); - goto err_finish_ringbuf; - } + if (!dev_addr) + return panvk_errorf(dev, VK_ERROR_OUT_OF_DEVICE_MEMORY, + "Failed to allocate virtual address for ringbuf BO"); struct pan_kmod_vm_op vm_ops[] = { { @@ -155,9 +149,11 @@ init_render_desc_ringbuf(struct panvk_queue *queue) ret = pan_kmod_vm_bind(dev->kmod.vm, PAN_KMOD_VM_OP_MODE_IMMEDIATE, vm_ops, tracing_enabled ? 1 : ARRAY_SIZE(vm_ops)); if (ret) { - result = panvk_errorf(dev, VK_ERROR_OUT_OF_DEVICE_MEMORY, - "Failed to GPU map ringbuf BO"); - goto err_finish_ringbuf; + simple_mtx_lock(&dev->as.lock); + util_vma_heap_free(&dev->as.heap, dev_addr, ringbuf->size * 2); + simple_mtx_unlock(&dev->as.lock); + return panvk_errorf(dev, VK_ERROR_OUT_OF_DEVICE_MEMORY, + "Failed to GPU map ringbuf BO"); } ringbuf->addr.dev = dev_addr; @@ -180,27 +176,15 @@ init_render_desc_ringbuf(struct panvk_queue *queue) struct panvk_cs_sync32 *syncobj = panvk_priv_mem_host_addr(ringbuf->syncobj); - if (!syncobj) { - result = panvk_errorf(dev, VK_ERROR_OUT_OF_DEVICE_MEMORY, - "Failed to create the render desc ringbuf context"); - goto err_finish_ringbuf; - } + if (!syncobj) + return panvk_errorf(dev, VK_ERROR_OUT_OF_DEVICE_MEMORY, + "Failed to create the render desc ringbuf context"); *syncobj = (struct panvk_cs_sync32){ .seqno = RENDER_DESC_RINGBUF_SIZE, }; return VK_SUCCESS; - -err_finish_ringbuf: - if (dev_addr && !ringbuf->addr.dev) { - simple_mtx_lock(&dev->as.lock); - util_vma_heap_free(&dev->as.heap, dev_addr, ringbuf->size * 2); - simple_mtx_unlock(&dev->as.lock); - } - - finish_render_desc_ringbuf(queue); - return result; } static void