From e0b4dfbbda97d98f4065c9cf536032936401c8d0 Mon Sep 17 00:00:00 2001 From: Lionel Landwerlin Date: Fri, 29 Dec 2023 09:34:57 +0200 Subject: [PATCH] anv: don't unmap AUX ranges at BO delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is possible to free memory backing images before images are destroyed : VkFreeMemory: "Memory can be freed whilst still bound to resources, but those resources must not be used afterwards." The spec leaves us the option to keep a reference on the associated memory and free it only when all the bound resources have been destroyed. Here we choose to free memory immediately. One particular test in the CTS (dEQP-VK.synchronization.internally_synchronized_objects.pipeline_cache_graphics) does the following : imgA = vkCreateImage() imgB = vkCreateImage() memA = vkAllocateMemory() vkBindImageMemory(imgA, memA) # Aux mapping with ref count = 1 vkFreeMemory(memA) # Aux mapping removed, ref count = 0 memB = vkAllocateMemory() # Same address as memA vkBindImageMemory(imgB, memB) vkDestroyImage(imgA) # Removes the mapping of imgB-memB vkQueueSubmit() # hang with pagefault in AUX-TT The solution implemented in this change is to not do anything AUX-TT related in vkFreeMemory(). This soluation has some consequences, because a virtual memory address range freed and reallocated cannot be rebound in the AUX-TT until all the associated resources have released their AUX-TT mapping (to bring back the AUX-TT refcount of the range to 0). This should still be better than keeping the memory allocated through refcounting of the anv_bo. Signed-off-by: Lionel Landwerlin Fixes: 7b87e1afbc ("anv: track & unbind image aux-tt binding") Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/10528 Reviewed-by: Tapani Pälli Part-of: --- src/intel/vulkan/anv_allocator.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_allocator.c index 587c8a8804f..868f8ebd907 100644 --- a/src/intel/vulkan/anv_allocator.c +++ b/src/intel/vulkan/anv_allocator.c @@ -1910,12 +1910,6 @@ anv_device_release_bo(struct anv_device *device, } assert(bo->refcount == 0); - /* Unmap the entire BO. In the case that some addresses lacked an aux-map - * entry, the unmapping function will add table entries for them. - */ - if (anv_bo_allows_aux_map(device, bo)) - intel_aux_map_unmap_range(device->aux_map_ctx, bo->offset, bo->size); - /* Memset the BO just in case. The refcount being zero should be enough to * prevent someone from assuming the data is valid but it's safer to just * stomp to zero just in case. We explicitly do this *before* we actually