[Mesa-dev] [PATCH 3/3] svga: only free shader variants from the context which created it

Brian Paul brianp at vmware.com
Tue Mar 5 23:57:52 UTC 2019


Since OpenGL shaders may be shared among contexts, we can wind up
with variants of a shader created with different contexts.
When we go to destroy a shader variant, we want to free it with the
same context that it was created with.

This patch implements solves that problem.

The Redway3D Watch demo exhibits this problem.
---
 src/gallium/drivers/svga/svga_context.c | 65 ++++++++++++++++++++++++++++++++-
 src/gallium/drivers/svga/svga_context.h | 20 ++++++++++
 src/gallium/drivers/svga/svga_shader.c  | 16 +++++++-
 src/gallium/drivers/svga/svga_shader.h  |  3 ++
 4 files changed, 102 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/svga/svga_context.c b/src/gallium/drivers/svga/svga_context.c
index 6b5f023..5857140 100644
--- a/src/gallium/drivers/svga/svga_context.c
+++ b/src/gallium/drivers/svga/svga_context.c
@@ -38,6 +38,7 @@
 #include "svga_resource_texture.h"
 #include "svga_resource_buffer.h"
 #include "svga_resource.h"
+#include "svga_shader.h"
 #include "svga_winsys.h"
 #include "svga_swtnl.h"
 #include "svga_draw.h"
@@ -111,6 +112,61 @@ free_zombie_resource_views(struct svga_context *svga)
 }
 
 
+/*
+ * Since OpenGL shaders may be shared among contexts, we can wind up
+ * with variants of a shader created with different contexts.
+ * When we go to destroy a shader variant, we want to free it with the
+ * same context that it was created with.  In svga_destroy_shader_variant()
+ * if the calling context doesn't match the variant's context, we call
+ * this function to add the variant to this context's list.
+ */
+void
+svga_save_zombie_shader_variant(struct svga_context *svga,
+                                struct svga_shader_variant *variant)
+{
+   struct svga_zombie_shader_variant_list *entry;
+
+   assert(variant->context == svga);
+
+   entry = MALLOC_STRUCT(svga_zombie_shader_variant_list);
+   if (!entry)
+      return;
+
+   entry->variant = variant;
+
+   /* We need a mutex since this function may be called from one thread
+    * while free_zombie_shader_variant() is called from another.
+    */
+   mtx_lock(&svga->zombie_shader_variant_mutex);
+   LIST_ADDTAIL(&entry->node, &svga->zombie_shader_variants);
+   mtx_unlock(&svga->zombie_shader_variant_mutex);
+}
+
+
+/*
+ * This function is called periodically (currently from svga_flush)
+ * to free any zombie shader variants attached to the context.
+ */
+static void
+free_zombie_shader_variants(struct svga_context *svga)
+{
+   struct svga_zombie_shader_variant_list *entry, *next;
+
+   mtx_lock(&svga->zombie_shader_variant_mutex);
+
+   LIST_FOR_EACH_ENTRY_SAFE(entry, next, &svga->zombie_shader_variants, node) {
+      LIST_DEL(&entry->node);  // remove this entry from the list
+      assert(entry->variant->context == svga);
+      svga_destroy_shader_variant(svga, entry->variant);
+      FREE(entry);
+   }
+
+   assert(LIST_IS_EMPTY(&svga->zombie_shader_variants));
+
+   mtx_unlock(&svga->zombie_shader_variant_mutex);
+}
+
+
 static void
 svga_destroy(struct pipe_context *pipe)
 {
@@ -121,6 +177,9 @@ svga_destroy(struct pipe_context *pipe)
    free_zombie_resource_views(svga);
    mtx_destroy(&svga->zombie_view_mutex);
 
+   free_zombie_shader_variants(svga);
+   mtx_destroy(&svga->zombie_shader_variant_mutex);
+
    /* free any alternate rasterizer states used for point sprite */
    for (i = 0; i < ARRAY_SIZE(svga->rasterizer_no_cull); i++) {
       if (svga->rasterizer_no_cull[i]) {
@@ -204,6 +263,9 @@ svga_context_create(struct pipe_screen *screen, void *priv, unsigned flags)
    LIST_INITHEAD(&svga->zombie_views);
    mtx_init(&svga->zombie_view_mutex, mtx_plain);
 
+   LIST_INITHEAD(&svga->zombie_shader_variants);
+   mtx_init(&svga->zombie_shader_variant_mutex, mtx_plain);
+
    svga->pipe.screen = screen;
    svga->pipe.priv = priv;
    svga->pipe.destroy = svga_destroy;
@@ -476,10 +538,11 @@ svga_context_flush(struct svga_context *svga,
 
    svgascreen->sws->fence_reference(svgascreen->sws, &fence, NULL);
 
-   /* We want to call this function periodically.  This seems to
+   /* We want to call these functions periodically.  This seems to
     * be a reasonable place to do so.
     */
    free_zombie_resource_views(svga);
+   free_zombie_shader_variants(svga);
 
    SVGA_STATS_TIME_POP(svga_sws(svga));
 }
diff --git a/src/gallium/drivers/svga/svga_context.h b/src/gallium/drivers/svga/svga_context.h
index 8c86018..99032c4 100644
--- a/src/gallium/drivers/svga/svga_context.h
+++ b/src/gallium/drivers/svga/svga_context.h
@@ -243,6 +243,18 @@ struct svga_zombie_view_list
 };
 
 
+/*
+ * For keeping a list of zombie shader variants.  A zombie shader variant is
+ * one that is supposed to be deleted, but the variant's parent context does
+ * not match the context passed to svga_destroy_shader_variant().
+ */
+struct svga_zombie_shader_variant_list
+{
+   struct svga_shader_variant *variant;
+   struct list_head node;  // there's a linked list of these structs
+};
+
+
 struct svga_velems_state {
    unsigned count;
    struct pipe_vertex_element velem[PIPE_MAX_ATTRIBS];
@@ -558,6 +570,10 @@ struct svga_context
    struct list_head zombie_views;
    mtx_t zombie_view_mutex;
 
+   /** List of zombie shader variants */
+   struct list_head zombie_shader_variants;
+   mtx_t zombie_shader_variant_mutex;
+
    /** performance / info queries for HUD */
    struct {
       uint64_t num_draw_calls;          /**< SVGA_QUERY_DRAW_CALLS */
@@ -709,6 +725,10 @@ void
 svga_save_zombie_view(struct svga_context *svga,
                       struct pipe_sampler_view *view);
 
+void
+svga_save_zombie_shader_variant(struct svga_context *svga,
+                                struct svga_shader_variant *variant);
+
 
 /***********************************************************************
  * Inline conversion functions.  These are better-typed than the
diff --git a/src/gallium/drivers/svga/svga_shader.c b/src/gallium/drivers/svga/svga_shader.c
index 22e4498..665f10b 100644
--- a/src/gallium/drivers/svga/svga_shader.c
+++ b/src/gallium/drivers/svga/svga_shader.c
@@ -466,6 +466,8 @@ svga_define_shader(struct svga_context *svga,
 
    variant->id = UTIL_BITMASK_INVALID_INDEX;
 
+   variant->context = svga;
+
    if (svga_have_gb_objects(svga)) {
       if (svga_have_vgpu10(svga))
          ret = define_gb_shader_vgpu10(svga, type, variant, codeLen);
@@ -546,12 +548,24 @@ svga_destroy_shader_variant(struct svga_context *svga,
                             SVGA3dShaderType type,
                             struct svga_shader_variant *variant)
 {
+   struct svga_screen *svgascreen = svga_screen(svga->pipe.screen);
    enum pipe_error ret = PIPE_OK;
 
+   if (variant->context != svga) {
+      if (svga_screen_context_exists(svgascreen, variant->context)) {
+         svga_save_zombie_shader_variant(variant->context, variant);
+      } else {
+         debug_printf("The parent context of a shader variant no longer "
+                      " exists.  The shader variant will be leaked.\n");
+      }
+      return;
+   }
+
    if (svga_have_gb_objects(svga) && variant->gb_shader) {
       if (svga_have_vgpu10(svga)) {
          struct svga_winsys_context *swc = svga->swc;
          swc->shader_destroy(swc, variant->gb_shader);
+
          ret = SVGA3D_vgpu10_DestroyShader(svga->swc, variant->id);
          if (ret != PIPE_OK) {
             /* flush and try again */
@@ -562,7 +576,7 @@ svga_destroy_shader_variant(struct svga_context *svga,
          util_bitmask_clear(svga->shader_id_bm, variant->id);
       }
       else {
-         struct svga_winsys_screen *sws = svga_screen(svga->pipe.screen)->sws;
+         struct svga_winsys_screen *sws = svgascreen->sws;
          sws->shader_destroy(sws, variant->gb_shader);
       }
       variant->gb_shader = NULL;
diff --git a/src/gallium/drivers/svga/svga_shader.h b/src/gallium/drivers/svga/svga_shader.h
index 68991e7..d61aae4 100644
--- a/src/gallium/drivers/svga/svga_shader.h
+++ b/src/gallium/drivers/svga/svga_shader.h
@@ -132,6 +132,9 @@ struct svga_shader_variant
 {
    const struct svga_shader *shader;
 
+   /** The context which created this variant */
+   struct svga_context *context;
+
    /** Parameters used to generate this variant */
    struct svga_compile_key key;
 
-- 
1.8.5.6



More information about the mesa-dev mailing list