[Mesa-dev] [PATCH 11/13] mesa: Replace uses of Shared->Mutex with hash-table mutexes
Timothy Arceri
t_arceri at yahoo.com.au
Fri Aug 7 02:09:49 PDT 2015
On Thu, 2015-08-06 at 17:11 -0700, Matt Turner wrote:
> We were locking the Shared->Mutex and then using calling functions like
> _mesa_HashInsert that do additional per-hash-table locking internally.
>
> Instead just lock each hash-table's mutex and use functions like
> _mesa_HashInsertLocked and the new _mesa_HashRemoveLocked.
>
> In order to do this, we need to remove the locking from
> _mesa_HashFindFreeKeyBlock since it will always be called with the
> per-hash-table lock taken.
> ---
> src/mesa/main/arbprogram.c | 7 ++++++-
> src/mesa/main/atifragshader.c | 6 +++++-
> src/mesa/main/bufferobj.c | 26 ++++++++++++++------------
> src/mesa/main/dlist.c | 8 ++++----
> src/mesa/main/fbobject.c | 23 ++++++++++++++---------
> src/mesa/main/hash.c | 4 ----
> src/mesa/main/samplerobj.c | 23 ++++++++++++++++++-----
> src/mesa/main/shaderapi.c | 10 ++++++++--
> src/mesa/main/texobj.c | 14 +++++---------
> 9 files changed, 74 insertions(+), 47 deletions(-)
>
> diff --git a/src/mesa/main/arbprogram.c b/src/mesa/main/arbprogram.c
> index f474951..3f7acda 100644
> --- a/src/mesa/main/arbprogram.c
> +++ b/src/mesa/main/arbprogram.c
> @@ -200,13 +200,18 @@ _mesa_GenProgramsARB(GLsizei n, GLuint *ids)
> if (!ids)
> return;
>
> + _mesa_HashLockMutex(ctx->Shared->Programs);
> +
> first = _mesa_HashFindFreeKeyBlock(ctx->Shared->Programs, n);
>
> /* Insert pointer to dummy program as placeholder */
> for (i = 0; i < (GLuint) n; i++) {
> - _mesa_HashInsert(ctx->Shared->Programs, first + i,
> &_mesa_DummyProgram);
> + _mesa_HashInsertLocked(ctx->Shared->Programs, first + i,
> + &_mesa_DummyProgram);
> }
>
> + _mesa_HashUnlockMutex(ctx->Shared->Programs);
> +
> /* Return the program names */
> for (i = 0; i < (GLuint) n; i++) {
> ids[i] = first + i;
> diff --git a/src/mesa/main/atifragshader.c b/src/mesa/main/atifragshader.c
> index 935ba05..9dd4e21 100644
> --- a/src/mesa/main/atifragshader.c
> +++ b/src/mesa/main/atifragshader.c
> @@ -199,11 +199,15 @@ _mesa_GenFragmentShadersATI(GLuint range)
> return 0;
> }
>
> + _mesa_HashLockMutex(ctx->Shared->ATIShaders);
> +
> first = _mesa_HashFindFreeKeyBlock(ctx->Shared->ATIShaders, range);
> for (i = 0; i < range; i++) {
> - _mesa_HashInsert(ctx->Shared->ATIShaders, first + i, &DummyShader);
> + _mesa_HashInsertLocked(ctx->Shared->ATIShaders, first + i,
> &DummyShader);
> }
>
> + _mesa_HashUnlockMutex(ctx->Shared->ATIShaders);
> +
> return first;
> }
>
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index 78af229..6a995e7 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -994,8 +994,11 @@ _mesa_lookup_bufferobj(struct gl_context *ctx, GLuint
> buffer)
> struct gl_buffer_object *
> _mesa_lookup_bufferobj_locked(struct gl_context *ctx, GLuint buffer)
> {
> - return (struct gl_buffer_object *)
> - _mesa_HashLookupLocked(ctx->Shared->BufferObjects, buffer);
> + if (buffer == 0)
> + return NULL;
> + else
> + return (struct gl_buffer_object *)
> + _mesa_HashLookupLocked(ctx->Shared->BufferObjects, buffer);
> }
>
> /**
> @@ -1179,10 +1182,11 @@ _mesa_DeleteBuffers(GLsizei n, const GLuint *ids)
> return;
> }
>
> - mtx_lock(&ctx->Shared->Mutex);
> + _mesa_HashLockMutex(ctx->Shared->BufferObjects);
>
> for (i = 0; i < n; i++) {
> - struct gl_buffer_object *bufObj = _mesa_lookup_bufferobj(ctx,
> ids[i]);
> + struct gl_buffer_object *bufObj =
> + _mesa_lookup_bufferobj_locked(ctx, ids[i]);
> if (bufObj) {
> struct gl_vertex_array_object *vao = ctx->Array.VAO;
> GLuint j;
> @@ -1276,7 +1280,7 @@ _mesa_DeleteBuffers(GLsizei n, const GLuint *ids)
> }
>
> /* The ID is immediately freed for re-use */
> - _mesa_HashRemove(ctx->Shared->BufferObjects, ids[i]);
> + _mesa_HashRemoveLocked(ctx->Shared->BufferObjects, ids[i]);
> /* Make sure we do not run into the classic ABA problem on bind.
> * We don't want to allow re-binding a buffer object that's been
> * "deleted" by glDeleteBuffers().
> @@ -1292,7 +1296,7 @@ _mesa_DeleteBuffers(GLsizei n, const GLuint *ids)
> }
> }
>
> - mtx_unlock(&ctx->Shared->Mutex);
> + _mesa_HashUnlockMutex(ctx->Shared->BufferObjects);
> }
>
>
> @@ -1326,7 +1330,7 @@ create_buffers(GLsizei n, GLuint *buffers, bool dsa)
> /*
> * This must be atomic (generation and allocation of buffer object IDs)
> */
> - mtx_lock(&ctx->Shared->Mutex);
> + _mesa_HashLockMutex(ctx->Shared->BufferObjects);
>
> first = _mesa_HashFindFreeKeyBlock(ctx->Shared->BufferObjects, n);
>
> @@ -1341,17 +1345,17 @@ create_buffers(GLsizei n, GLuint *buffers, bool dsa)
> buf = ctx->Driver.NewBufferObject(ctx, buffers[i]);
> if (!buf) {
> _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", func);
> - mtx_unlock(&ctx->Shared->Mutex);
> + _mesa_HashUnlockMutex(ctx->Shared->BufferObjects);
> return;
> }
> }
> else
> buf = &DummyBufferObject;
>
> - _mesa_HashInsert(ctx->Shared->BufferObjects, buffers[i], buf);
> + _mesa_HashInsertLocked(ctx->Shared->BufferObjects, buffers[i], buf);
> }
>
> - mtx_unlock(&ctx->Shared->Mutex);
> + _mesa_HashUnlockMutex(ctx->Shared->BufferObjects);
> }
>
> /**
> @@ -1393,9 +1397,7 @@ _mesa_IsBuffer(GLuint id)
> GET_CURRENT_CONTEXT(ctx);
> ASSERT_OUTSIDE_BEGIN_END_WITH_RETVAL(ctx, GL_FALSE);
>
> - mtx_lock(&ctx->Shared->Mutex);
> bufObj = _mesa_lookup_bufferobj(ctx, id);
> - mtx_unlock(&ctx->Shared->Mutex);
>
> return bufObj && bufObj != &DummyBufferObject;
> }
> diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c
> index 5554738..f00d0c8 100644
> --- a/src/mesa/main/dlist.c
> +++ b/src/mesa/main/dlist.c
> @@ -8969,19 +8969,19 @@ _mesa_GenLists(GLsizei range)
> /*
> * Make this an atomic operation
> */
> - mtx_lock(&ctx->Shared->Mutex);
> + _mesa_HashLockMutex(ctx->Shared->DisplayList);
>
> base = _mesa_HashFindFreeKeyBlock(ctx->Shared->DisplayList, range);
> if (base) {
> /* reserve the list IDs by with empty/dummy lists */
> GLint i;
> for (i = 0; i < range; i++) {
> - _mesa_HashInsert(ctx->Shared->DisplayList, base + i,
> - make_list(base + i, 1));
> + _mesa_HashInsertLocked(ctx->Shared->DisplayList, base + i,
> + make_list(base + i, 1));
> }
> }
>
> - mtx_unlock(&ctx->Shared->Mutex);
> + _mesa_HashUnlockMutex(ctx->Shared->DisplayList);
>
> return base;
> }
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index 918889e..6c6de2f 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -1300,10 +1300,8 @@ allocate_renderbuffer(struct gl_context *ctx, GLuint
allocate_renderbuffer_locked ?
> renderbuffer,
> return NULL;
> }
> assert(newRb->AllocStorage);
> - mtx_lock(&ctx->Shared->Mutex);
> - _mesa_HashInsert(ctx->Shared->RenderBuffers, renderbuffer, newRb);
> + _mesa_HashInsertLocked(ctx->Shared->RenderBuffers, renderbuffer, newRb);
> newRb->RefCount = 1; /* referenced by hash table */
> - mtx_unlock(&ctx->Shared->Mutex);
>
> return newRb;
> }
> @@ -1337,7 +1335,9 @@ bind_renderbuffer(GLenum target, GLuint renderbuffer,
> bool allow_user_names)
> }
>
> if (!newRb) {
> + _mesa_HashLockMutex(ctx->Shared->RenderBuffers);
> newRb = allocate_renderbuffer(ctx, renderbuffer,
> "glBindRenderbufferEXT");
> + _mesa_HashUnlockMutex(ctx->Shared->RenderBuffers);
> }
> }
> else {
> @@ -1620,6 +1620,8 @@ create_render_buffers(struct gl_context *ctx, GLsizei
> n, GLuint *renderbuffers,
> if (!renderbuffers)
> return;
>
> + _mesa_HashLockMutex(ctx->Shared->RenderBuffers);
> +
> first = _mesa_HashFindFreeKeyBlock(ctx->Shared->RenderBuffers, n);
>
> for (i = 0; i < n; i++) {
> @@ -1630,11 +1632,12 @@ create_render_buffers(struct gl_context *ctx,
> GLsizei n, GLuint *renderbuffers,
> allocate_renderbuffer(ctx, name, func);
> } else {
> /* insert a dummy renderbuffer into the hash table */
> - mtx_lock(&ctx->Shared->Mutex);
> - _mesa_HashInsert(ctx->Shared->RenderBuffers, name,
> &DummyRenderbuffer);
> - mtx_unlock(&ctx->Shared->Mutex);
> + _mesa_HashInsertLocked(ctx->Shared->RenderBuffers, name,
> + &DummyRenderbuffer);
> }
> }
> +
> + _mesa_HashUnlockMutex(ctx->Shared->RenderBuffers);
> }
>
>
> @@ -2623,6 +2626,8 @@ create_framebuffers(GLsizei n, GLuint *framebuffers,
> bool dsa)
> if (!framebuffers)
> return;
>
> + _mesa_HashLockMutex(ctx->Shared->FrameBuffers);
> +
> first = _mesa_HashFindFreeKeyBlock(ctx->Shared->FrameBuffers, n);
>
> for (i = 0; i < n; i++) {
> @@ -2639,10 +2644,10 @@ create_framebuffers(GLsizei n, GLuint *framebuffers,
> bool dsa)
> else
> fb = &DummyFramebuffer;
>
> - mtx_lock(&ctx->Shared->Mutex);
> - _mesa_HashInsert(ctx->Shared->FrameBuffers, name, fb);
> - mtx_unlock(&ctx->Shared->Mutex);
> + _mesa_HashInsertLocked(ctx->Shared->FrameBuffers, name, fb);
> }
> +
> + _mesa_HashUnlockMutex(ctx->Shared->FrameBuffers);
> }
>
>
> diff --git a/src/mesa/main/hash.c b/src/mesa/main/hash.c
> index aa1c6a1..faa541e 100644
> --- a/src/mesa/main/hash.c
> +++ b/src/mesa/main/hash.c
> @@ -468,10 +468,8 @@ GLuint
> _mesa_HashFindFreeKeyBlock(struct _mesa_HashTable *table, GLuint numKeys)
> {
> const GLuint maxKey = ~((GLuint) 0) - 1;
> - mtx_lock(&table->Mutex);
> if (maxKey - numKeys > table->MaxKey) {
> /* the quick solution */
> - mtx_unlock(&table->Mutex);
> return table->MaxKey + 1;
> }
> else {
> @@ -489,13 +487,11 @@ _mesa_HashFindFreeKeyBlock(struct _mesa_HashTable
> *table, GLuint numKeys)
> /* this key not in use, check if we've found enough */
> freeCount++;
> if (freeCount == numKeys) {
> - mtx_unlock(&table->Mutex);
> return freeStart;
> }
> }
> }
> /* cannot allocate a block of numKeys consecutive keys */
> - mtx_unlock(&table->Mutex);
> return 0;
> }
> }
> diff --git a/src/mesa/main/samplerobj.c b/src/mesa/main/samplerobj.c
> index dba2087..19e4021 100644
> --- a/src/mesa/main/samplerobj.c
> +++ b/src/mesa/main/samplerobj.c
> @@ -51,6 +51,15 @@ _mesa_lookup_samplerobj(struct gl_context *ctx, GLuint
> name)
> _mesa_HashLookup(ctx->Shared->SamplerObjects, name);
> }
>
> +static struct gl_sampler_object *
> +_mesa_lookup_samplerobj_locked(struct gl_context *ctx, GLuint name)
> +{
> + if (name == 0)
> + return NULL;
> + else
> + return (struct gl_sampler_object *)
> + _mesa_HashLookupLocked(ctx->Shared->SamplerObjects, name);
> +}
>
> static inline void
> begin_samplerobj_lookups(struct gl_context *ctx)
> @@ -183,15 +192,19 @@ create_samplers(struct gl_context *ctx, GLsizei count,
> GLuint *samplers,
> if (!samplers)
> return;
>
> + _mesa_HashLockMutex(ctx->Shared->SamplerObjects);
> +
> first = _mesa_HashFindFreeKeyBlock(ctx->Shared->SamplerObjects, count);
>
> /* Insert the ID and pointer to new sampler object into hash table */
> for (i = 0; i < count; i++) {
> struct gl_sampler_object *sampObj =
> ctx->Driver.NewSamplerObject(ctx, first + i);
> - _mesa_HashInsert(ctx->Shared->SamplerObjects, first + i, sampObj);
> + _mesa_HashInsertLocked(ctx->Shared->SamplerObjects, first + i,
> sampObj);
> samplers[i] = first + i;
> }
> +
> + _mesa_HashUnlockMutex(ctx->Shared->SamplerObjects);
> }
>
> void GLAPIENTRY
> @@ -222,13 +235,13 @@ _mesa_DeleteSamplers(GLsizei count, const GLuint
> *samplers)
> return;
> }
>
> - mtx_lock(&ctx->Shared->Mutex);
> + _mesa_HashLockMutex(ctx->Shared->SamplerObjects);
>
> for (i = 0; i < count; i++) {
> if (samplers[i]) {
> GLuint j;
> struct gl_sampler_object *sampObj =
> - _mesa_lookup_samplerobj(ctx, samplers[i]);
> + _mesa_lookup_samplerobj_locked(ctx, samplers[i]);
>
> if (sampObj) {
> /* If the sampler is currently bound, unbind it. */
> @@ -240,14 +253,14 @@ _mesa_DeleteSamplers(GLsizei count, const GLuint
> *samplers)
> }
>
> /* The ID is immediately freed for re-use */
> - _mesa_HashRemove(ctx->Shared->SamplerObjects, samplers[i]);
> + _mesa_HashRemoveLocked(ctx->Shared->SamplerObjects,
> samplers[i]);
> /* But the object exists until its reference count goes to zero
> */
> _mesa_reference_sampler_object(ctx, &sampObj, NULL);
> }
> }
> }
>
> - mtx_unlock(&ctx->Shared->Mutex);
> + _mesa_HashUnlockMutex(ctx->Shared->SamplerObjects);
> }
>
>
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 5b28a2c..67acdc1 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -306,9 +306,11 @@ create_shader(struct gl_context *ctx, GLenum type)
> return 0;
> }
>
> + _mesa_HashLockMutex(ctx->Shared->ShaderObjects);
> name = _mesa_HashFindFreeKeyBlock(ctx->Shared->ShaderObjects, 1);
> sh = ctx->Driver.NewShader(ctx, name, type);
> - _mesa_HashInsert(ctx->Shared->ShaderObjects, name, sh);
> + _mesa_HashInsertLocked(ctx->Shared->ShaderObjects, name, sh);
> + _mesa_HashUnlockMutex(ctx->Shared->ShaderObjects);
>
> return name;
> }
> @@ -320,14 +322,18 @@ create_shader_program(struct gl_context *ctx)
> GLuint name;
> struct gl_shader_program *shProg;
>
> + _mesa_HashLockMutex(ctx->Shared->ShaderObjects);
> +
> name = _mesa_HashFindFreeKeyBlock(ctx->Shared->ShaderObjects, 1);
>
> shProg = ctx->Driver.NewShaderProgram(name);
>
> - _mesa_HashInsert(ctx->Shared->ShaderObjects, name, shProg);
> + _mesa_HashInsertLocked(ctx->Shared->ShaderObjects, name, shProg);
>
> assert(shProg->RefCount == 1);
>
> + _mesa_HashUnlockMutex(ctx->Shared->ShaderObjects);
> +
> return name;
> }
>
> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
> index 7c0ae93..d6d9684 100644
> --- a/src/mesa/main/texobj.c
> +++ b/src/mesa/main/texobj.c
> @@ -1222,7 +1222,7 @@ create_textures(struct gl_context *ctx, GLenum target,
> /*
> * This must be atomic (generation and allocation of texture IDs)
> */
> - mtx_lock(&ctx->Shared->Mutex);
> + _mesa_HashLockMutex(ctx->Shared->TexObjects);
>
> first = _mesa_HashFindFreeKeyBlock(ctx->Shared->TexObjects, n);
>
> @@ -1233,7 +1233,7 @@ create_textures(struct gl_context *ctx, GLenum target,
> GLuint name = first + i;
> texObj = ctx->Driver.NewTextureObject(ctx, name, target);
> if (!texObj) {
> - mtx_unlock(&ctx->Shared->Mutex);
> + _mesa_HashUnlockMutex(ctx->Shared->TexObjects);
> _mesa_error(ctx, GL_OUT_OF_MEMORY, "gl%sTextures", func);
> return;
> }
> @@ -1242,7 +1242,7 @@ create_textures(struct gl_context *ctx, GLenum target,
> if (target != 0) {
> targetIndex = _mesa_tex_target_to_index(ctx, texObj->Target);
> if (targetIndex < 0) { /* Bad Target */
> - mtx_unlock(&ctx->Shared->Mutex);
> + _mesa_HashUnlockMutex(ctx->Shared->TexObjects);
> _mesa_error(ctx, GL_INVALID_ENUM, "gl%sTextures(target = %s)",
> func, _mesa_enum_to_string(texObj->Target));
> return;
> @@ -1252,12 +1252,12 @@ create_textures(struct gl_context *ctx, GLenum
> target,
> }
>
> /* insert into hash table */
> - _mesa_HashInsert(ctx->Shared->TexObjects, texObj->Name, texObj);
> + _mesa_HashInsertLocked(ctx->Shared->TexObjects, texObj->Name,
> texObj);
>
> textures[i] = name;
> }
>
> - mtx_unlock(&ctx->Shared->Mutex);
> + _mesa_HashUnlockMutex(ctx->Shared->TexObjects);
> }
>
> /*@}*/
> @@ -1500,9 +1500,7 @@ _mesa_DeleteTextures( GLsizei n, const GLuint
> *textures)
> /* The texture _name_ is now free for re-use.
> * Remove it from the hash table now.
> */
> - mtx_lock(&ctx->Shared->Mutex);
> _mesa_HashRemove(ctx->Shared->TexObjects, delObj->Name);
> - mtx_unlock(&ctx->Shared->Mutex);
>
> /* Unreference the texobj. If refcount hits zero, the texture
> * will be deleted.
> @@ -1679,9 +1677,7 @@ _mesa_BindTexture( GLenum target, GLuint texName )
> }
>
> /* and insert it into hash table */
> - mtx_lock(&ctx->Shared->Mutex);
> _mesa_HashInsert(ctx->Shared->TexObjects, texName, newTexObj);
> - mtx_unlock(&ctx->Shared->Mutex);
> }
> newTexObj->Target = target;
> newTexObj->TargetIndex = targetIndex;
More information about the mesa-dev
mailing list