[Mesa-dev] [PATCH 12/13] mesa: Remove unnecessary locking from container objects.
Ian Romanick
idr at freedesktop.org
Fri Aug 7 09:54:47 PDT 2015
On 08/07/2015 09:50 AM, Fredrik Höglund wrote:
> On Friday 07 August 2015, Matt Turner wrote:
>> "Container objects" like vertex array objects, framebuffer objects, and
>> pipeline objects are not shared between contexts, so they require no
>> locking.
>
> Unfortunately it's not quite that simple when it comes to FBO's;
> EXT_framebuffer_object defines FBO's to be shared, while
> ARB_framebuffer_object defines them to not be shared. There is a
> note about this in section 2.6 of the OpenGL specification.
>
> I see three possible solutions:
>
> 1) Use separate hash tables and namespaces for EXT_fbo and
> ARB_fbo. The spec says that passing an FBO created by an
> EXT command to an ARB command and vice-versa results in
> undefined behavior, so this is legal. But it doesn't appear
> to match what the proprietary drivers are doing, so some
> applications may break as a result.
I seem to recall encountering some Valve games (maybe TF2?) that were
mixing ARB and EXT functions. It seems like they were using EXT for
most things, but using some functions that were added by the ARB
extension. It was quite a long time ago, so my memory is pretty fuzzy.
> 2) Make FBO's non-shared in core contexts, where we don't expose
> EXT_fbo.
That's an easy compromise that I hadn't considered. I like it.
> 3) Keep doing what we're doing now, i.e. allow all FBO's to be shared.
>
>> ---
>> src/mesa/main/arrayobj.c | 6 ------
>> src/mesa/main/fbobject.c | 9 ---------
>> src/mesa/main/framebuffer.c | 9 ---------
>> src/mesa/main/mtypes.h | 5 -----
>> src/mesa/main/pipelineobj.c | 6 ------
>> src/mesa/main/shaderapi.c | 3 ---
>> 6 files changed, 38 deletions(-)
>>
>> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
>> index 2885143..f581e4c 100644
>> --- a/src/mesa/main/arrayobj.c
>> +++ b/src/mesa/main/arrayobj.c
>> @@ -174,7 +174,6 @@ _mesa_delete_vao(struct gl_context *ctx, struct gl_vertex_array_object *obj)
>> {
>> unbind_array_object_vbos(ctx, obj);
>> _mesa_reference_buffer_object(ctx, &obj->IndexBufferObj, NULL);
>> - mtx_destroy(&obj->Mutex);
>> free(obj->Label);
>> free(obj);
>> }
>> @@ -197,11 +196,9 @@ _mesa_reference_vao_(struct gl_context *ctx,
>> GLboolean deleteFlag = GL_FALSE;
>> struct gl_vertex_array_object *oldObj = *ptr;
>>
>> - mtx_lock(&oldObj->Mutex);
>> assert(oldObj->RefCount > 0);
>> oldObj->RefCount--;
>> deleteFlag = (oldObj->RefCount == 0);
>> - mtx_unlock(&oldObj->Mutex);
>>
>> if (deleteFlag) {
>> assert(ctx->Driver.DeleteArrayObject);
>> @@ -214,7 +211,6 @@ _mesa_reference_vao_(struct gl_context *ctx,
>>
>> if (vao) {
>> /* reference new array object */
>> - mtx_lock(&vao->Mutex);
>> if (vao->RefCount == 0) {
>> /* this array's being deleted (look just above) */
>> /* Not sure this can every really happen. Warn if it does. */
>> @@ -225,7 +221,6 @@ _mesa_reference_vao_(struct gl_context *ctx,
>> vao->RefCount++;
>> *ptr = vao;
>> }
>> - mtx_unlock(&vao->Mutex);
>> }
>> }
>>
>> @@ -274,7 +269,6 @@ _mesa_initialize_vao(struct gl_context *ctx,
>>
>> obj->Name = name;
>>
>> - mtx_init(&obj->Mutex, mtx_plain);
>> obj->RefCount = 1;
>>
>> /* Init the individual arrays */
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index 6c6de2f..6bc8204 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -89,8 +89,6 @@ delete_dummy_framebuffer(struct gl_framebuffer *fb)
>> void
>> _mesa_init_fbobjects(struct gl_context *ctx)
>> {
>> - mtx_init(&DummyFramebuffer.Mutex, mtx_plain);
>> - mtx_init(&IncompleteFramebuffer.Mutex, mtx_plain);
>> DummyFramebuffer.Delete = delete_dummy_framebuffer;
>> DummyRenderbuffer.Delete = delete_dummy_renderbuffer;
>> IncompleteFramebuffer.Delete = delete_dummy_framebuffer;
>> @@ -527,8 +525,6 @@ _mesa_FramebufferRenderbuffer_sw(struct gl_context *ctx,
>> {
>> struct gl_renderbuffer_attachment *att;
>>
>> - mtx_lock(&fb->Mutex);
>> -
>> att = get_attachment(ctx, fb, attachment);
>> assert(att);
>> if (rb) {
>> @@ -552,8 +548,6 @@ _mesa_FramebufferRenderbuffer_sw(struct gl_context *ctx,
>> }
>>
>> invalidate_framebuffer(fb);
>> -
>> - mtx_unlock(&fb->Mutex);
>> }
>>
>>
>> @@ -3084,7 +3078,6 @@ _mesa_framebuffer_texture(struct gl_context *ctx, struct gl_framebuffer *fb,
>>
>> FLUSH_VERTICES(ctx, _NEW_BUFFERS);
>>
>> - mtx_lock(&fb->Mutex);
>> if (texObj) {
>> if (attachment == GL_DEPTH_ATTACHMENT &&
>> texObj == fb->Attachment[BUFFER_STENCIL].Texture &&
>> @@ -3142,8 +3135,6 @@ _mesa_framebuffer_texture(struct gl_context *ctx, struct gl_framebuffer *fb,
>> }
>>
>> invalidate_framebuffer(fb);
>> -
>> - mtx_unlock(&fb->Mutex);
>> }
>>
>>
>> diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
>> index 37e2c29..475b01b 100644
>> --- a/src/mesa/main/framebuffer.c
>> +++ b/src/mesa/main/framebuffer.c
>> @@ -130,8 +130,6 @@ _mesa_initialize_window_framebuffer(struct gl_framebuffer *fb,
>>
>> memset(fb, 0, sizeof(struct gl_framebuffer));
>>
>> - mtx_init(&fb->Mutex, mtx_plain);
>> -
>> fb->RefCount = 1;
>>
>> /* save the visual */
>> @@ -183,7 +181,6 @@ _mesa_initialize_user_framebuffer(struct gl_framebuffer *fb, GLuint name)
>> fb->ColorReadBuffer = GL_COLOR_ATTACHMENT0_EXT;
>> fb->_ColorReadBufferIndex = BUFFER_COLOR0;
>> fb->Delete = _mesa_destroy_framebuffer;
>> - mtx_init(&fb->Mutex, mtx_plain);
>> }
>>
>>
>> @@ -214,8 +211,6 @@ _mesa_free_framebuffer_data(struct gl_framebuffer *fb)
>> assert(fb);
>> assert(fb->RefCount == 0);
>>
>> - mtx_destroy(&fb->Mutex);
>> -
>> for (i = 0; i < BUFFER_COUNT; i++) {
>> struct gl_renderbuffer_attachment *att = &fb->Attachment[i];
>> if (att->Renderbuffer) {
>> @@ -245,11 +240,9 @@ _mesa_reference_framebuffer_(struct gl_framebuffer **ptr,
>> GLboolean deleteFlag = GL_FALSE;
>> struct gl_framebuffer *oldFb = *ptr;
>>
>> - mtx_lock(&oldFb->Mutex);
>> assert(oldFb->RefCount > 0);
>> oldFb->RefCount--;
>> deleteFlag = (oldFb->RefCount == 0);
>> - mtx_unlock(&oldFb->Mutex);
>>
>> if (deleteFlag)
>> oldFb->Delete(oldFb);
>> @@ -259,9 +252,7 @@ _mesa_reference_framebuffer_(struct gl_framebuffer **ptr,
>> assert(!*ptr);
>>
>> if (fb) {
>> - mtx_lock(&fb->Mutex);
>> fb->RefCount++;
>> - mtx_unlock(&fb->Mutex);
>> *ptr = fb;
>> }
>> }
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index 43d2f67..a1d3ecf 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -1595,8 +1595,6 @@ struct gl_vertex_array_object
>>
>> GLchar *Label; /**< GL_KHR_debug */
>>
>> - mtx_t Mutex;
>> -
>> /**
>> * Does the VAO use ARB semantics or Apple semantics?
>> *
>> @@ -2971,8 +2969,6 @@ struct gl_pipeline_object
>>
>> GLint RefCount;
>>
>> - mtx_t Mutex;
>> -
>> GLchar *Label; /**< GL_KHR_debug */
>>
>> /**
>> @@ -3272,7 +3268,6 @@ struct gl_renderbuffer_attachment
>> */
>> struct gl_framebuffer
>> {
>> - mtx_t Mutex; /**< for thread safety */
>> /**
>> * If zero, this is a window system framebuffer. If non-zero, this
>> * is a FBO framebuffer; note that for some devices (i.e. those with
>> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
>> index 07acbf1..80ac012 100644
>> --- a/src/mesa/main/pipelineobj.c
>> +++ b/src/mesa/main/pipelineobj.c
>> @@ -64,7 +64,6 @@ _mesa_delete_pipeline_object(struct gl_context *ctx,
>> _mesa_reference_shader_program(ctx, &obj->CurrentProgram[i], NULL);
>>
>> _mesa_reference_shader_program(ctx, &obj->ActiveProgram, NULL);
>> - mtx_destroy(&obj->Mutex);
>> free(obj->Label);
>> ralloc_free(obj);
>> }
>> @@ -78,7 +77,6 @@ _mesa_new_pipeline_object(struct gl_context *ctx, GLuint name)
>> struct gl_pipeline_object *obj = rzalloc(NULL, struct gl_pipeline_object);
>> if (obj) {
>> obj->Name = name;
>> - mtx_init(&obj->Mutex, mtx_plain);
>> obj->RefCount = 1;
>> obj->Flags = _mesa_get_shader_flags();
>> obj->InfoLog = NULL;
>> @@ -187,11 +185,9 @@ _mesa_reference_pipeline_object_(struct gl_context *ctx,
>> GLboolean deleteFlag = GL_FALSE;
>> struct gl_pipeline_object *oldObj = *ptr;
>>
>> - mtx_lock(&oldObj->Mutex);
>> assert(oldObj->RefCount > 0);
>> oldObj->RefCount--;
>> deleteFlag = (oldObj->RefCount == 0);
>> - mtx_unlock(&oldObj->Mutex);
>>
>> if (deleteFlag) {
>> _mesa_delete_pipeline_object(ctx, oldObj);
>> @@ -203,7 +199,6 @@ _mesa_reference_pipeline_object_(struct gl_context *ctx,
>>
>> if (obj) {
>> /* reference new pipeline object */
>> - mtx_lock(&obj->Mutex);
>> if (obj->RefCount == 0) {
>> /* this pipeline's being deleted (look just above) */
>> /* Not sure this can ever really happen. Warn if it does. */
>> @@ -214,7 +209,6 @@ _mesa_reference_pipeline_object_(struct gl_context *ctx,
>> obj->RefCount++;
>> *ptr = obj;
>> }
>> - mtx_unlock(&obj->Mutex);
>> }
>> }
>>
>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> index 67acdc1..193117e 100644
>> --- a/src/mesa/main/shaderapi.c
>> +++ b/src/mesa/main/shaderapi.c
>> @@ -126,8 +126,6 @@ _mesa_init_shader_state(struct gl_context *ctx)
>>
>> /* Extended for ARB_separate_shader_objects */
>> ctx->Shader.RefCount = 1;
>> - mtx_init(&ctx->Shader.Mutex, mtx_plain);
>> -
>> ctx->TessCtrlProgram.patch_vertices = 3;
>> for (i = 0; i < 4; ++i)
>> ctx->TessCtrlProgram.patch_default_outer_level[i] = 1.0;
>> @@ -155,7 +153,6 @@ _mesa_free_shader_state(struct gl_context *ctx)
>> _mesa_reference_pipeline_object(ctx, &ctx->_Shader, NULL);
>>
>> assert(ctx->Shader.RefCount == 1);
>> - mtx_destroy(&ctx->Shader.Mutex);
>> }
>>
>>
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list