Mesa (master): mesa: fix Blender crash due to optimizations in buffer reference counting
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Wed Mar 17 23:14:17 UTC 2021
Module: Mesa
Branch: master
Commit: ae0ce3e3bae10b94c2e5d2f318203dfea5577eb2
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=ae0ce3e3bae10b94c2e5d2f318203dfea5577eb2
Author: Marek Olšák <marek.olsak at amd.com>
Date: Tue Mar 2 01:38:26 2021 -0500
mesa: fix Blender crash due to optimizations in buffer reference counting
The problem was that I assumed that deleted zombie buffers can't have any
references in the context, so buffers were released sooner than they should
have been.
The fix is to count the non-atomic references in the new field
gl_buffer_object::CtxRefCount. When we detach the context from the buffer,
we can just add CtxRefCount to RefCount to re-enable atomic reference
counting. This also allows removing code that was doing a similar thing.
Fixes: e014e3b6be63 "mesa: don't count buffer references for the context that created them"
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4259
Reviewed-by: Zoltán Böszörményi <zboszor at gmail.com>
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer at amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9360>
---
src/mesa/main/bufferobj.c | 141 ++++++++++++----------------------------------
src/mesa/main/mtypes.h | 1 +
2 files changed, 37 insertions(+), 105 deletions(-)
diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 52d9c4bdc62..b2a36a34756 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -518,10 +518,15 @@ _mesa_reference_buffer_object_(struct gl_context *ctx,
* 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);
+ if (shared_binding || ctx != oldObj->Ctx) {
+ if (p_atomic_dec_zero(&oldObj->RefCount)) {
+ assert(ctx->Driver.DeleteBuffer);
+ ctx->Driver.DeleteBuffer(ctx, oldObj);
+ }
+ } else if (ctx == oldObj->Ctx) {
+ /* Update the private ref count. */
+ assert(oldObj->CtxRefCount >= 1);
+ oldObj->CtxRefCount--;
}
*ptr = NULL;
@@ -532,6 +537,9 @@ _mesa_reference_buffer_object_(struct gl_context *ctx,
/* reference new buffer */
if (shared_binding || ctx != bufObj->Ctx)
p_atomic_inc(&bufObj->RefCount);
+ else if (ctx == bufObj->Ctx)
+ bufObj->CtxRefCount++;
+
*ptr = bufObj;
}
}
@@ -911,6 +919,27 @@ _mesa_init_buffer_objects( struct gl_context *ctx )
}
}
+/**
+ * Detach the context from the buffer to re-enable buffer reference counting
+ * for this context.
+ */
+static void
+detach_ctx_from_buffer(struct gl_context *ctx, struct gl_buffer_object *buf)
+{
+ assert(buf->Ctx == ctx);
+
+ /* Move private non-atomic context references to the global ref count. */
+ p_atomic_add(&buf->RefCount, buf->CtxRefCount);
+ buf->CtxRefCount = 0;
+ buf->Ctx = NULL;
+
+ /* 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.
+ */
+ _mesa_reference_buffer_object(ctx, &buf, NULL);
+}
+
/**
* Zombie buffers are buffers that were created by one context and deleted
* by another context. The creating context holds a global reference for each
@@ -932,8 +961,7 @@ unreference_zombie_buffers_for_ctx(struct gl_context *ctx)
if (buf->Ctx == ctx) {
_mesa_set_remove(ctx->Shared->ZombieBufferObjects, entry);
- buf->Ctx = NULL;
- _mesa_reference_buffer_object(ctx, &buf, NULL);
+ detach_ctx_from_buffer(ctx, buf);
}
}
}
@@ -960,6 +988,7 @@ detach_unrefcounted_buffer_from_ctx(void *data, void *userData)
* unreference the global reference. Other contexts and texture objects
* might still be using the buffer.
*/
+ assert(buf->CtxRefCount == 0);
buf->Ctx = NULL;
_mesa_reference_buffer_object(ctx, &buf, NULL);
}
@@ -1530,82 +1559,6 @@ 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.
*
@@ -1748,29 +1701,7 @@ delete_buffers(struct gl_context *ctx, GLsizei n, const GLuint *ids)
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;
+ detach_ctx_from_buffer(ctx, bufObj);
} else if (bufObj->Ctx) {
/* Only the context holding it can release it. */
_mesa_set_add(ctx->Shared->ZombieBufferObjects, bufObj);
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 58f58cea636..95da90828fb 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1427,6 +1427,7 @@ struct gl_buffer_object
* reference from those buffers that it created.
*/
struct gl_context *Ctx;
+ GLint CtxRefCount; /**< Non-atomic references held by Ctx. */
GLenum16 Usage; /**< GL_STREAM_DRAW_ARB, GL_STREAM_READ_ARB, etc. */
GLbitfield StorageFlags; /**< GL_MAP_PERSISTENT_BIT, etc. */
More information about the mesa-commit
mailing list