mesa: don't count buffer references for the context that created them

See mtypes.h - it gives you the top level view of how it works, and other
code contains a lot of comments.

Reviewed-by: Zoltán Böszörményi <zboszor@gmail.com>
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8606>

src/mesa/main/bufferobj.c

@@ -47,6 +47,7 @@
 #include "varray.h"
 #include "util/u_atomic.h"
 #include "util/u_memory.h"
+#include "util/set.h"
 
 
 /* Debug flags */
@@ -506,13 +507,19 @@ _mesa_delete_buffer_object(struct gl_context *ctx,
 void
 _mesa_reference_buffer_object_(struct gl_context *ctx,
                                struct gl_buffer_object **ptr,
-                               struct gl_buffer_object *bufObj)
+                               struct gl_buffer_object *bufObj,
+                               bool shared_binding)
 {
    if (*ptr) {
       /* Unreference the old buffer */
       struct gl_buffer_object *oldObj = *ptr;
 
-      if (p_atomic_dec_zero(&oldObj->RefCount)) {
+      /* Count references only if the context doesn't own the buffer or if
+       * ptr is a binding point shared by multiple contexts (such as a texture
+       * buffer object being a buffer bound within a texture object).
+       */
+      if ((shared_binding || ctx != oldObj->Ctx) &&
+          p_atomic_dec_zero(&oldObj->RefCount)) {
 	 assert(ctx->Driver.DeleteBuffer);
          ctx->Driver.DeleteBuffer(ctx, oldObj);
       }
@@ -523,7 +530,8 @@ _mesa_reference_buffer_object_(struct gl_context *ctx,
 
    if (bufObj) {
       /* reference new buffer */
-      p_atomic_inc(&bufObj->RefCount);
+      if (shared_binding || ctx != bufObj->Ctx)
+         p_atomic_inc(&bufObj->RefCount);
       *ptr = bufObj;
    }
 }
@@ -903,6 +911,59 @@ _mesa_init_buffer_objects( struct gl_context *ctx )
    }
 }
 
+/**
+ * Zombie buffers are buffers that were created by one context and deleted
+ * by another context. The creating context holds a global reference for each
+ * buffer it created that can't be unreferenced when another context deletes
+ * it. Such a buffer becomes a zombie, which means that it's no longer usable
+ * by OpenGL, but the creating context still holds its global reference of
+ * the buffer. Only the creating context can remove the reference, which is
+ * what this function does.
+ *
+ * For all zombie buffers, decrement the reference count if the current
+ * context owns the buffer.
+ */
+static void
+unreference_zombie_buffers_for_ctx(struct gl_context *ctx)
+{
+   /* It's assumed that the mutex of Shared->BufferObjects is locked. */
+   set_foreach(ctx->Shared->ZombieBufferObjects, entry) {
+      struct gl_buffer_object *buf = (struct gl_buffer_object *)entry->key;
+
+      if (buf->Ctx == ctx) {
+         _mesa_set_remove(ctx->Shared->ZombieBufferObjects, entry);
+         buf->Ctx = NULL;
+         _mesa_reference_buffer_object(ctx, &buf, NULL);
+      }
+   }
+}
+
+/**
+ * When a context creates buffers, it holds a global buffer reference count
+ * for each buffer and doesn't update their RefCount. When the context is
+ * destroyed before the buffers are destroyed, the context must remove
+ * its global reference from the buffers, so that the buffers can live
+ * on their own.
+ *
+ * At this point, the buffers shouldn't be bound in any bounding point owned
+ * by the context. (it would crash if they did)
+ */
+static void
+detach_unrefcounted_buffer_from_ctx(void *data, void *userData)
+{
+   struct gl_context *ctx = (struct gl_context *)userData;
+   struct gl_buffer_object *buf = (struct gl_buffer_object *)data;
+
+   if (buf->Ctx == ctx) {
+      /* Detach the current context from live objects. There should be no
+       * bound buffer in the context at this point, therefore we can just
+       * unreference the global reference. Other contexts and texture objects
+       * might still be using the buffer.
+       */
+      buf->Ctx = NULL;
+      _mesa_reference_buffer_object(ctx, &buf, NULL);
+   }
+}
 
 void
 _mesa_free_buffer_objects( struct gl_context *ctx )
@@ -946,6 +1007,28 @@ _mesa_free_buffer_objects( struct gl_context *ctx )
 				    NULL);
    }
 
+   _mesa_HashLockMutex(ctx->Shared->BufferObjects);
+   unreference_zombie_buffers_for_ctx(ctx);
+   _mesa_HashWalkLocked(ctx->Shared->BufferObjects,
+                        detach_unrefcounted_buffer_from_ctx, ctx);
+   _mesa_HashUnlockMutex(ctx->Shared->BufferObjects);
+}
+
+/**
+ * Create a buffer object that will be backed by an OpenGL buffer ID
+ * where the creating context will hold one global buffer reference instead
+ * of updating buffer RefCount for every binding point.
+ *
+ * This shouldn't be used for internal buffers.
+ */
+static struct gl_buffer_object *
+new_gl_buffer_object(struct gl_context *ctx, GLuint id)
+{
+   struct gl_buffer_object *buf = ctx->Driver.NewBufferObject(ctx, id);
+
+   buf->Ctx = ctx;
+   buf->RefCount++; /* global buffer reference held by the context */
+   return buf;
 }
 
 bool
@@ -965,7 +1048,7 @@ _mesa_handle_bind_buffer_gen(struct gl_context *ctx,
       /* If this is a new buffer object id, or one which was generated but
        * never used before, allocate a buffer object now.
        */
-      *buf_handle = ctx->Driver.NewBufferObject(ctx, buffer);
+      *buf_handle = new_gl_buffer_object(ctx, buffer);
       if (!*buf_handle) {
 	 _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", caller);
 	 return false;
@@ -1447,6 +1530,82 @@ bind_buffer_base_atomic_buffer(struct gl_context *ctx,
       bind_atomic_buffer(ctx, index, bufObj, 0, 0, GL_TRUE);
 }
 
+struct hash_walk_input {
+   struct gl_context *ctx;
+   struct gl_buffer_object *buf;
+};
+
+/**
+ * When a buffer ID is deleted, the buffer is unbound from the current VAO
+ * but stays bound in other VAOs until they are deleted too.
+ * When the context owns the buffer reference globally, it doesn't count
+ * buffer references for the context. When the buffer is deleted, the context
+ * gives up its ownership, and thus it has to increase the buffer reference
+ * count for each binding point where the buffer is bound.
+ */
+static void
+detach_ctx_from_vao_buffers(void *data, void *user)
+{
+   struct gl_context *ctx = ((struct hash_walk_input *)user)->ctx;
+   struct gl_buffer_object *buf = ((struct hash_walk_input *)user)->buf;
+   struct gl_vertex_array_object *vao = (struct gl_vertex_array_object *)data;
+
+   /* The buffer should already be unbound in the currently-bound VAO. */
+   if (vao == ctx->Array.VAO)
+      return;
+
+   /* Increase the reference count for each binding point where the buffer is
+    * bound.
+    */
+   unsigned add_count = 0;
+
+   if (vao->IndexBufferObj == buf)
+      add_count++;
+
+   for (unsigned i = 0; i < ARRAY_SIZE(vao->BufferBinding); i++) {
+      if (vao->BufferBinding[i].BufferObj == buf)
+         add_count++;
+   }
+
+   if (add_count)
+      p_atomic_add(&buf->RefCount, add_count);
+}
+
+/**
+ * When a buffer ID is deleted, the buffer is unbound from the current
+ * transform feedback object but stays bound in other TFBs until they are
+ * deleted too.
+ *
+ * When the context owns the buffer reference globally, it doesn't count
+ * buffer references for the context. When the buffer is deleted, the context
+ * gives up its ownership, and thus it has to increase the buffer reference
+ * count for each binding point where the buffer is bound.
+ */
+static void
+detach_ctx_from_tfb_buffers(void *data, void *user)
+{
+   struct gl_context *ctx = ((struct hash_walk_input *)user)->ctx;
+   struct gl_buffer_object *buf = ((struct hash_walk_input *)user)->buf;
+   struct gl_transform_feedback_object *tfb =
+      (struct gl_transform_feedback_object *)data;
+
+   /* The buffer should already be unbound in the currently-bound TFB. */
+   if (tfb == ctx->TransformFeedback.CurrentObject)
+      return;
+
+   /* Increase the reference count for each binding point where the buffer is
+    * bound.
+    */
+   unsigned add_count = 0;
+   for (unsigned i = 0; i < ARRAY_SIZE(tfb->Buffers); i++) {
+      if (tfb->Buffers[i] == buf)
+         add_count++;
+   }
+
+   if (add_count)
+      p_atomic_add(&buf->RefCount, add_count);
+}
+
 /**
  * Delete a set of buffer objects.
  *
@@ -1460,6 +1619,7 @@ delete_buffers(struct gl_context *ctx, GLsizei n, const GLuint *ids)
 
    _mesa_HashLockMaybeLocked(ctx->Shared->BufferObjects,
                              ctx->BufferObjectsLocked);
+   unreference_zombie_buffers_for_ctx(ctx);
 
    for (GLsizei i = 0; i < n; i++) {
       struct gl_buffer_object *bufObj =
@@ -1586,6 +1746,36 @@ delete_buffers(struct gl_context *ctx, GLsizei n, const GLuint *ids)
           * which would introduce more runtime overhead than this.
           */
          bufObj->DeletePending = GL_TRUE;
+
+         if (bufObj->Ctx == ctx) {
+            struct hash_walk_input input = {ctx, bufObj};
+
+            /* Increase reference counts for all binding points where
+             * the buffer remains bound after deleting. This is needed to
+             * enable reference counting for the buffer for this context
+             * because the context only holds the global buffer reference
+             * while the GL buffer ID exists.
+             */
+            _mesa_HashWalk(ctx->Array.Objects,
+                           detach_ctx_from_vao_buffers, &input);
+            detach_ctx_from_vao_buffers(ctx->Array.DefaultVAO, &input);
+
+            _mesa_HashWalk(ctx->TransformFeedback.Objects,
+                           detach_ctx_from_tfb_buffers, &input);
+            detach_ctx_from_tfb_buffers(ctx->TransformFeedback.DefaultObject,
+                                        &input);
+
+            /* Remove the context reference where the context holds one
+             * reference for the lifetime of the buffer ID to skip refcount
+             * atomics instead of each binding point holding the reference.
+             */
+            p_atomic_dec(&bufObj->RefCount);
+            bufObj->Ctx = NULL;
+         } else if (bufObj->Ctx) {
+            /* Only the context holding it can release it. */
+            _mesa_set_add(ctx->Shared->ZombieBufferObjects, bufObj);
+         }
+
          _mesa_reference_buffer_object(ctx, &bufObj, NULL);
       }
    }
@@ -1645,7 +1835,7 @@ create_buffers(struct gl_context *ctx, GLsizei n, GLuint *buffers, bool dsa)
    for (int i = 0; i < n; i++) {
       if (dsa) {
          assert(ctx->Driver.NewBufferObject);
-         buf = ctx->Driver.NewBufferObject(ctx, buffers[i]);
+         buf = new_gl_buffer_object(ctx, buffers[i]);
          if (!buf) {
             _mesa_error(ctx, GL_OUT_OF_MEMORY, "glCreateBuffers");
             _mesa_HashUnlockMaybeLocked(ctx->Shared->BufferObjects,

src/mesa/main/bufferobj.h

@@ -105,15 +105,33 @@ _mesa_delete_buffer_object(struct gl_context *ctx,
 extern void
 _mesa_reference_buffer_object_(struct gl_context *ctx,
                                struct gl_buffer_object **ptr,
-                               struct gl_buffer_object *bufObj);
+                               struct gl_buffer_object *bufObj,
+                               bool shared_binding);
 
+/**
+ * Assign a buffer into a pointer with reference counting. The destination
+ * must be private within a context.
+ */
 static inline void
 _mesa_reference_buffer_object(struct gl_context *ctx,
                               struct gl_buffer_object **ptr,
                               struct gl_buffer_object *bufObj)
 {
    if (*ptr != bufObj)
-      _mesa_reference_buffer_object_(ctx, ptr, bufObj);
+      _mesa_reference_buffer_object_(ctx, ptr, bufObj, false);
+}
+
+/**
+ * Assign a buffer into a pointer with reference counting. The destination
+ * must be shareable among multiple contexts.
+ */
+static inline void
+_mesa_reference_buffer_object_shared(struct gl_context *ctx,
+                                     struct gl_buffer_object **ptr,
+                                     struct gl_buffer_object *bufObj)
+{
+   if (*ptr != bufObj)
+      _mesa_reference_buffer_object_(ctx, ptr, bufObj, true);
 }
 
 extern GLuint

src/mesa/main/context.c

@@ -1346,7 +1346,6 @@ _mesa_free_context_data(struct gl_context *ctx, bool destroy_debug_output)
    _mesa_reference_vao(ctx, &ctx->Array._DrawVAO, NULL);
 
    _mesa_free_attrib_data(ctx);
-   _mesa_free_buffer_objects(ctx);
    _mesa_free_eval_data( ctx );
    _mesa_free_texture_data( ctx );
    _mesa_free_image_textures(ctx);
@@ -1367,6 +1366,11 @@ _mesa_free_context_data(struct gl_context *ctx, bool destroy_debug_output)
    _mesa_reference_buffer_object(ctx, &ctx->DefaultPacking.BufferObj, NULL);
    _mesa_reference_buffer_object(ctx, &ctx->Array.ArrayBufferObj, NULL);
 
+   /* This must be called after all buffers are unbound because global buffer
+    * references that this context holds will be removed.
+    */
+   _mesa_free_buffer_objects(ctx);
+
    /* free dispatch tables */
    free(ctx->BeginEnd);
    free(ctx->OutsideBeginEnd);

src/mesa/main/mtypes.h

@@ -1402,6 +1402,34 @@ struct gl_buffer_object
    GLint RefCount;
    GLuint Name;
    GLchar *Label;       /**< GL_KHR_debug */
+
+   /**
+    * The context that holds a global buffer reference for the lifetime of
+    * the GL buffer ID to skip refcounting for all its private bind points.
+    * Other contexts must still do refcounting as usual. Shared binding points
+    * like TBO within gl_texture_object are always refcounted.
+    *
+    * Implementation details:
+    * - Only the context that creates the buffer ("creating context") skips
+    *   refcounting.
+    * - Only buffers represented by an OpenGL buffer ID skip refcounting.
+    *   Other internal buffers don't. (glthread requires refcounting for
+    *   internal buffers, etc.)
+    * - glDeleteBuffers removes the global buffer reference and increments
+    *   RefCount for all private bind points where the deleted buffer is bound
+    *   (e.g. unbound VAOs that are not changed by glDeleteBuffers),
+    *   effectively enabling refcounting for that context. This is the main
+    *   point where the global buffer reference is removed.
+    * - glDeleteBuffers called from a different context adds the buffer into
+    *   the ZombieBufferObjects list, which is a way to notify the creating
+    *   context that it should remove its global buffer reference to allow
+    *   freeing the buffer. The creating context walks over that list in a few
+    *   GL functions.
+    * - xxxDestroyContext walks over all buffers and removes its global
+    *   reference from those buffers that it created.
+    */
+   struct gl_context *Ctx;
+
    GLenum16 Usage;      /**< GL_STREAM_DRAW_ARB, GL_STREAM_READ_ARB, etc. */
    GLbitfield StorageFlags; /**< GL_MAP_PERSISTENT_BIT, etc. */
    GLsizeiptrARB Size;  /**< Size of buffer storage in bytes */
@@ -3401,6 +3429,19 @@ struct gl_shared_state
 
    struct _mesa_HashTable *BufferObjects;
 
+   /* Buffer objects released by a different context than the one that
+    * created them. Since the creating context holds one global buffer
+    * reference for each buffer it created and skips reference counting,
+    * deleting a buffer by another context can't touch the buffer reference
+    * held by the context that created it. Only the creating context can
+    * remove its global buffer reference.
+    *
+    * This list contains all buffers that were deleted by a different context
+    * than the one that created them. This list should be probed by all
+    * contexts regularly and remove references of those buffers that they own.
+    */
+   struct set *ZombieBufferObjects;
+
    /** Table of both gl_shader and gl_shader_program objects */
    struct _mesa_HashTable *ShaderObjects;
 

src/mesa/main/shared.c

@@ -85,6 +85,8 @@ _mesa_alloc_shared_state(struct gl_context *ctx)
    shared->ShaderObjects = _mesa_NewHashTable();
 
    shared->BufferObjects = _mesa_NewHashTable();
+   shared->ZombieBufferObjects = _mesa_set_create(NULL, _mesa_hash_pointer,
+                                                  _mesa_key_pointer_equal);
 
    /* GL_ARB_sampler_objects */
    shared->SamplerObjects = _mesa_NewHashTable();
@@ -391,6 +393,13 @@ free_shared_state(struct gl_context *ctx, struct gl_shared_state *shared)
       _mesa_DeleteHashTable(shared->BufferObjects);
    }
 
+   if (shared->ZombieBufferObjects) {
+      set_foreach(shared->ZombieBufferObjects, entry) {
+         assert(!"ZombieBufferObjects should be empty");
+      }
+      _mesa_set_destroy(shared->ZombieBufferObjects, NULL);
+   }
+
    if (shared->FrameBuffers) {
       _mesa_HashDeleteAll(shared->FrameBuffers, delete_framebuffer_cb, ctx);
       _mesa_DeleteHashTable(shared->FrameBuffers);

src/mesa/main/teximage.c

@@ -6335,7 +6335,7 @@ texture_buffer_range(struct gl_context *ctx,
 
    _mesa_lock_texture(ctx, texObj);
    {
-      _mesa_reference_buffer_object(ctx, &texObj->BufferObject, bufObj);
+      _mesa_reference_buffer_object_shared(ctx, &texObj->BufferObject, bufObj);
       texObj->BufferObjectFormat = internalFormat;
       texObj->_BufferObjectFormat = format;
       texObj->BufferOffset = offset;

src/mesa/main/texobj.c

@@ -447,7 +447,7 @@ _mesa_delete_texture_object(struct gl_context *ctx,
    /* Delete all texture/image handles. */
    _mesa_delete_texture_handles(ctx, texObj);
 
-   _mesa_reference_buffer_object(ctx, &texObj->BufferObject, NULL);
+   _mesa_reference_buffer_object_shared(ctx, &texObj->BufferObject, NULL);
 
    /* destroy the mutex -- it may have allocated memory (eg on bsd) */
    simple_mtx_destroy(&texObj->Mutex);