Patches 7, 8, and 9 are Reviewed-by: Ian Romanick <[email protected]>
I made a few minor comments on 7, but they should be trivial to resolve. Patches 10 through 13 are Acked-by: Ian Romanick <[email protected]> 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 <[email protected]> > --- > 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 */ > _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
