From 6f5af78e25ad8942b3d7db8e4e89c801075dda67 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Mon, 11 Oct 2021 14:54:56 -0700 Subject: [PATCH] iris: improve error checking in functions that call vma_alloc() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On these functions, for vm_bind I want to add another function call that can fail and needs to be handled. As a preparation for that, add real error checks around vma_alloc() and try to fix some of the other error-related issues in these functions. This way, it will be much simpler to just drop the new vm_bind-related function calls and error handling code. My fear is that having vma-related errors (and in the future vm_bind-related errors) go unannounced may create some hard-to-debug bugs. v2: Unlock only after bo_free() (Marcin Ślusarz). v3: Prefer goto over early returns (Marcin Ślusarz). Reviewed-by: Marcin Ślusarz Reviewed-by: Kenneth Graunke Signed-off-by: Paulo Zanoni Part-of: --- src/gallium/drivers/iris/iris_bufmgr.c | 37 ++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/gallium/drivers/iris/iris_bufmgr.c b/src/gallium/drivers/iris/iris_bufmgr.c index f1758953d41..ebed227719e 100644 --- a/src/gallium/drivers/iris/iris_bufmgr.c +++ b/src/gallium/drivers/iris/iris_bufmgr.c @@ -1145,7 +1145,9 @@ iris_bo_alloc(struct iris_bufmgr *bufmgr, return bo; err_free: + simple_mtx_lock(&bufmgr->lock); bo_free(bo); + simple_mtx_unlock(&bufmgr->lock); return NULL; } @@ -1250,8 +1252,11 @@ iris_bo_gem_create_from_name(struct iris_bufmgr *bufmgr, goto out; bo = bo_calloc(); - if (!bo) + if (!bo) { + struct drm_gem_close close = { .handle = open_arg.handle, }; + intel_ioctl(bufmgr->fd, DRM_IOCTL_GEM_CLOSE, &close); goto out; + } p_atomic_set(&bo->refcount, 1); @@ -1266,6 +1271,12 @@ iris_bo_gem_create_from_name(struct iris_bufmgr *bufmgr, bo->real.kflags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS | EXEC_OBJECT_PINNED; bo->address = vma_alloc(bufmgr, IRIS_MEMZONE_OTHER, bo->size, 1); + if (bo->address == 0ull) { + bo_free(bo); + bo = NULL; + goto out; + } + _mesa_hash_table_insert(bufmgr->handle_table, &bo->gem_handle, bo); _mesa_hash_table_insert(bufmgr->name_table, &bo->real.global_name, bo); @@ -1879,6 +1890,7 @@ iris_bo_import_dmabuf(struct iris_bufmgr *bufmgr, int prime_fd) bo->real.imported = true; bo->real.mmap_mode = IRIS_MMAP_NONE; bo->real.kflags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS | EXEC_OBJECT_PINNED; + bo->gem_handle = handle; /* From the Bspec, Memory Compression - Gfx12: * @@ -1890,10 +1902,14 @@ iris_bo_import_dmabuf(struct iris_bufmgr *bufmgr, int prime_fd) * in case. We always align to 64KB even on platforms where we don't need * to, because it's a fairly reasonable thing to do anyway. */ - bo->address = - vma_alloc(bufmgr, IRIS_MEMZONE_OTHER, bo->size, 64 * 1024); + bo->address = vma_alloc(bufmgr, IRIS_MEMZONE_OTHER, bo->size, 64 * 1024); + + if (bo->address == 0ull) { + bo_free(bo); + bo = NULL; + goto out; + } - bo->gem_handle = handle; _mesa_hash_table_insert(bufmgr->handle_table, &bo->gem_handle, bo); out: @@ -2223,10 +2239,21 @@ intel_aux_map_buffer_alloc(void *driver_ctx, uint32_t size) size = MAX2(ALIGN(size, page_size), page_size); struct iris_bo *bo = alloc_fresh_bo(bufmgr, size, 0); + if (!bo) { + free(buf); + return NULL; + } simple_mtx_lock(&bufmgr->lock); + bo->address = vma_alloc(bufmgr, IRIS_MEMZONE_OTHER, bo->size, 64 * 1024); - assert(bo->address != 0ull); + if (bo->address == 0ull) { + free(buf); + bo_free(bo); + simple_mtx_unlock(&bufmgr->lock); + return NULL; + } + simple_mtx_unlock(&bufmgr->lock); bo->name = "aux-map";