Re: [Mesa-dev] [PATCH v2 4/5] anv: Add missing error-checking to anv_CreateDevice (v2)

[Emil Velikov]

Hi Mun Gwan-gyeong,

There is a minor comment below while the rest of your patches are in master.
Thanks!

On 26 November 2016 at 07:29, Mun Gwan-gyeong  wrote:

> @@ -942,7 +968,7 @@ VkResult anv_CreateDevice(
>unreachable("unhandled gen");
> }
> if (result != VK_SUCCESS)
> -  goto fail_fd;
> +  goto fail_workaround_bo;
>
> anv_device_init_blorp(device);
>
> @@ -952,6 +978,25 @@ VkResult anv_CreateDevice(
>
> return VK_SUCCESS;
>
> + fail_workaround_bo:
The following two seems to be missing yet I haven't checked if/how
much it matters.

   anv_queue_finish(>queue);
   anv_scratch_pool_finish(device, >scratch_pool);


-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 4/5] anv: Add missing error-checking to anv_CreateDevice (v2)

[Mun Gwan-gyeong]

This patch adds missing error-checking and fixes resource leak in
allocation failure path on anv_CreateDevice()

v2: Fixes from Jason Ekstrand's review
  a) Add missing destructors for all of the state pools on allocation
 failure path
  b) Add missing destructor for batch bo pools on allocation failure path

Signed-off-by: Mun Gwan-gyeong 
---
 src/intel/vulkan/anv_device.c | 63 ---
 1 file changed, 54 insertions(+), 9 deletions(-)

diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 0fd7d41..768e8f9 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -893,31 +893,57 @@ VkResult anv_CreateDevice(
device->robust_buffer_access = pCreateInfo->pEnabledFeatures &&
   pCreateInfo->pEnabledFeatures->robustBufferAccess;
 
-   pthread_mutex_init(>mutex, NULL);
+   if (pthread_mutex_init(>mutex, NULL) != 0) {
+  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
+  goto fail_context_id;
+   }
 
pthread_condattr_t condattr;
-   pthread_condattr_init();
-   pthread_condattr_setclock(, CLOCK_MONOTONIC);
-   pthread_cond_init(>queue_submit, NULL);
+   if (pthread_condattr_init() != 0) {
+  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
+  goto fail_mutex;
+   }
+   if (pthread_condattr_setclock(, CLOCK_MONOTONIC) != 0) {
+  pthread_condattr_destroy();
+  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
+  goto fail_mutex;
+   }
+   if (pthread_cond_init(>queue_submit, NULL) != 0) {
+  pthread_condattr_destroy();
+  result = vk_error(VK_ERROR_INITIALIZATION_FAILED);
+  goto fail_mutex;
+   }
pthread_condattr_destroy();
 
anv_bo_pool_init(>batch_bo_pool, device);
 
-   anv_block_pool_init(>dynamic_state_block_pool, device, 16384);
+   result = anv_block_pool_init(>dynamic_state_block_pool, device,
+16384);
+   if (result != VK_SUCCESS)
+  goto fail_batch_bo_pool;
 
anv_state_pool_init(>dynamic_state_pool,
>dynamic_state_block_pool);
 
-   anv_block_pool_init(>instruction_block_pool, device, 128 * 1024);
+   result = anv_block_pool_init(>instruction_block_pool, device,
+128 * 1024);
+   if (result != VK_SUCCESS)
+  goto fail_dynamic_state_block_pool;
+
anv_state_pool_init(>instruction_state_pool,
>instruction_block_pool);
 
-   anv_block_pool_init(>surface_state_block_pool, device, 4096);
+   result = anv_block_pool_init(>surface_state_block_pool, device,
+4096);
+   if (result != VK_SUCCESS)
+  goto fail_instruction_block_pool;
 
anv_state_pool_init(>surface_state_pool,
>surface_state_block_pool);
 
-   anv_bo_init_new(>workaround_bo, device, 1024);
+   result = anv_bo_init_new(>workaround_bo, device, 1024);
+   if (result != VK_SUCCESS)
+  goto fail_surface_state_block_pool;
 
anv_scratch_pool_init(device, >scratch_pool);
 
@@ -942,7 +968,7 @@ VkResult anv_CreateDevice(
   unreachable("unhandled gen");
}
if (result != VK_SUCCESS)
-  goto fail_fd;
+  goto fail_workaround_bo;
 
anv_device_init_blorp(device);
 
@@ -952,6 +978,25 @@ VkResult anv_CreateDevice(
 
return VK_SUCCESS;
 
+ fail_workaround_bo:
+   anv_gem_munmap(device->workaround_bo.map, device->workaround_bo.size);
+   anv_gem_close(device, device->workaround_bo.gem_handle);
+ fail_surface_state_block_pool:
+   anv_state_pool_finish(>surface_state_pool);
+   anv_block_pool_finish(>surface_state_block_pool);
+ fail_instruction_block_pool:
+   anv_state_pool_finish(>instruction_state_pool);
+   anv_block_pool_finish(>instruction_block_pool);
+ fail_dynamic_state_block_pool:
+   anv_state_pool_finish(>dynamic_state_pool);
+   anv_block_pool_finish(>dynamic_state_block_pool);
+ fail_batch_bo_pool:
+   anv_bo_pool_finish(>batch_bo_pool);
+   pthread_cond_destroy(>queue_submit);
+ fail_mutex:
+   pthread_mutex_destroy(>mutex);
+ fail_context_id:
+   anv_gem_destroy_context(device, device->context_id);
  fail_fd:
close(device->fd);
  fail_device:
-- 
2.10.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev