winsys/amdgpu: fix a race between import and destroy

amdgpu_bo_destroy is called when the bo ref count reaches 0.  But if the
bo is on bo_export_table, amdgpu_bo_from_handle can race with
amdgpu_bo_destroy and increments the bo ref count.  When that happens,
amdgpu_bo_destroy should bail.

v2:
 - reorder amdgpu_bo_free and amdgpu_bo_unmap
 - fix an assert

Reviewed-by: Marek Olšák <marek.olsak@amd.com> (v1)
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24526>

diff --git a/src/gallium/auxiliary/pipebuffer/pb_buffer.h b/src/gallium/auxiliary/pipebuffer/pb_buffer.h
index 4955846..ea153f2 100644 (file)
--- a/src/gallium/auxiliary/pipebuffer/pb_buffer.h
+++ b/src/gallium/auxiliary/pipebuffer/pb_buffer.h
@@ -255,7 +255,11 @@ pb_destroy(void *winsys, struct pb_buffer *buf)
    assert(buf);
    if (!buf)
       return;
-   assert(!pipe_is_referenced(&buf->reference));
+
+   /* we can't assert(!pipe_is_referenced(&buf->reference)) because the winsys
+    * might have means to revive a buf whose refcount reaches 0, such as when
+    * destroy and import race against each other
+    */
    buf->vtbl->destroy(winsys, buf);
 }
 

diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
index be79533..1e77431 100644 (file)
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
@@ -154,12 +154,31 @@ void amdgpu_bo_destroy(struct amdgpu_winsys *ws, struct pb_buffer *_buf)
 
    assert(bo->bo && "must not be called for slab entries");
 
+   simple_mtx_lock(&ws->bo_export_table_lock);
+
+   /* amdgpu_bo_from_handle might have revived the bo */
+   if (p_atomic_read(&bo->base.reference.count)) {
+      simple_mtx_unlock(&ws->bo_export_table_lock);
+      return;
+   }
+
+   _mesa_hash_table_remove_key(ws->bo_export_table, bo->bo);
+
+   if (bo->base.placement & RADEON_DOMAIN_VRAM_GTT) {
+      amdgpu_bo_va_op(bo->bo, 0, bo->base.size, bo->va, 0, AMDGPU_VA_OP_UNMAP);
+      amdgpu_va_range_free(bo->u.real.va_handle);
+   }
+
+   simple_mtx_unlock(&ws->bo_export_table_lock);
+
    if (!bo->u.real.is_user_ptr && bo->u.real.cpu_ptr) {
       bo->u.real.cpu_ptr = NULL;
       amdgpu_bo_unmap(&ws->dummy_ws.base, &bo->base);
    }
    assert(bo->u.real.is_user_ptr || bo->u.real.map_count == 0);
 
+   amdgpu_bo_free(bo->bo);
+
 #if DEBUG
    if (ws->debug_all_bos) {
       simple_mtx_lock(&ws->global_bo_list_lock);
@@ -187,16 +206,6 @@ void amdgpu_bo_destroy(struct amdgpu_winsys *ws, struct pb_buffer *_buf)
    }
    simple_mtx_unlock(&ws->sws_list_lock);
 
-   simple_mtx_lock(&ws->bo_export_table_lock);
-   _mesa_hash_table_remove_key(ws->bo_export_table, bo->bo);
-   simple_mtx_unlock(&ws->bo_export_table_lock);
-
-   if (bo->base.placement & RADEON_DOMAIN_VRAM_GTT) {
-      amdgpu_bo_va_op(bo->bo, 0, bo->base.size, bo->va, 0, AMDGPU_VA_OP_UNMAP);
-      amdgpu_va_range_free(bo->u.real.va_handle);
-   }
-   amdgpu_bo_free(bo->bo);
-
    amdgpu_bo_remove_fences(bo);
 
    if (bo->base.placement & RADEON_DOMAIN_VRAM)