Module: Mesa
Branch: main
Commit: e642cafdae68ef407f9aea1cb7f48b18a956a8ee
URL:    
http://cgit.freedesktop.org/mesa/mesa/commit/?id=e642cafdae68ef407f9aea1cb7f48b18a956a8ee

Author: Paulo Zanoni <[email protected]>
Date:   Fri Jan 13 16:32:59 2023 -0800

anv: run buf_finish() if add_bo() fails during execute_simple_batch()

This is the only code path where we don't run anv_execbuf_finish() in
case anv_execbuf_add_bo() fails. While there is not a bug in the
current tree, I recently made an (uncommitted) modification that
started leaking memory and made me realize the lack of cleanup here.
If we had anv_execbuf_finish() being called upon error like we're
going to have after this patch my modification wouldn't have caused
the memory leak.

I think it's much safer and future-proof if we're able to operate
under the assumption that whatever is allocated and set to anv_execbuf
will be dealt with upon failure of anything else related to it, so
functions that fail should only be required to free pointers not yet
assigned to anv_execbuf.

The dEQP-VK 'alloc_callback_fail' tests should exercise this code
path. The one I was specifically using here is:
  dEQP-VK.api.object_management.alloc_callback_fail.device_group

v2: Rebase.

Reviewed-by: Ivan Briano <[email protected]>
Reviewed-by: Lionel Landwerlin <[email protected]>
Signed-off-by: Paulo Zanoni <[email protected]>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20703>

---

 src/intel/vulkan/i915/anv_batch_chain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/intel/vulkan/i915/anv_batch_chain.c 
b/src/intel/vulkan/i915/anv_batch_chain.c
index 705c5a0e7ee..f9cf800e106 100644
--- a/src/intel/vulkan/i915/anv_batch_chain.c
+++ b/src/intel/vulkan/i915/anv_batch_chain.c
@@ -770,7 +770,7 @@ anv_i915_execute_simple_batch(struct anv_queue *queue,
 
    VkResult result = anv_execbuf_add_bo(device, &execbuf, batch_bo, NULL, 0);
    if (result != VK_SUCCESS)
-      return result;
+      goto fail;
 
    execbuf.execbuf = (struct drm_i915_gem_execbuffer2) {
       .buffers_ptr = (uintptr_t) execbuf.objects,

Reply via email to