Module: Mesa Branch: master Commit: ae0ce3e3bae10b94c2e5d2f318203dfea5577eb2 URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=ae0ce3e3bae10b94c2e5d2f318203dfea5577eb2
Author: Marek Olšák <[email protected]> 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 <[email protected]> Reviewed-by: Pierre-Eric Pelloux-Prayer <[email protected]> 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. */ _______________________________________________ mesa-commit mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-commit
