From 511013d3131bd6367129d684717057c2a889cb4e Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Thu, 27 Oct 2022 10:03:14 +0200 Subject: [PATCH] v3dv: drop layout refs for all allocated sets from a pool on destroy / reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In 7f6ecb8667c we added reference counting for descriptor set layouts, however, we didn't realize that pools created without the flag VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT don't free individual descriptors and can only be reset or destroyed. Since we only drop references when individual descriptor sets were destroyed, we would leak set layouts referenced from descriptor sets allocated from these pools. Fix that by keeping a list of all allocated descriptor sets (no matter whether VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT is present or not) and then traversing the list dropping the references on pool resets and destroys. Fixes: 7f6ecb8667c ('v3dv: add reference counting for descriptor set layouts') Reviewed-by: Alejandro Piñeiro Part-of: (cherry picked from commit 07c7d846e5fa6867ce20dd959b4609fd1a0f1646) --- .pick_status.json | 2 +- src/broadcom/vulkan/v3dv_descriptor_set.c | 26 +++++++++++++++++++---- src/broadcom/vulkan/v3dv_private.h | 6 ++++++ 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 7b00be0c0ab..7eefcc7e524 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -2740,7 +2740,7 @@ "description": "v3dv: drop layout refs for all allocated sets from a pool on destroy / reset", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "7f6ecb8667c6c756109954ad23f8c2c0ae0a9bf8" }, diff --git a/src/broadcom/vulkan/v3dv_descriptor_set.c b/src/broadcom/vulkan/v3dv_descriptor_set.c index 104ca0d19a3..8eff20985ca 100644 --- a/src/broadcom/vulkan/v3dv_descriptor_set.c +++ b/src/broadcom/vulkan/v3dv_descriptor_set.c @@ -504,6 +504,8 @@ v3dv_CreateDescriptorPool(VkDevice _device, pool->bo = NULL; } + list_inithead(&pool->set_list); + *pDescriptorPool = v3dv_descriptor_pool_to_handle(pool); return VK_SUCCESS; @@ -521,8 +523,6 @@ descriptor_set_destroy(struct v3dv_device *device, { assert(!pool->host_memory_base); - v3dv_descriptor_set_layout_unref(device, set->layout); - if (free_bo && !pool->host_memory_base) { for (uint32_t i = 0; i < pool->entry_count; i++) { if (pool->entries[i].set == set) { @@ -547,6 +547,11 @@ v3dv_DestroyDescriptorPool(VkDevice _device, if (!pool) return; + list_for_each_entry_safe(struct v3dv_descriptor_set, set, + &pool->set_list, pool_link) { + v3dv_descriptor_set_layout_unref(device, set->layout); + } + if (!pool->host_memory_base) { for(int i = 0; i < pool->entry_count; ++i) { descriptor_set_destroy(device, pool, pool->entries[i].set, false); @@ -569,6 +574,12 @@ v3dv_ResetDescriptorPool(VkDevice _device, V3DV_FROM_HANDLE(v3dv_device, device, _device); V3DV_FROM_HANDLE(v3dv_descriptor_pool, pool, descriptorPool); + list_for_each_entry_safe(struct v3dv_descriptor_set, set, + &pool->set_list, pool_link) { + v3dv_descriptor_set_layout_unref(device, set->layout); + } + list_inithead(&pool->set_list); + if (!pool->host_memory_base) { for(int i = 0; i < pool->entry_count; ++i) { descriptor_set_destroy(device, pool, pool->entries[i].set, false); @@ -898,6 +909,8 @@ descriptor_set_create(struct v3dv_device *device, } v3dv_descriptor_set_layout_ref(layout); + list_addtail(&set->pool_link, &pool->set_list); + *out_set = set; return VK_SUCCESS; @@ -948,8 +961,13 @@ v3dv_FreeDescriptorSets(VkDevice _device, for (uint32_t i = 0; i < count; i++) { V3DV_FROM_HANDLE(v3dv_descriptor_set, set, pDescriptorSets[i]); - if (set && !pool->host_memory_base) - descriptor_set_destroy(device, pool, set, true); + + if (set) { + v3dv_descriptor_set_layout_unref(device, set->layout); + list_del(&set->pool_link); + if (!pool->host_memory_base) + descriptor_set_destroy(device, pool, set, true); + } } return VK_SUCCESS; diff --git a/src/broadcom/vulkan/v3dv_private.h b/src/broadcom/vulkan/v3dv_private.h index a7a52cb9fa8..217b0461145 100644 --- a/src/broadcom/vulkan/v3dv_private.h +++ b/src/broadcom/vulkan/v3dv_private.h @@ -1667,6 +1667,9 @@ struct v3dv_descriptor_pool_entry struct v3dv_descriptor_pool { struct vk_object_base base; + /* A list with all descriptor sets allocated from the pool. */ + struct list_head set_list; + /* If this descriptor pool has been allocated for the driver for internal * use, typically to implement meta operations. */ @@ -1696,6 +1699,9 @@ struct v3dv_descriptor_pool { struct v3dv_descriptor_set { struct vk_object_base base; + /* List link into the list of all sets allocated from the pool */ + struct list_head pool_link; + struct v3dv_descriptor_pool *pool; struct v3dv_descriptor_set_layout *layout;