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 <alyssa@collabora.com>
Cc: mesa-stable
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12525>

diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
index b7ff1ae04a0e9b5c2d1ee4e4cc282a89eaea4f7d..bff821bd9bcfa7b621c1c05bff6994f1b4bd2fdc 100644 (file)
--- 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 7e49c2c4faf2ed02e41c6a5c5ffa04097ef7b17f..98394c1bcf853b010a78f2dfb535d2c5d4e3c2fa 100644 (file)
--- 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;