Re: [Mesa-dev] [PATCHv2 01/13] mesa: protect the debug state with a mutex
Hi Ian,
Thanks for the reviews.
On Thu, Aug 14, 2014 at 8:45 AM, Ian Romanick i...@freedesktop.org wrote:
Patches 7, 8, and 9 are
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
I made a few minor comments on 7, but they should be trivial to resolve.
Patches 10 through 13 are
Acked-by: Ian Romanick ian.d.roman...@intel.com
I'd like to see some feed back on those last four from Ken and / or
Matt. I'd also like, if possible, for this to land this week.
Sorry that I wasn't quite available until now. Our two-year-old was
in and out of the hopsital for the last ten days or so...
When do you plan to branch off 10.3? I should be able to send out v3
late evening Tuesday, and be responsive.
On 07/09/2014 12:47 AM, Chia-I Wu wrote:
We are about to change mesa to spawn threads for deferred glCompileShader and
glLinkProgram, and we need to make sure those threads can send compiler
warnings/errors to the debug output safely.
Signed-off-by: Chia-I Wu o...@lunarg.com
---
src/mesa/main/errors.c | 172
+++--
src/mesa/main/mtypes.h | 1 +
2 files changed, 126 insertions(+), 47 deletions(-)
diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index aa0ff50..156eb0d 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -676,22 +676,41 @@ debug_pop_group(struct gl_debug_state *debug)
/**
- * Return debug state for the context. The debug state will be allocated
- * and initialized upon the first call.
+ * Lock and return debug state for the context. The debug state will be
+ * allocated and initialized upon the first call. When NULL is returned,
the
+ * debug state is not locked.
*/
static struct gl_debug_state *
-_mesa_get_debug_state(struct gl_context *ctx)
+_mesa_lock_debug_state(struct gl_context *ctx)
{
+ mtx_lock(ctx-DebugMutex);
+
if (!ctx-Debug) {
ctx-Debug = debug_create();
if (!ctx-Debug) {
- _mesa_error(ctx, GL_OUT_OF_MEMORY, allocating debug state);
+ GET_CURRENT_CONTEXT(cur);
+ mtx_unlock(ctx-DebugMutex);
+
+ /*
+ * This function may be called from other threads. When that is
the
+ * case, we cannot record this OOM error.
+ */
+ if (ctx == cur)
+_mesa_error(ctx, GL_OUT_OF_MEMORY, allocating debug state);
+
+ return NULL;
}
}
return ctx-Debug;
}
+static void
+_mesa_unlock_debug_state(struct gl_context *ctx)
+{
+ mtx_unlock(ctx-DebugMutex);
+}
+
/**
* Set the integer debug state specified by \p pname. This can be called
from
* _mesa_set_enable for example.
@@ -699,7 +718,7 @@ _mesa_get_debug_state(struct gl_context *ctx)
bool
_mesa_set_debug_state_int(struct gl_context *ctx, GLenum pname, GLint val)
{
- struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
+ struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
if (!debug)
return false;
@@ -716,6 +735,8 @@ _mesa_set_debug_state_int(struct gl_context *ctx, GLenum
pname, GLint val)
break;
}
+ _mesa_unlock_debug_state(ctx);
+
return true;
}
@@ -729,9 +750,12 @@ _mesa_get_debug_state_int(struct gl_context *ctx,
GLenum pname)
struct gl_debug_state *debug;
GLint val;
+ mtx_lock(ctx-DebugMutex);
debug = ctx-Debug;
- if (!debug)
+ if (!debug) {
+ mtx_unlock(ctx-DebugMutex);
return 0;
+ }
switch (pname) {
case GL_DEBUG_OUTPUT:
@@ -756,6 +780,8 @@ _mesa_get_debug_state_int(struct gl_context *ctx, GLenum
pname)
break;
}
+ mtx_unlock(ctx-DebugMutex);
+
return val;
}
@@ -769,9 +795,12 @@ _mesa_get_debug_state_ptr(struct gl_context *ctx,
GLenum pname)
struct gl_debug_state *debug;
void *val;
+ mtx_lock(ctx-DebugMutex);
debug = ctx-Debug;
- if (!debug)
+ if (!debug) {
+ mtx_unlock(ctx-DebugMutex);
return NULL;
+ }
switch (pname) {
case GL_DEBUG_CALLBACK_FUNCTION_ARB:
@@ -786,9 +815,49 @@ _mesa_get_debug_state_ptr(struct gl_context *ctx,
GLenum pname)
break;
}
+ mtx_unlock(ctx-DebugMutex);
+
return val;
}
+/**
+ * Insert a debug message. The mutex is assumed to be locked, and will be
+ * unlocked by this call.
+ */
+static void
+log_msg_locked_and_unlock(struct gl_context *ctx,
+ enum mesa_debug_source source,
+ enum mesa_debug_type type, GLuint id,
+ enum mesa_debug_severity severity,
+ GLint len, const char *buf)
+{
+ struct gl_debug_state *debug = ctx-Debug;
+
+ if (!debug_is_message_enabled(debug, source, type, id, severity)) {
+ _mesa_unlock_debug_state(ctx);
+ return;
+ }
+
+ if (ctx-Debug-Callback) {
+ GLenum gl_source = debug_source_enums[source];
+ GLenum gl_type = debug_type_enums[type];
+
Re: [Mesa-dev] [PATCHv2 01/13] mesa: protect the debug state with a mutex
This series fell off my radar. Sorry about the delays. :(
Patches 1 through 5 are
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
I'm still working my way through the rest. I'd like to see these land
as soon as reasonably possible.
On 07/09/2014 12:47 AM, Chia-I Wu wrote:
We are about to change mesa to spawn threads for deferred glCompileShader and
glLinkProgram, and we need to make sure those threads can send compiler
warnings/errors to the debug output safely.
Signed-off-by: Chia-I Wu o...@lunarg.com
---
src/mesa/main/errors.c | 172
+++--
src/mesa/main/mtypes.h | 1 +
2 files changed, 126 insertions(+), 47 deletions(-)
diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index aa0ff50..156eb0d 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -676,22 +676,41 @@ debug_pop_group(struct gl_debug_state *debug)
/**
- * Return debug state for the context. The debug state will be allocated
- * and initialized upon the first call.
+ * Lock and return debug state for the context. The debug state will be
+ * allocated and initialized upon the first call. When NULL is returned, the
+ * debug state is not locked.
*/
static struct gl_debug_state *
-_mesa_get_debug_state(struct gl_context *ctx)
+_mesa_lock_debug_state(struct gl_context *ctx)
{
+ mtx_lock(ctx-DebugMutex);
+
if (!ctx-Debug) {
ctx-Debug = debug_create();
if (!ctx-Debug) {
- _mesa_error(ctx, GL_OUT_OF_MEMORY, allocating debug state);
+ GET_CURRENT_CONTEXT(cur);
+ mtx_unlock(ctx-DebugMutex);
+
+ /*
+ * This function may be called from other threads. When that is the
+ * case, we cannot record this OOM error.
+ */
+ if (ctx == cur)
+_mesa_error(ctx, GL_OUT_OF_MEMORY, allocating debug state);
+
+ return NULL;
}
}
return ctx-Debug;
}
+static void
+_mesa_unlock_debug_state(struct gl_context *ctx)
+{
+ mtx_unlock(ctx-DebugMutex);
+}
+
/**
* Set the integer debug state specified by \p pname. This can be called
from
* _mesa_set_enable for example.
@@ -699,7 +718,7 @@ _mesa_get_debug_state(struct gl_context *ctx)
bool
_mesa_set_debug_state_int(struct gl_context *ctx, GLenum pname, GLint val)
{
- struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
+ struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
if (!debug)
return false;
@@ -716,6 +735,8 @@ _mesa_set_debug_state_int(struct gl_context *ctx, GLenum
pname, GLint val)
break;
}
+ _mesa_unlock_debug_state(ctx);
+
return true;
}
@@ -729,9 +750,12 @@ _mesa_get_debug_state_int(struct gl_context *ctx, GLenum
pname)
struct gl_debug_state *debug;
GLint val;
+ mtx_lock(ctx-DebugMutex);
debug = ctx-Debug;
- if (!debug)
+ if (!debug) {
+ mtx_unlock(ctx-DebugMutex);
return 0;
+ }
switch (pname) {
case GL_DEBUG_OUTPUT:
@@ -756,6 +780,8 @@ _mesa_get_debug_state_int(struct gl_context *ctx, GLenum
pname)
break;
}
+ mtx_unlock(ctx-DebugMutex);
+
return val;
}
@@ -769,9 +795,12 @@ _mesa_get_debug_state_ptr(struct gl_context *ctx, GLenum
pname)
struct gl_debug_state *debug;
void *val;
+ mtx_lock(ctx-DebugMutex);
debug = ctx-Debug;
- if (!debug)
+ if (!debug) {
+ mtx_unlock(ctx-DebugMutex);
return NULL;
+ }
switch (pname) {
case GL_DEBUG_CALLBACK_FUNCTION_ARB:
@@ -786,9 +815,49 @@ _mesa_get_debug_state_ptr(struct gl_context *ctx, GLenum
pname)
break;
}
+ mtx_unlock(ctx-DebugMutex);
+
return val;
}
+/**
+ * Insert a debug message. The mutex is assumed to be locked, and will be
+ * unlocked by this call.
+ */
+static void
+log_msg_locked_and_unlock(struct gl_context *ctx,
+ enum mesa_debug_source source,
+ enum mesa_debug_type type, GLuint id,
+ enum mesa_debug_severity severity,
+ GLint len, const char *buf)
+{
+ struct gl_debug_state *debug = ctx-Debug;
+
+ if (!debug_is_message_enabled(debug, source, type, id, severity)) {
+ _mesa_unlock_debug_state(ctx);
+ return;
+ }
+
+ if (ctx-Debug-Callback) {
+ GLenum gl_source = debug_source_enums[source];
+ GLenum gl_type = debug_type_enums[type];
+ GLenum gl_severity = debug_severity_enums[severity];
+ GLDEBUGPROC callback = ctx-Debug-Callback;
+ const void *data = ctx-Debug-CallbackData;
+
+ /*
+ * When ctx-Debug-SyncOutput is GL_FALSE, the client is prepared for
+ * unsynchronous calls. When it is GL_TRUE, we will not spawn threads.
+ * In either case, we can call the callback unlocked.
+ */
+ _mesa_unlock_debug_state(ctx);
+
Re: [Mesa-dev] [PATCHv2 01/13] mesa: protect the debug state with a mutex
Patches 7, 8, and 9 are
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
I made a few minor comments on 7, but they should be trivial to resolve.
Patches 10 through 13 are
Acked-by: Ian Romanick ian.d.roman...@intel.com
I'd like to see some feed back on those last four from Ken and / or
Matt. I'd also like, if possible, for this to land this week.
On 07/09/2014 12:47 AM, Chia-I Wu wrote:
We are about to change mesa to spawn threads for deferred glCompileShader and
glLinkProgram, and we need to make sure those threads can send compiler
warnings/errors to the debug output safely.
Signed-off-by: Chia-I Wu o...@lunarg.com
---
src/mesa/main/errors.c | 172
+++--
src/mesa/main/mtypes.h | 1 +
2 files changed, 126 insertions(+), 47 deletions(-)
diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index aa0ff50..156eb0d 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -676,22 +676,41 @@ debug_pop_group(struct gl_debug_state *debug)
/**
- * Return debug state for the context. The debug state will be allocated
- * and initialized upon the first call.
+ * Lock and return debug state for the context. The debug state will be
+ * allocated and initialized upon the first call. When NULL is returned, the
+ * debug state is not locked.
*/
static struct gl_debug_state *
-_mesa_get_debug_state(struct gl_context *ctx)
+_mesa_lock_debug_state(struct gl_context *ctx)
{
+ mtx_lock(ctx-DebugMutex);
+
if (!ctx-Debug) {
ctx-Debug = debug_create();
if (!ctx-Debug) {
- _mesa_error(ctx, GL_OUT_OF_MEMORY, allocating debug state);
+ GET_CURRENT_CONTEXT(cur);
+ mtx_unlock(ctx-DebugMutex);
+
+ /*
+ * This function may be called from other threads. When that is the
+ * case, we cannot record this OOM error.
+ */
+ if (ctx == cur)
+_mesa_error(ctx, GL_OUT_OF_MEMORY, allocating debug state);
+
+ return NULL;
}
}
return ctx-Debug;
}
+static void
+_mesa_unlock_debug_state(struct gl_context *ctx)
+{
+ mtx_unlock(ctx-DebugMutex);
+}
+
/**
* Set the integer debug state specified by \p pname. This can be called
from
* _mesa_set_enable for example.
@@ -699,7 +718,7 @@ _mesa_get_debug_state(struct gl_context *ctx)
bool
_mesa_set_debug_state_int(struct gl_context *ctx, GLenum pname, GLint val)
{
- struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
+ struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
if (!debug)
return false;
@@ -716,6 +735,8 @@ _mesa_set_debug_state_int(struct gl_context *ctx, GLenum
pname, GLint val)
break;
}
+ _mesa_unlock_debug_state(ctx);
+
return true;
}
@@ -729,9 +750,12 @@ _mesa_get_debug_state_int(struct gl_context *ctx, GLenum
pname)
struct gl_debug_state *debug;
GLint val;
+ mtx_lock(ctx-DebugMutex);
debug = ctx-Debug;
- if (!debug)
+ if (!debug) {
+ mtx_unlock(ctx-DebugMutex);
return 0;
+ }
switch (pname) {
case GL_DEBUG_OUTPUT:
@@ -756,6 +780,8 @@ _mesa_get_debug_state_int(struct gl_context *ctx, GLenum
pname)
break;
}
+ mtx_unlock(ctx-DebugMutex);
+
return val;
}
@@ -769,9 +795,12 @@ _mesa_get_debug_state_ptr(struct gl_context *ctx, GLenum
pname)
struct gl_debug_state *debug;
void *val;
+ mtx_lock(ctx-DebugMutex);
debug = ctx-Debug;
- if (!debug)
+ if (!debug) {
+ mtx_unlock(ctx-DebugMutex);
return NULL;
+ }
switch (pname) {
case GL_DEBUG_CALLBACK_FUNCTION_ARB:
@@ -786,9 +815,49 @@ _mesa_get_debug_state_ptr(struct gl_context *ctx, GLenum
pname)
break;
}
+ mtx_unlock(ctx-DebugMutex);
+
return val;
}
+/**
+ * Insert a debug message. The mutex is assumed to be locked, and will be
+ * unlocked by this call.
+ */
+static void
+log_msg_locked_and_unlock(struct gl_context *ctx,
+ enum mesa_debug_source source,
+ enum mesa_debug_type type, GLuint id,
+ enum mesa_debug_severity severity,
+ GLint len, const char *buf)
+{
+ struct gl_debug_state *debug = ctx-Debug;
+
+ if (!debug_is_message_enabled(debug, source, type, id, severity)) {
+ _mesa_unlock_debug_state(ctx);
+ return;
+ }
+
+ if (ctx-Debug-Callback) {
+ GLenum gl_source = debug_source_enums[source];
+ GLenum gl_type = debug_type_enums[type];
+ GLenum gl_severity = debug_severity_enums[severity];
+ GLDEBUGPROC callback = ctx-Debug-Callback;
+ const void *data = ctx-Debug-CallbackData;
+
+ /*
+ * When ctx-Debug-SyncOutput is GL_FALSE, the client is prepared for
+ * unsynchronous calls. When it is GL_TRUE, we will not spawn threads.
+
[Mesa-dev] [PATCHv2 01/13] mesa: protect the debug state with a mutex
We are about to change mesa to spawn threads for deferred glCompileShader and
glLinkProgram, and we need to make sure those threads can send compiler
warnings/errors to the debug output safely.
Signed-off-by: Chia-I Wu o...@lunarg.com
---
src/mesa/main/errors.c | 172 +++--
src/mesa/main/mtypes.h | 1 +
2 files changed, 126 insertions(+), 47 deletions(-)
diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index aa0ff50..156eb0d 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -676,22 +676,41 @@ debug_pop_group(struct gl_debug_state *debug)
/**
- * Return debug state for the context. The debug state will be allocated
- * and initialized upon the first call.
+ * Lock and return debug state for the context. The debug state will be
+ * allocated and initialized upon the first call. When NULL is returned, the
+ * debug state is not locked.
*/
static struct gl_debug_state *
-_mesa_get_debug_state(struct gl_context *ctx)
+_mesa_lock_debug_state(struct gl_context *ctx)
{
+ mtx_lock(ctx-DebugMutex);
+
if (!ctx-Debug) {
ctx-Debug = debug_create();
if (!ctx-Debug) {
- _mesa_error(ctx, GL_OUT_OF_MEMORY, allocating debug state);
+ GET_CURRENT_CONTEXT(cur);
+ mtx_unlock(ctx-DebugMutex);
+
+ /*
+ * This function may be called from other threads. When that is the
+ * case, we cannot record this OOM error.
+ */
+ if (ctx == cur)
+_mesa_error(ctx, GL_OUT_OF_MEMORY, allocating debug state);
+
+ return NULL;
}
}
return ctx-Debug;
}
+static void
+_mesa_unlock_debug_state(struct gl_context *ctx)
+{
+ mtx_unlock(ctx-DebugMutex);
+}
+
/**
* Set the integer debug state specified by \p pname. This can be called from
* _mesa_set_enable for example.
@@ -699,7 +718,7 @@ _mesa_get_debug_state(struct gl_context *ctx)
bool
_mesa_set_debug_state_int(struct gl_context *ctx, GLenum pname, GLint val)
{
- struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
+ struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
if (!debug)
return false;
@@ -716,6 +735,8 @@ _mesa_set_debug_state_int(struct gl_context *ctx, GLenum
pname, GLint val)
break;
}
+ _mesa_unlock_debug_state(ctx);
+
return true;
}
@@ -729,9 +750,12 @@ _mesa_get_debug_state_int(struct gl_context *ctx, GLenum
pname)
struct gl_debug_state *debug;
GLint val;
+ mtx_lock(ctx-DebugMutex);
debug = ctx-Debug;
- if (!debug)
+ if (!debug) {
+ mtx_unlock(ctx-DebugMutex);
return 0;
+ }
switch (pname) {
case GL_DEBUG_OUTPUT:
@@ -756,6 +780,8 @@ _mesa_get_debug_state_int(struct gl_context *ctx, GLenum
pname)
break;
}
+ mtx_unlock(ctx-DebugMutex);
+
return val;
}
@@ -769,9 +795,12 @@ _mesa_get_debug_state_ptr(struct gl_context *ctx, GLenum
pname)
struct gl_debug_state *debug;
void *val;
+ mtx_lock(ctx-DebugMutex);
debug = ctx-Debug;
- if (!debug)
+ if (!debug) {
+ mtx_unlock(ctx-DebugMutex);
return NULL;
+ }
switch (pname) {
case GL_DEBUG_CALLBACK_FUNCTION_ARB:
@@ -786,9 +815,49 @@ _mesa_get_debug_state_ptr(struct gl_context *ctx, GLenum
pname)
break;
}
+ mtx_unlock(ctx-DebugMutex);
+
return val;
}
+/**
+ * Insert a debug message. The mutex is assumed to be locked, and will be
+ * unlocked by this call.
+ */
+static void
+log_msg_locked_and_unlock(struct gl_context *ctx,
+ enum mesa_debug_source source,
+ enum mesa_debug_type type, GLuint id,
+ enum mesa_debug_severity severity,
+ GLint len, const char *buf)
+{
+ struct gl_debug_state *debug = ctx-Debug;
+
+ if (!debug_is_message_enabled(debug, source, type, id, severity)) {
+ _mesa_unlock_debug_state(ctx);
+ return;
+ }
+
+ if (ctx-Debug-Callback) {
+ GLenum gl_source = debug_source_enums[source];
+ GLenum gl_type = debug_type_enums[type];
+ GLenum gl_severity = debug_severity_enums[severity];
+ GLDEBUGPROC callback = ctx-Debug-Callback;
+ const void *data = ctx-Debug-CallbackData;
+
+ /*
+ * When ctx-Debug-SyncOutput is GL_FALSE, the client is prepared for
+ * unsynchronous calls. When it is GL_TRUE, we will not spawn threads.
+ * In either case, we can call the callback unlocked.
+ */
+ _mesa_unlock_debug_state(ctx);
+ callback(gl_source, gl_type, id, gl_severity, len, buf, data);
+ }
+ else {
+ debug_log_message(ctx-Debug, source, type, id, severity, len, buf);
+ _mesa_unlock_debug_state(ctx);
+ }
+}
/**
* Log a client or driver debug message.
@@ -798,24 +867,12 @@ log_msg(struct gl_context *ctx, enum mesa_debug_source
source,
enum mesa_debug_type type, GLuint id,
enum mesa_debug_severity severity, GLint len, const
