[Mesa-dev] [PATCH] Make generation of framebuffer and renderbuffer id's threadsafe

2015-11-13 Thread Samuel Maroy 
This should fix the issue described in
https://bugs.freedesktop.org/show_bug.cgi?id=92633

---
 src/mesa/main/fbobject.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index fe6bdc2..6398ff6 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -1637,6 +1637,8 @@ create_render_buffers(struct gl_context *ctx, GLsizei n, 
GLuint *renderbuffers,
if (!renderbuffers)
   return;

+   mtx_lock(>Shared->Mutex);
+
first = _mesa_HashFindFreeKeyBlock(ctx->Shared->RenderBuffers, n);

for (i = 0; i < n; i++) {
@@ -1647,11 +1649,10 @@ 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(>Shared->Mutex);
  _mesa_HashInsert(ctx->Shared->RenderBuffers, name, 
);
- mtx_unlock(>Shared->Mutex);
   }
}
+   mtx_unlock(>Shared->Mutex);
 }


@@ -2650,6 +2651,7 @@ create_framebuffers(GLsizei n, GLuint *framebuffers, bool 
dsa)
if (!framebuffers)
   return;

+   mtx_lock(>Shared->Mutex);
first = _mesa_HashFindFreeKeyBlock(ctx->Shared->FrameBuffers, n);

for (i = 0; i < n; i++) {
@@ -2660,16 +2662,17 @@ create_framebuffers(GLsizei n, GLuint *framebuffers, 
bool dsa)
  fb = ctx->Driver.NewFramebuffer(ctx, framebuffers[i]);
  if (!fb) {
 _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", func);
-return;
+goto beach;
  }
   }
   else
  fb = 

-  mtx_lock(>Shared->Mutex);
   _mesa_HashInsert(ctx->Shared->FrameBuffers, name, fb);
-  mtx_unlock(>Shared->Mutex);
}
+
+beach:
+   mtx_unlock(>Shared->Mutex);
 }


--
2.1.4

This message is subject to the following terms and conditions: MAIL 
DISCLAIMER
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] Make generation of framebuffer and renderbuffer id's threadsafe

2015-11-13 Thread Brian Paul 

Hi Samuel,

The subject line should probably be something like:

"mesa: add locking in create_render_buffers() and create_framebuffers()"

Then, the comment body:

"""
This makes generation of framebuffer and renderbuffer id's threadsafe

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92633
"""


On 11/13/2015 03:57 AM, Samuel Maroy wrote:

This should fix the issue described in
https://bugs.freedesktop.org/show_bug.cgi?id=92633

---
  src/mesa/main/fbobject.c | 13 -
  1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index fe6bdc2..6398ff6 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -1637,6 +1637,8 @@ create_render_buffers(struct gl_context *ctx, GLsizei n, 
GLuint *renderbuffers,
 if (!renderbuffers)
return;

+   mtx_lock(>Shared->Mutex);
+
 first = _mesa_HashFindFreeKeyBlock(ctx->Shared->RenderBuffers, n);

 for (i = 0; i < n; i++) {
@@ -1647,11 +1649,10 @@ create_render_buffers(struct gl_context *ctx, GLsizei 
n, GLuint *renderbuffers,
   allocate_renderbuffer(ctx, name, func);


This function also calls mtx_lock(ctx->Shared->Mutex) so it will be a 
recursive lock.


In shared.c we init the mutex with:

   mtx_init(>Mutex, mtx_plain);

So we should probably use (mtx_plain | mtx_recursive) there.

I think someone else spotted this a while ago but it looks like nothing 
was changed.  We should probably do an audit of our locking to check for 
other places where this might happen.





} else {
   /* insert a dummy renderbuffer into the hash table */
- mtx_lock(>Shared->Mutex);
   _mesa_HashInsert(ctx->Shared->RenderBuffers, name, 
);
- mtx_unlock(>Shared->Mutex);
}
 }
+   mtx_unlock(>Shared->Mutex);


I'd insert one blank line before the new mtx_unlock().


  }


@@ -2650,6 +2651,7 @@ create_framebuffers(GLsizei n, GLuint *framebuffers, bool 
dsa)
 if (!framebuffers)
return;

+   mtx_lock(>Shared->Mutex);
 first = _mesa_HashFindFreeKeyBlock(ctx->Shared->FrameBuffers, n);

 for (i = 0; i < n; i++) {
@@ -2660,16 +2662,17 @@ create_framebuffers(GLsizei n, GLuint *framebuffers, 
bool dsa)
   fb = ctx->Driver.NewFramebuffer(ctx, framebuffers[i]);
   if (!fb) {
  _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", func);
-return;
+goto beach;


Cute, but I think some people would prefer "goto cleanup" or something 
like that.



   }
}
else
   fb = 

-  mtx_lock(>Shared->Mutex);
_mesa_HashInsert(ctx->Shared->FrameBuffers, name, fb);
-  mtx_unlock(>Shared->Mutex);
 }
+
+beach:
+   mtx_unlock(>Shared->Mutex);
  }



-Brian



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev