[Mesa-dev] [PATCH 5/6] st/mesa: guard st_finalize_texture by the texture validation mutex

Nicolai Hähnle nhaehnle at gmail.com
Fri Oct 6 20:38:15 UTC 2017


From: Nicolai Hähnle <nicolai.haehnle at amd.com>

Since finalization of textures is done lazily, a well-formed GL program
might enter the main part of st_finalize_texture simultaneously in two
threads.

The mutex guards against corruption of our internal data structures.
However, finalization may also involve image copies (for copying image
data into a single mip-tree). We still lack proper synchronization of
these copy operations in a multi-context situation like:

 Context 1                       Context 2
 ---------                       ---------
 glTexImage*() operations
 glFinish()
 render using texture
 (implied image copy operations)
                                 render using texture
                                 glFlush()
 glFlush()

Since glFinish() was called after setting up the texture, the GL spec
guarantees that the rendering operations in both contexts will use the
expected texel data. However, we run the main part of st_finalize_texture
during context 1's rendering operations, and if that involves texture
image copies, then those may still be queued up and un-flushed when
context 2 is explicitly flushed.
---
 src/mesa/state_tracker/st_cb_texture.c   | 41 ++++++++++++++++++++++++++++----
 src/mesa/state_tracker/st_sampler_view.c | 22 +++++++++++++----
 src/mesa/state_tracker/st_sampler_view.h |  4 ++++
 3 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
index 6d9e40baa95..44e0dd11dcd 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -2480,33 +2480,60 @@ st_finalize_texture(struct gl_context *ctx,
        stObj->base.BaseLevel >= stObj->validated_first_level &&
        stObj->lastLevel <= stObj->validated_last_level) {
       return GL_TRUE;
    }
 
    /* If this texture comes from a window system, there is nothing else to do. */
    if (stObj->surface_based) {
       return GL_TRUE;
    }
 
+   /* The mutex guards against corruption of our internal data structures.
+    *
+    * TODO: Finalization may also involve image copies (for copying image
+    * data into a single mip-tree). We still lack proper synchronization of
+    * these copy operations in a multi-context situation like:
+    *
+    *  Context 1                            Context 2
+    *  ---------                            ---------
+    *  glTexImage*() operations
+    *  glFinish() or other sync
+    *
+    *  render using texture
+    *  (triggers image copy operations)
+    *                                       render using texture
+    *                                       glFlush()
+    *  glFlush()
+    *
+    * Since glFinish() was called after setting up the texture, the GL spec
+    * guarantees that the rendering operations in both contexts will use the
+    * expected texel data. However, we run the main part of st_finalize_texture
+    * during context 1's rendering operations, and if that involves texture
+    * image copies, then those may still be queued up and un-flushed when
+    * context 2 is explicitly flushed.
+    */
+   bool ok = false;
+   mtx_lock(&stObj->validate_mutex);
+
    firstImage = st_texture_image_const(stObj->base.Image[cubeMapFace][stObj->base.BaseLevel]);
    assert(firstImage);
 
    /* If both firstImage and stObj point to a texture which can contain
     * all active images, favour firstImage.  Note that because of the
     * completeness requirement, we know that the image dimensions
     * will match.
     */
    if (firstImage->pt &&
        firstImage->pt != stObj->pt &&
        (!stObj->pt || firstImage->pt->last_level >= stObj->pt->last_level)) {
       pipe_resource_reference(&stObj->pt, firstImage->pt);
-      st_texture_release_all_sampler_views(st, stObj);
+      st_texture_release_all_sampler_views_locked(st, stObj);
    }
 
    /* Find gallium format for the Mesa texture */
    firstImageFormat =
       st_mesa_format_to_pipe_format(st, firstImage->base.TexFormat);
 
    /* Find size of level=0 Gallium mipmap image, plus number of texture layers */
    {
       unsigned width;
       uint16_t height, depth;
@@ -2548,21 +2575,22 @@ st_finalize_texture(struct gl_context *ctx,
          }
 
          /* At this point, the texture may be incomplete (mismatched cube
           * face sizes, for example).  If that's the case, give up, but
           * don't return GL_FALSE as that would raise an incorrect
           * GL_OUT_OF_MEMORY error.  See Piglit fbo-incomplete-texture-03 test.
           */
          if (!stObj->base._BaseComplete) {
             _mesa_test_texobj_completeness(ctx, &stObj->base);
             if (!stObj->base._BaseComplete) {
-               return TRUE;
+               ok = true;
+               goto out;
             }
          }
       }
 
       ptNumSamples = firstImage->base.NumSamples;
    }
 
    /* If we already have a gallium texture, check that it matches the texture
     * object's format, target, size, num_levels, etc.
     */
@@ -2573,21 +2601,21 @@ st_finalize_texture(struct gl_context *ctx,
           stObj->pt->width0 != ptWidth ||
           stObj->pt->height0 != ptHeight ||
           stObj->pt->depth0 != ptDepth ||
           stObj->pt->nr_samples != ptNumSamples ||
           stObj->pt->array_size != ptLayers)
       {
          /* The gallium texture does not match the Mesa texture so delete the
           * gallium texture now.  We'll make a new one below.
           */
          pipe_resource_reference(&stObj->pt, NULL);
-         st_texture_release_all_sampler_views(st, stObj);
+         st_texture_release_all_sampler_views_locked(st, stObj);
          st->dirty |= ST_NEW_FRAMEBUFFER;
       }
    }
 
    /* May need to create a new gallium texture:
     */
    if (!stObj->pt) {
       GLuint bindings = default_bindings(st, firstImageFormat);
 
       stObj->pt = st_texture_create(st,
@@ -2595,21 +2623,21 @@ st_finalize_texture(struct gl_context *ctx,
                                     firstImageFormat,
                                     stObj->lastLevel,
                                     ptWidth,
                                     ptHeight,
                                     ptDepth,
                                     ptLayers, ptNumSamples,
                                     bindings);
 
       if (!stObj->pt) {
          _mesa_error(ctx, GL_OUT_OF_MEMORY, "glTexImage");
-         return GL_FALSE;
+         goto out;
       }
    }
 
    /* Pull in any images not in the object's texture:
     */
    for (face = 0; face < nr_faces; face++) {
       GLuint level;
       for (level = stObj->base.BaseLevel; level <= stObj->lastLevel; level++) {
          struct st_texture_image *stImage =
             st_texture_image(stObj->base.Image[face][level]);
@@ -2639,22 +2667,25 @@ st_finalize_texture(struct gl_context *ctx,
                /* src image fits expected dest mipmap level size */
                copy_image_data_to_texture(st, stObj, level, stImage);
             }
          }
       }
    }
 
    stObj->validated_first_level = stObj->base.BaseLevel;
    stObj->validated_last_level = stObj->lastLevel;
    stObj->needs_validation = false;
+   ok = true;
 
-   return GL_TRUE;
+out:
+   mtx_unlock(&stObj->validate_mutex);
+   return ok;
 }
 
 /**
  * Allocate a new pipe_resource object
  * width0, height0, depth0 are the dimensions of the level 0 image
  * (the highest resolution).  last_level indicates how many mipmap levels
  * to allocate storage for.  For non-mipmapped textures, this will be zero.
  */
 static struct pipe_resource *
 st_texture_create_from_memory(struct st_context *st,
diff --git a/src/mesa/state_tracker/st_sampler_view.c b/src/mesa/state_tracker/st_sampler_view.c
index a06b6e64aaf..638fac671b2 100644
--- a/src/mesa/state_tracker/st_sampler_view.c
+++ b/src/mesa/state_tracker/st_sampler_view.c
@@ -136,34 +136,48 @@ st_texture_release_sampler_view(struct st_context *st,
          break;
       }
    }
    mtx_unlock(&stObj->validate_mutex);
 }
 
 
 /**
  * Release all sampler views attached to the given texture object, regardless
  * of the context.
+ *
+ * Must be called with the validate_mutex locked.
  */
 void
-st_texture_release_all_sampler_views(struct st_context *st,
-                                     struct st_texture_object *stObj)
+st_texture_release_all_sampler_views_locked(struct st_context *st,
+                                            struct st_texture_object *stObj)
 {
    GLuint i;
 
+   for (i = 0; i < stObj->num_sampler_views; ++i)
+      pipe_sampler_view_release(st->pipe, &stObj->sampler_views[i].view);
+}
+
+
+/**
+ * Release all sampler views attached to the given texture object, regardless
+ * of the context.
+ */
+void
+st_texture_release_all_sampler_views(struct st_context *st,
+                                     struct st_texture_object *stObj)
+{
    /* This function is called from code paths that do not correspond to a
     * modification of the texture, so it may race with other accesses of the
     * sampler views array in a well-formed GL program.
     */
    mtx_lock(&stObj->validate_mutex);
-   for (i = 0; i < stObj->num_sampler_views; ++i)
-      pipe_sampler_view_release(st->pipe, &stObj->sampler_views[i].view);
+   st_texture_release_all_sampler_views_locked(st, stObj);
    mtx_unlock(&stObj->validate_mutex);
 }
 
 
 void
 st_texture_free_sampler_views(struct st_texture_object *stObj)
 {
    free(stObj->sampler_views);
    stObj->sampler_views = NULL;
    stObj->num_sampler_views = 0;
diff --git a/src/mesa/state_tracker/st_sampler_view.h b/src/mesa/state_tracker/st_sampler_view.h
index 9e5b964976e..9a7e54cff4e 100644
--- a/src/mesa/state_tracker/st_sampler_view.h
+++ b/src/mesa/state_tracker/st_sampler_view.h
@@ -54,20 +54,24 @@ st_create_texture_sampler_view(struct pipe_context *pipe,
 {
    return st_create_texture_sampler_view_format(pipe, texture,
                                                 texture->format);
 }
 
 
 extern void
 st_texture_release_sampler_view(struct st_context *st,
                                 struct st_texture_object *stObj);
 
+void
+st_texture_release_all_sampler_views_locked(struct st_context *st,
+                                            struct st_texture_object *stObj);
+
 extern void
 st_texture_release_all_sampler_views(struct st_context *st,
                                      struct st_texture_object *stObj);
 
 void
 st_texture_free_sampler_views(struct st_texture_object *stObj);
 
 struct pipe_sampler_view *
 st_texture_get_current_sampler_view(const struct st_context *st,
                                     const struct st_texture_object *stObj);
-- 
2.11.0



More information about the mesa-dev mailing list