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; _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev