[Mesa-dev] [PATCHv2 01/13] mesa: protect the debug state with a mutex
Ian Romanick
idr at freedesktop.org
Wed Aug 13 17:45:03 PDT 2014
Patches 7, 8, and 9 are
Reviewed-by: Ian Romanick <ian.d.romanick at 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.romanick at 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 <olv at 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 char *buf)
> {
> - struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
> + struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
>
> if (!debug)
> return;
>
> - if (!debug_is_message_enabled(debug, source, type, id, severity))
> - return;
> -
> - if (debug->Callback) {
> - GLenum gl_type = debug_type_enums[type];
> - GLenum gl_severity = debug_severity_enums[severity];
> -
> - debug->Callback(debug_source_enums[source], gl_type, id, gl_severity,
> - len, buf, debug->CallbackData);
> - return;
> - }
> -
> - debug_log_message(debug, source, type, id, severity, len, buf);
> + log_msg_locked_and_unlock(ctx, source, type, id, severity, len, buf);
> }
>
>
> @@ -956,7 +1013,7 @@ _mesa_GetDebugMessageLog(GLuint count, GLsizei logSize, GLenum *sources,
> return 0;
> }
>
> - debug = _mesa_get_debug_state(ctx);
> + debug = _mesa_lock_debug_state(ctx);
> if (!debug)
> return 0;
>
> @@ -991,6 +1048,8 @@ _mesa_GetDebugMessageLog(GLuint count, GLsizei logSize, GLenum *sources,
> debug_delete_messages(debug, 1);
> }
>
> + _mesa_unlock_debug_state(ctx);
> +
> return ret;
> }
>
> @@ -1027,7 +1086,7 @@ _mesa_DebugMessageControl(GLenum gl_source, GLenum gl_type,
> return;
> }
>
> - debug = _mesa_get_debug_state(ctx);
> + debug = _mesa_lock_debug_state(ctx);
> if (!debug)
> return;
>
> @@ -1039,6 +1098,8 @@ _mesa_DebugMessageControl(GLenum gl_source, GLenum gl_type,
> else {
> debug_set_message_enable_all(debug, source, type, severity, enabled);
> }
> +
> + _mesa_unlock_debug_state(ctx);
> }
>
>
> @@ -1046,10 +1107,11 @@ void GLAPIENTRY
> _mesa_DebugMessageCallback(GLDEBUGPROC callback, const void *userParam)
> {
> GET_CURRENT_CONTEXT(ctx);
> - struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
> + struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
> if (debug) {
> debug->Callback = callback;
> debug->CallbackData = userParam;
> + _mesa_unlock_debug_state(ctx);
> }
> }
>
> @@ -1059,18 +1121,10 @@ _mesa_PushDebugGroup(GLenum source, GLuint id, GLsizei length,
> const GLchar *message)
> {
> GET_CURRENT_CONTEXT(ctx);
> - struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
> const char *callerstr = "glPushDebugGroup";
> + struct gl_debug_state *debug;
> struct gl_debug_message *emptySlot;
>
> - if (!debug)
> - return;
> -
> - if (debug->GroupStackDepth >= MAX_DEBUG_GROUP_STACK_DEPTH-1) {
> - _mesa_error(ctx, GL_STACK_OVERFLOW, "%s", callerstr);
> - return;
> - }
> -
> switch(source) {
> case GL_DEBUG_SOURCE_APPLICATION:
> case GL_DEBUG_SOURCE_THIRD_PARTY:
> @@ -1086,10 +1140,15 @@ _mesa_PushDebugGroup(GLenum source, GLuint id, GLsizei length,
> if (!validate_length(ctx, callerstr, length))
> return; /* GL_INVALID_VALUE */
>
> - log_msg(ctx, gl_enum_to_debug_source(source),
> - MESA_DEBUG_TYPE_PUSH_GROUP, id,
> - MESA_DEBUG_SEVERITY_NOTIFICATION, length,
> - message);
> + debug = _mesa_lock_debug_state(ctx);
> + if (!debug)
> + return;
> +
> + if (debug->GroupStackDepth >= MAX_DEBUG_GROUP_STACK_DEPTH-1) {
> + _mesa_unlock_debug_state(ctx);
> + _mesa_error(ctx, GL_STACK_OVERFLOW, "%s", callerstr);
> + return;
> + }
>
> /* pop reuses the message details from push so we store this */
> emptySlot = debug_get_group_message(debug);
> @@ -1101,6 +1160,12 @@ _mesa_PushDebugGroup(GLenum source, GLuint id, GLsizei length,
> length, message);
>
> debug_push_group(debug);
> +
> + log_msg_locked_and_unlock(ctx,
> + gl_enum_to_debug_source(source),
> + MESA_DEBUG_TYPE_PUSH_GROUP, id,
> + MESA_DEBUG_SEVERITY_NOTIFICATION, length,
> + message);
> }
>
>
> @@ -1108,35 +1173,43 @@ void GLAPIENTRY
> _mesa_PopDebugGroup(void)
> {
> GET_CURRENT_CONTEXT(ctx);
> - struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
> const char *callerstr = "glPopDebugGroup";
> - struct gl_debug_message *gdmessage;
> + struct gl_debug_state *debug;
> + struct gl_debug_message *gdmessage, msg;
>
> + debug = _mesa_lock_debug_state(ctx);
> if (!debug)
> return;
>
> if (debug->GroupStackDepth <= 0) {
> + _mesa_unlock_debug_state(ctx);
> _mesa_error(ctx, GL_STACK_UNDERFLOW, "%s", callerstr);
> return;
> }
>
> debug_pop_group(debug);
>
> + /* make a shallow copy */
> gdmessage = debug_get_group_message(debug);
> - log_msg(ctx, gdmessage->source,
> - gl_enum_to_debug_type(GL_DEBUG_TYPE_POP_GROUP),
> - gdmessage->id,
> - gl_enum_to_debug_severity(GL_DEBUG_SEVERITY_NOTIFICATION),
> - gdmessage->length, gdmessage->message);
> -
> - debug_message_clear(gdmessage);
> + msg = *gdmessage;
> + gdmessage->message = NULL;
> + gdmessage->length = 0;
> +
> + log_msg_locked_and_unlock(ctx,
> + msg.source,
> + gl_enum_to_debug_type(GL_DEBUG_TYPE_POP_GROUP),
> + msg.id,
> + gl_enum_to_debug_severity(GL_DEBUG_SEVERITY_NOTIFICATION),
> + msg.length, msg.message);
> +
> + debug_message_clear(&msg);
> }
>
>
> void
> _mesa_init_errors(struct gl_context *ctx)
> {
> - /* no-op */
> + mtx_init(&ctx->DebugMutex, mtx_plain);
> }
>
>
> @@ -1148,6 +1221,8 @@ _mesa_free_errors_data(struct gl_context *ctx)
> /* set to NULL just in case it is used before context is completely gone. */
> ctx->Debug = NULL;
> }
> +
> + mtx_destroy(&ctx->DebugMutex);
> }
>
>
> @@ -1362,6 +1437,8 @@ _mesa_error( struct gl_context *ctx, GLenum error, const char *fmtString, ... )
> debug_get_id(&error_msg_id);
>
> do_output = should_output(ctx, error, fmtString);
> +
> + mtx_lock(&ctx->DebugMutex);
> if (ctx->Debug) {
> do_log = debug_is_message_enabled(ctx->Debug,
> MESA_DEBUG_SOURCE_API,
> @@ -1372,6 +1449,7 @@ _mesa_error( struct gl_context *ctx, GLenum error, const char *fmtString, ... )
> else {
> do_log = GL_FALSE;
> }
> + mtx_unlock(&ctx->DebugMutex);
>
> if (do_output || do_log) {
> char s[MAX_DEBUG_MESSAGE_LENGTH], s2[MAX_DEBUG_MESSAGE_LENGTH];
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index a7126fd..5964576 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -4191,6 +4191,7 @@ struct gl_context
> GLuint ErrorDebugCount;
>
> /* GL_ARB_debug_output/GL_KHR_debug */
> + mtx_t DebugMutex;
> struct gl_debug_state *Debug;
>
> GLenum RenderMode; /**< either GL_RENDER, GL_SELECT, GL_FEEDBACK */
>
More information about the mesa-dev
mailing list