From 40edc87956aac09b60d17eb4e479bcb02b8fa20c Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Thu, 12 Aug 2021 20:17:48 +0000 Subject: [PATCH] panfrost: Protect the variants array with a lock Without a lock, two threads may bind the same shader CSO simultaneously, allocate the same variant simultaneously, and then race each other in the compiler. This manifests in various ways, most commonly failing the assertion that UBO pushing has only run once. The simple_mtx_t solution is used in Iris. Fixes the crash in: dEQP-EGL.functional.sharing.gles2.multithread.simple.buffers.bufferdata_render Signed-off-by: Alyssa Rosenzweig Cc: mesa-stable Part-of: --- src/gallium/drivers/panfrost/pan_context.c | 11 +++++++++++ src/gallium/drivers/panfrost/pan_context.h | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c index b7ff1ae04a0..bff821bd9bc 100644 --- a/src/gallium/drivers/panfrost/pan_context.c +++ b/src/gallium/drivers/panfrost/pan_context.c @@ -297,6 +297,8 @@ panfrost_create_shader_state( struct panfrost_device *dev = pan_device(pctx->screen); so->base = *cso; + simple_mtx_init(&so->lock, mtx_plain); + /* Token deep copy to prevent memory corruption */ if (cso->type == PIPE_SHADER_IR_TGSI) @@ -339,6 +341,8 @@ panfrost_delete_shader_state( panfrost_bo_unreference(shader_state->linkage.bo); } + simple_mtx_destroy(&cso->lock); + free(cso->variants); free(so); } @@ -451,6 +455,8 @@ panfrost_bind_shader_state( signed variant = -1; struct panfrost_shader_variants *variants = (struct panfrost_shader_variants *) hwcso; + simple_mtx_lock(&variants->lock); + for (unsigned i = 0; i < variants->variant_count; ++i) { if (panfrost_variant_matches(ctx, &variants->variants[i], type)) { variant = i; @@ -528,6 +534,11 @@ panfrost_bind_shader_state( update_so_info(&shader_state->stream_output, shader_state->info.outputs_written); } + + /* TODO: it would be more efficient to release the lock before + * compiling instead of after, but that can race if thread A compiles a + * variant while thread B searches for that same variant */ + simple_mtx_unlock(&variants->lock); } static void * diff --git a/src/gallium/drivers/panfrost/pan_context.h b/src/gallium/drivers/panfrost/pan_context.h index 7e49c2c4faf..98394c1bcf8 100644 --- a/src/gallium/drivers/panfrost/pan_context.h +++ b/src/gallium/drivers/panfrost/pan_context.h @@ -44,6 +44,7 @@ #include "pipe/p_state.h" #include "util/u_blitter.h" #include "util/hash_table.h" +#include "util/simple_mtx.h" #include "midgard/midgard_compile.h" #include "compiler/shader_enums.h" @@ -286,6 +287,9 @@ struct panfrost_shader_variants { struct pipe_compute_state cbase; }; + /** Lock for the variants array */ + simple_mtx_t lock; + struct panfrost_shader_state *variants; unsigned variant_space;