[Mesa-dev] [PATCH 2/3] svga: only free sampler views from the context which created it

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


Some applications, like google-chrome, cause the state tracker to
destroy sampler/resource views from a context other than the one
which was used to create it.  Previously, we just leaked the view.

Now, when we're in this situation we declare the view a zombie and
add it to a list of zombie views associated with the context which
created the context.

VMware bug 2274734.
---
 src/gallium/drivers/svga/svga_context.c      | 67 ++++++++++++++++++++++++++++
 src/gallium/drivers/svga/svga_context.h      | 21 +++++++++
 src/gallium/drivers/svga/svga_pipe_sampler.c | 45 +++++++++++--------
 3 files changed, 115 insertions(+), 18 deletions(-)

diff --git a/src/gallium/drivers/svga/svga_context.c b/src/gallium/drivers/svga/svga_context.c
index 1284d2f..6b5f023 100644
--- a/src/gallium/drivers/svga/svga_context.c
+++ b/src/gallium/drivers/svga/svga_context.c
@@ -54,6 +54,63 @@ DEBUG_GET_ONCE_BOOL_OPTION(no_line_width, "SVGA_NO_LINE_WIDTH", FALSE);
 DEBUG_GET_ONCE_BOOL_OPTION(force_hw_line_stipple, "SVGA_FORCE_HW_LINE_STIPPLE", FALSE);
 
 
+/*
+ * In some circumstances (such as running google-chrome) the state
+ * tracker may try to delete a resource view from a context different
+ * than when it was created.  We don't want to do that.
+ * In that situation, svga_sampler_view_destroy() calls this function
+ * to save the view for later deletion.  The context here is expected
+ * to be the context which created the view.
+ */
+void
+svga_save_zombie_view(struct svga_context *svga,
+                      struct pipe_sampler_view *view)
+{
+   struct svga_pipe_sampler_view *sv = svga_pipe_sampler_view(view);
+   struct svga_zombie_view_list *entry;
+
+   assert(view->context == &svga->pipe);
+
+   entry = MALLOC_STRUCT(svga_zombie_view_list);
+   if (!entry)
+      return;
+
+   entry->view = sv;
+
+   /* We need a mutex since this function may be called from one thread
+    * while free_zombie_resource_views() is called from another.
+    */
+   mtx_lock(&svga->zombie_view_mutex);
+   LIST_ADDTAIL(&entry->node, &svga->zombie_views);
+   mtx_unlock(&svga->zombie_view_mutex);
+}
+
+
+/*
+ * This function is called periodically (currently from svga_flush)
+ * to free any zombie resource views attached to the context.
+ */
+static void
+free_zombie_resource_views(struct svga_context *svga)
+{
+   struct pipe_context *pipe = &svga->pipe;
+   struct svga_zombie_view_list *entry, *next;
+
+   mtx_lock(&svga->zombie_view_mutex);
+
+   LIST_FOR_EACH_ENTRY_SAFE(entry, next, &svga->zombie_views, node) {
+      LIST_DEL(&entry->node);  // remove this entry from the list
+      assert(entry->view->base.context == pipe);
+      pipe->sampler_view_destroy(pipe, &entry->view->base);
+      FREE(entry);
+   }
+
+   assert(LIST_IS_EMPTY(&svga->zombie_views));
+
+   mtx_unlock(&svga->zombie_view_mutex);
+}
+
+
 static void
 svga_destroy(struct pipe_context *pipe)
 {
@@ -61,6 +118,9 @@ svga_destroy(struct pipe_context *pipe)
    struct svga_context *svga = svga_context(pipe);
    unsigned shader, i;
 
+   free_zombie_resource_views(svga);
+   mtx_destroy(&svga->zombie_view_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]) {
@@ -141,6 +201,8 @@ svga_context_create(struct pipe_screen *screen, void *priv, unsigned flags)
       goto done;
 
    LIST_INITHEAD(&svga->dirty_buffers);
+   LIST_INITHEAD(&svga->zombie_views);
+   mtx_init(&svga->zombie_view_mutex, mtx_plain);
 
    svga->pipe.screen = screen;
    svga->pipe.priv = priv;
@@ -414,6 +476,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
+    * be a reasonable place to do so.
+    */
+   free_zombie_resource_views(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 2ec6b3f..8c86018 100644
--- a/src/gallium/drivers/svga/svga_context.h
+++ b/src/gallium/drivers/svga/svga_context.h
@@ -231,6 +231,18 @@ svga_pipe_sampler_view(struct pipe_sampler_view *v)
 }
 
 
+/*
+ * For keeping a list of zombie sampler views.  A zombie sampler view is
+ * one that is supposed to be deleted, but the view's parent context is not
+ * the context passed to pipe_context::sampler_view_destroy().
+ */
+struct svga_zombie_view_list
+{
+   struct svga_pipe_sampler_view *view;
+   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];
@@ -542,6 +554,10 @@ struct svga_context
    /** List of buffers with queued transfers */
    struct list_head dirty_buffers;
 
+   /** List of zombie resource views */
+   struct list_head zombie_views;
+   mtx_t zombie_view_mutex;
+
    /** performance / info queries for HUD */
    struct {
       uint64_t num_draw_calls;          /**< SVGA_QUERY_DRAW_CALLS */
@@ -689,6 +705,11 @@ void svga_toggle_render_condition(struct svga_context *svga,
                                   boolean render_condition_enabled,
                                   boolean on);
 
+void
+svga_save_zombie_view(struct svga_context *svga,
+                      struct pipe_sampler_view *view);
+
+
 /***********************************************************************
  * Inline conversion functions.  These are better-typed than the
  * macros used previously:
diff --git a/src/gallium/drivers/svga/svga_pipe_sampler.c b/src/gallium/drivers/svga/svga_pipe_sampler.c
index b32bb09..1c440f2 100644
--- a/src/gallium/drivers/svga/svga_pipe_sampler.c
+++ b/src/gallium/drivers/svga/svga_pipe_sampler.c
@@ -401,32 +401,41 @@ static void
 svga_sampler_view_destroy(struct pipe_context *pipe,
                           struct pipe_sampler_view *view)
 {
+   struct svga_screen *svgascreen = svga_screen(pipe->screen);
    struct svga_context *svga = svga_context(pipe);
    struct svga_pipe_sampler_view *sv = svga_pipe_sampler_view(view);
 
-   if (svga_have_vgpu10(svga) && sv->id != SVGA3D_INVALID_ID) {
-      if (view->context != pipe) {
-         /* The SVGA3D device will generate an error (and on Linux, cause
-          * us to abort) if we try to destroy a shader resource view from
-          * a context other than the one it was created with.  Skip the
-          * SVGA3D_vgpu10_DestroyShaderResourceView() and leak the sampler
-          * view for now.  This should only sometimes happen when a shared
-          * texture is deleted.
-          */
-         _debug_printf("context mismatch in %s\n", __func__);
+   if (view->context != pipe) {
+      /* The SVGA3D device will generate an error (and on Linux, cause
+       * us to abort) if we try to destroy a shader resource view from
+       * a context other than the one it was created with.
+       * Declare this a zombie view and attach it to the creating context.
+       */
+
+      /* make sure the parent context of the view still exists */
+      if (svga_screen_context_exists(svgascreen,
+                                     svga_context(view->context))) {
+         svga_save_zombie_view(svga_context(view->context), view);
+      } else {
+         debug_printf("The parent context of a sampler view no longer "
+                      " exists.  The view will be leaked.\n");
       }
-      else {
-         enum pipe_error ret;
 
-         svga_hwtnl_flush_retry(svga); /* XXX is this needed? */
+      return;
+   }
+
+   if (svga_have_vgpu10(svga) && sv->id != SVGA3D_INVALID_ID) {
+      enum pipe_error ret;
+
+      /* This just flushes queued up DX9 primitives */
+      svga_hwtnl_flush_retry(svga);
 
+      ret = SVGA3D_vgpu10_DestroyShaderResourceView(svga->swc, sv->id);
+      if (ret != PIPE_OK) {
+         svga_context_flush(svga, NULL);
          ret = SVGA3D_vgpu10_DestroyShaderResourceView(svga->swc, sv->id);
-         if (ret != PIPE_OK) {
-            svga_context_flush(svga, NULL);
-            ret = SVGA3D_vgpu10_DestroyShaderResourceView(svga->swc, sv->id);
-         }
-         util_bitmask_clear(svga->sampler_view_id_bm, sv->id);
       }
+      util_bitmask_clear(svga->sampler_view_id_bm, sv->id);
    }
 
    pipe_resource_reference(&sv->base.texture, NULL);
-- 
1.8.5.6



More information about the mesa-dev mailing list