On Fri, Aug 7, 2015 at 2:09 AM, Timothy Arceri <t_arc...@yahoo.com.au> wrote: > 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 ?
Sure, that's a good idea. I'll make that change. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev