From: Nicolai Hähnle <nicolai.haeh...@amd.com> Some locking is unfortunately required, because well-formed GL programs can have multiple threads racing to access the same texture, e.g.: two threads/contexts rendering from the same texture, or one thread destroying a context while the other is rendering from or modifying a texture.
Since even the simple mutex caused noticable slowdowns in the piglit drawoverhead micro-benchmark, this patch uses a slightly more involved approach to keep locks out of the fast path: - the initial lookup of sampler views happens without taking a lock - a per-texture lock is only taken when we have to modify the sampler view(s) - since each thread mostly operates only on the entry corresponding to its context, the main issue is re-allocation of the sampler view array when it needs to be grown, but the old copy is not freed Old copies of the sampler views array are kept around in a linked list until the entire texture object is deleted. The total memory wasted in this way is roughly equal to the size of the current sampler views array. Fixes non-deterministic memory corruption in some dEQP-EGL.functional.sharing.gles2.multithread.* tests, e.g. dEQP-EGL.functional.sharing.gles2.multithread.simple.images.texture_source.create_texture_render --- src/mesa/state_tracker/st_atom_sampler.c | 21 +-- src/mesa/state_tracker/st_cb_texture.c | 12 ++ src/mesa/state_tracker/st_sampler_view.c | 270 ++++++++++++++++++++++--------- src/mesa/state_tracker/st_sampler_view.h | 3 + src/mesa/state_tracker/st_texture.h | 40 ++++- 5 files changed, 250 insertions(+), 96 deletions(-) diff --git a/src/mesa/state_tracker/st_atom_sampler.c b/src/mesa/state_tracker/st_atom_sampler.c index ff3f49fa4e4..6d83f963f0a 100644 --- a/src/mesa/state_tracker/st_atom_sampler.c +++ b/src/mesa/state_tracker/st_atom_sampler.c @@ -36,20 +36,21 @@ #include "main/mtypes.h" #include "main/glformats.h" #include "main/samplerobj.h" #include "main/teximage.h" #include "main/texobj.h" #include "st_context.h" #include "st_cb_texture.h" #include "st_format.h" #include "st_atom.h" +#include "st_sampler_view.h" #include "st_texture.h" #include "pipe/p_context.h" #include "pipe/p_defines.h" #include "cso_cache/cso_context.h" #include "util/u_format.h" /** @@ -157,40 +158,32 @@ st_convert_sampler(const struct st_context *st, (sampler->wrap_s | sampler->wrap_t | sampler->wrap_r) & 0x1 && (msamp->BorderColor.ui[0] || msamp->BorderColor.ui[1] || msamp->BorderColor.ui[2] || msamp->BorderColor.ui[3])) { const GLboolean is_integer = texobj->_IsIntegerFormat; GLenum texBaseFormat = _mesa_base_tex_image(texobj)->_BaseFormat; if (st->apply_texture_swizzle_to_border_color) { const struct st_texture_object *stobj = st_texture_object_const(texobj); - const struct pipe_sampler_view *sv = NULL; - - /* Just search for the first used view. We can do this because the - swizzle is per-texture, not per context. */ /* XXX: clean that up to not use the sampler view at all */ - for (unsigned i = 0; i < stobj->num_sampler_views; ++i) { - if (stobj->sampler_views[i].view) { - sv = stobj->sampler_views[i].view; - break; - } - } + const struct st_sampler_view *sv = st_texture_get_current_sampler_view(st, stobj); if (sv) { + struct pipe_sampler_view *view = sv->view; union pipe_color_union tmp; const unsigned char swz[4] = { - sv->swizzle_r, - sv->swizzle_g, - sv->swizzle_b, - sv->swizzle_a, + view->swizzle_r, + view->swizzle_g, + view->swizzle_b, + view->swizzle_a, }; st_translate_color(&msamp->BorderColor, &tmp, texBaseFormat, is_integer); util_format_apply_color_swizzle(&sampler->border_color, &tmp, swz, is_integer); } else { st_translate_color(&msamp->BorderColor, &sampler->border_color, diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c index 3b7a3b5e985..7766273381b 100644 --- a/src/mesa/state_tracker/st_cb_texture.c +++ b/src/mesa/state_tracker/st_cb_texture.c @@ -144,40 +144,52 @@ st_DeleteTextureImage(struct gl_context * ctx, struct gl_texture_image *img) /* nothing special (yet) for st_texture_image */ _mesa_delete_texture_image(ctx, img); } /** called via ctx->Driver.NewTextureObject() */ static struct gl_texture_object * st_NewTextureObject(struct gl_context * ctx, GLuint name, GLenum target) { struct st_texture_object *obj = ST_CALLOC_STRUCT(st_texture_object); + if (!obj) + return NULL; + + /* Pre-allocate a sampler views container to save a branch in the fast path. */ + obj->sampler_views = calloc(1, sizeof(struct st_sampler_views) + sizeof(struct st_sampler_view)); + if (!obj->sampler_views) { + free(obj); + return NULL; + } + obj->sampler_views->max = 1; DBG("%s\n", __func__); _mesa_initialize_texture_object(ctx, &obj->base, name, target); + simple_mtx_init(&obj->validate_mutex, mtx_plain); obj->needs_validation = true; return &obj->base; } /** called via ctx->Driver.DeleteTextureObject() */ static void st_DeleteTextureObject(struct gl_context *ctx, struct gl_texture_object *texObj) { struct st_context *st = st_context(ctx); struct st_texture_object *stObj = st_texture_object(texObj); pipe_resource_reference(&stObj->pt, NULL); st_texture_release_all_sampler_views(st, stObj); st_texture_free_sampler_views(stObj); + simple_mtx_destroy(&stObj->validate_mutex); _mesa_delete_texture_object(ctx, texObj); } /** called via ctx->Driver.FreeTextureImageBuffer() */ static void st_FreeTextureImageBuffer(struct gl_context *ctx, struct gl_texture_image *texImage) { struct st_context *st = st_context(ctx); diff --git a/src/mesa/state_tracker/st_sampler_view.c b/src/mesa/state_tracker/st_sampler_view.c index 892725671d2..b737011d501 100644 --- a/src/mesa/state_tracker/st_sampler_view.c +++ b/src/mesa/state_tracker/st_sampler_view.c @@ -36,104 +36,216 @@ #include "st_context.h" #include "st_sampler_view.h" #include "st_texture.h" #include "st_format.h" #include "st_cb_bufferobjects.h" #include "st_cb_texture.h" /** - * Try to find a matching sampler view for the given context. - * If none is found an empty slot is initialized with a - * template and returned instead. + * Set the given view as the current context's view for the texture. + * + * Overwrites any pre-existing view of the context. + * + * Takes ownership of the view (i.e., stores the view without incrementing the + * reference count). + * + * \return the view, or NULL on error. In case of error, the reference to the + * view is released. */ -static struct st_sampler_view * -st_texture_get_sampler_view(struct st_context *st, - struct st_texture_object *stObj) +static struct pipe_sampler_view * +st_texture_set_sampler_view(struct st_context *st, + struct st_texture_object *stObj, + struct pipe_sampler_view *view, + bool glsl130_or_later, bool srgb_skip_decode) { + struct st_sampler_views *views; struct st_sampler_view *free = NULL; + struct st_sampler_view *sv; GLuint i; - for (i = 0; i < stObj->num_sampler_views; ++i) { - struct st_sampler_view *sv = &stObj->sampler_views[i]; + simple_mtx_lock(&stObj->validate_mutex); + views = stObj->sampler_views; + + for (i = 0; i < views->count; ++i) { + sv = &views->views[i]; + /* Is the array entry used ? */ if (sv->view) { /* check if the context matches */ if (sv->view->context == st->pipe) { - return sv; + pipe_sampler_view_release(st->pipe, &sv->view); + goto found; } } else { /* Found a free slot, remember that */ free = sv; } } /* Couldn't find a slot for our context, create a new one */ + if (free) { + sv = free; + } else { + if (views->count >= views->max) { + /* Allocate a larger container. */ + unsigned new_max = 2 * views->max; + unsigned new_size = sizeof(*views) + new_max * sizeof(views->views[0]); + + if (new_max < views->max || + new_max > (UINT_MAX - sizeof(*views)) / sizeof(views->views[0])) { + pipe_sampler_view_release(st->pipe, &view); + goto out; + } + + struct st_sampler_views *new_views = malloc(new_size); + if (!new_views) { + pipe_sampler_view_release(st->pipe, &view); + goto out; + } + + new_views->count = views->count; + new_views->max = new_max; + memcpy(&new_views->views[0], &views->views[0], + views->count * sizeof(views->views[0])); + + /* Initialize the pipe_sampler_view pointers to zero so that we don't + * have to worry about racing against readers when incrementing + * views->count. + */ + memset(&new_views->views[views->count], 0, + (new_max - views->count) * sizeof(views->views[0])); + + /* Use memory release semantics to ensure that concurrent readers will + * get the correct contents of the new container. + * + * Also, the write should be atomic, but that's guaranteed anyway on + * all supported platforms. + */ + p_atomic_set(&stObj->sampler_views, new_views); + + /* We keep the old container around until the texture object is + * deleted, because another thread may still be reading from it. We + * double the size of the container each time, so we end up with + * at most twice the total memory allocation. + */ + views->next = stObj->sampler_views_old; + stObj->sampler_views_old = views; + + views = new_views; + } + + sv = &views->views[views->count]; - if (!free) { - /* Haven't even found a free one, resize the array */ - unsigned new_size = (stObj->num_sampler_views + 1) * - sizeof(struct st_sampler_view); - stObj->sampler_views = realloc(stObj->sampler_views, new_size); - free = &stObj->sampler_views[stObj->num_sampler_views++]; - free->view = NULL; + /* Since modification is guarded by the lock, only the write part of the + * increment has to be atomic, and that's already guaranteed on all + * supported platforms without using an atomic intrinsic. + */ + views->count++; } - assert(free->view == NULL); +found: + assert(sv->view == NULL); + + sv->glsl130_or_later = glsl130_or_later; + sv->srgb_skip_decode = srgb_skip_decode; + sv->view = view; + +out: + simple_mtx_unlock(&stObj->validate_mutex); + return view; +} + + +/** + * Return the most-recently validated sampler view for the texture \p stObj + * in the given context, if any. + * + * Performs no additional validation. + */ +const struct st_sampler_view * +st_texture_get_current_sampler_view(const struct st_context *st, + const struct st_texture_object *stObj) +{ + const struct st_sampler_views *views = p_atomic_read(&stObj->sampler_views); + + for (unsigned i = 0; i < views->count; ++i) { + const struct st_sampler_view *sv = &views->views[i]; + if (sv->view && sv->view->context == st->pipe) + return sv; + } - return free; + return NULL; } /** * For the given texture object, release any sampler views which belong * to the calling context. */ void st_texture_release_sampler_view(struct st_context *st, struct st_texture_object *stObj) { GLuint i; - for (i = 0; i < stObj->num_sampler_views; ++i) { - struct pipe_sampler_view **sv = &stObj->sampler_views[i].view; + simple_mtx_lock(&stObj->validate_mutex); + struct st_sampler_views *views = stObj->sampler_views; + for (i = 0; i < views->count; ++i) { + struct pipe_sampler_view **sv = &views->views[i].view; if (*sv && (*sv)->context == st->pipe) { pipe_sampler_view_reference(sv, NULL); break; } } + simple_mtx_unlock(&stObj->validate_mutex); } /** * 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) { GLuint i; - for (i = 0; i < stObj->num_sampler_views; ++i) - pipe_sampler_view_release(st->pipe, &stObj->sampler_views[i].view); + /* TODO: This happens while a texture is deleted, because the Driver API + * is asymmetric: the driver allocates the texture object memory, but + * mesa/main frees it. + */ + if (!stObj->sampler_views) + return; + + simple_mtx_lock(&stObj->validate_mutex); + struct st_sampler_views *views = stObj->sampler_views; + for (i = 0; i < views->count; ++i) + pipe_sampler_view_release(st->pipe, &views->views[i].view); + simple_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; + + while (stObj->sampler_views_old) { + struct st_sampler_views *views = stObj->sampler_views_old; + stObj->sampler_views_old = views->next; + free(views); + } } /** * Return swizzle1(swizzle2) */ static unsigned swizzle_swizzle(unsigned swizzle1, unsigned swizzle2) { unsigned i, swz[4]; @@ -403,115 +515,117 @@ st_create_texture_sampler_view_from_stobj(struct st_context *st, } struct pipe_sampler_view * st_get_texture_sampler_view_from_stobj(struct st_context *st, struct st_texture_object *stObj, const struct gl_sampler_object *samp, bool glsl130_or_later, bool ignore_srgb_decode) { - struct st_sampler_view *sv; - struct pipe_sampler_view *view; + const struct st_sampler_view *sv; bool srgb_skip_decode = false; - sv = st_texture_get_sampler_view(st, stObj); - view = sv->view; - if (!ignore_srgb_decode && samp->sRGBDecode == GL_SKIP_DECODE_EXT) srgb_skip_decode = true; - if (view && + sv = st_texture_get_current_sampler_view(st, stObj); + + if (sv && sv->glsl130_or_later == glsl130_or_later && sv->srgb_skip_decode == srgb_skip_decode) { /* Debug check: make sure that the sampler view's parameters are * what they're supposed to be. */ - MAYBE_UNUSED struct pipe_sampler_view *view = sv->view; + struct pipe_sampler_view *view = sv->view; assert(stObj->pt == view->texture); assert(!check_sampler_swizzle(st, stObj, view, glsl130_or_later)); assert(get_sampler_view_format(st, stObj, srgb_skip_decode) == view->format); assert(gl_target_to_pipe(stObj->base.Target) == view->target); assert(stObj->level_override || stObj->base.MinLevel + stObj->base.BaseLevel == view->u.tex.first_level); assert(stObj->level_override || last_level(stObj) == view->u.tex.last_level); assert(stObj->layer_override || stObj->base.MinLayer == view->u.tex.first_layer); assert(stObj->layer_override || last_layer(stObj) == view->u.tex.last_layer); assert(!stObj->layer_override || (stObj->layer_override == view->u.tex.first_layer && stObj->layer_override == view->u.tex.last_layer)); + return view; } - else { - /* create new sampler view */ - enum pipe_format format = get_sampler_view_format(st, stObj, srgb_skip_decode); - - sv->glsl130_or_later = glsl130_or_later; - sv->srgb_skip_decode = srgb_skip_decode; - pipe_sampler_view_release(st->pipe, &sv->view); - view = sv->view = + /* create new sampler view */ + enum pipe_format format = get_sampler_view_format(st, stObj, srgb_skip_decode); + struct pipe_sampler_view *view = st_create_texture_sampler_view_from_stobj(st, stObj, format, glsl130_or_later); - } + + view = st_texture_set_sampler_view(st, stObj, view, glsl130_or_later, srgb_skip_decode); return view; } struct pipe_sampler_view * st_get_buffer_sampler_view_from_stobj(struct st_context *st, struct st_texture_object *stObj) { - struct st_sampler_view *sv; + const struct st_sampler_view *sv; struct st_buffer_object *stBuf = st_buffer_object(stObj->base.BufferObject); if (!stBuf || !stBuf->buffer) return NULL; - sv = st_texture_get_sampler_view(st, stObj); + sv = st_texture_get_current_sampler_view(st, stObj); struct pipe_resource *buf = stBuf->buffer; - struct pipe_sampler_view *view = sv->view; - if (view && view->texture == buf) { - /* Debug check: make sure that the sampler view's parameters are - * what they're supposed to be. - */ - assert(st_mesa_format_to_pipe_format(st, stObj->base._BufferObjectFormat) + if (sv) { + struct pipe_sampler_view *view = sv->view; + + if (view->texture == buf) { + /* Debug check: make sure that the sampler view's parameters are + * what they're supposed to be. + */ + assert(st_mesa_format_to_pipe_format(st, stObj->base._BufferObjectFormat) == view->format); - assert(view->target == PIPE_BUFFER); - unsigned base = stObj->base.BufferOffset; - MAYBE_UNUSED unsigned size = MIN2(buf->width0 - base, + assert(view->target == PIPE_BUFFER); + unsigned base = stObj->base.BufferOffset; + MAYBE_UNUSED unsigned size = MIN2(buf->width0 - base, (unsigned) stObj->base.BufferSize); - assert(view->u.buf.offset == base); - assert(view->u.buf.size == size); - } else { - unsigned base = stObj->base.BufferOffset; + assert(view->u.buf.offset == base); + assert(view->u.buf.size == size); + return view; + } + } - if (base >= buf->width0) - return NULL; + unsigned base = stObj->base.BufferOffset; - unsigned size = buf->width0 - base; - size = MIN2(size, (unsigned)stObj->base.BufferSize); - if (!size) - return NULL; + if (base >= buf->width0) + return NULL; + + unsigned size = buf->width0 - base; + size = MIN2(size, (unsigned)stObj->base.BufferSize); + if (!size) + return NULL; + + /* Create a new sampler view. There is no need to clear the entire + * structure (consider CPU overhead). + */ + struct pipe_sampler_view templ; + + templ.format = + st_mesa_format_to_pipe_format(st, stObj->base._BufferObjectFormat); + templ.target = PIPE_BUFFER; + templ.swizzle_r = PIPE_SWIZZLE_X; + templ.swizzle_g = PIPE_SWIZZLE_Y; + templ.swizzle_b = PIPE_SWIZZLE_Z; + templ.swizzle_a = PIPE_SWIZZLE_W; + templ.u.buf.offset = base; + templ.u.buf.size = size; + + struct pipe_sampler_view *view = + st->pipe->create_sampler_view(st->pipe, buf, &templ); + + view = st_texture_set_sampler_view(st, stObj, view, false, false); - /* Create a new sampler view. There is no need to clear the entire - * structure (consider CPU overhead). - */ - struct pipe_sampler_view templ; - - templ.format = - st_mesa_format_to_pipe_format(st, stObj->base._BufferObjectFormat); - templ.target = PIPE_BUFFER; - templ.swizzle_r = PIPE_SWIZZLE_X; - templ.swizzle_g = PIPE_SWIZZLE_Y; - templ.swizzle_b = PIPE_SWIZZLE_Z; - templ.swizzle_a = PIPE_SWIZZLE_W; - templ.u.buf.offset = base; - templ.u.buf.size = size; - - pipe_sampler_view_release(st->pipe, &sv->view); - view = sv->view = st->pipe->create_sampler_view(st->pipe, buf, &templ); - } return view; } diff --git a/src/mesa/state_tracker/st_sampler_view.h b/src/mesa/state_tracker/st_sampler_view.h index cf46513f1b4..47c100df647 100644 --- a/src/mesa/state_tracker/st_sampler_view.h +++ b/src/mesa/state_tracker/st_sampler_view.h @@ -61,20 +61,23 @@ extern void st_texture_release_sampler_view(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); +const struct st_sampler_view * +st_texture_get_current_sampler_view(const struct st_context *st, + const struct st_texture_object *stObj); struct pipe_sampler_view * st_get_texture_sampler_view_from_stobj(struct st_context *st, struct st_texture_object *stObj, const struct gl_sampler_object *samp, bool glsl130_or_later, bool ignore_srgb_decode); struct pipe_sampler_view * st_get_buffer_sampler_view_from_stobj(struct st_context *st, diff --git a/src/mesa/state_tracker/st_texture.h b/src/mesa/state_tracker/st_texture.h index 8b549b86085..c10a2753104 100644 --- a/src/mesa/state_tracker/st_texture.h +++ b/src/mesa/state_tracker/st_texture.h @@ -24,20 +24,21 @@ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. * **************************************************************************/ #ifndef ST_TEXTURE_H #define ST_TEXTURE_H #include "pipe/p_context.h" #include "util/u_sampler.h" +#include "util/simple_mtx.h" #include "main/mtypes.h" struct pipe_resource; struct st_texture_image_transfer { struct pipe_transfer *transfer; @@ -55,20 +56,30 @@ struct st_sampler_view { struct pipe_sampler_view *view; /** The glsl version of the shader seen during validation */ bool glsl130_or_later; /** Derived from the sampler's sRGBDecode state during validation */ bool srgb_skip_decode; }; /** + * Container for per-context sampler views of a texture. + */ +struct st_sampler_views { + struct st_sampler_views *next; + uint32_t max; + uint32_t count; + struct st_sampler_view views[0]; +}; + +/** * Subclass of gl_texure_image. */ struct st_texture_image { struct gl_texture_image base; /* If stImage->pt != NULL, image data is stored here. * Else there is no image data. */ struct pipe_resource *pt; @@ -98,27 +109,48 @@ struct st_texture_object GLuint lastLevel; unsigned int validated_first_level; unsigned int validated_last_level; /* On validation any active images held in main memory or in other * textures will be copied to this texture and the old storage freed. */ struct pipe_resource *pt; - /* Number of views in sampler_views array */ - GLuint num_sampler_views; + /* Protect modifications of the sampler_views array */ + simple_mtx_t validate_mutex; - /* Array of sampler views (one per context) attached to this texture + /* Container of sampler views (one per context) attached to this texture * object. Created lazily on first binding in context. + * + * Purely read-only accesses to the current context's own sampler view + * require no locking. Another thread may simultaneously replace the + * container object in order to grow the array, but the old container will + * be kept alive. + * + * Writing to the container (even for modifying the current context's own + * sampler view) always requires taking the validate_mutex to protect against + * concurrent container switches. + * + * NULL'ing another context's sampler view is allowed only while + * implementing an API call that modifies the texture: an application which + * calls those while simultaneously reading the texture in another context + * invokes undefined behavior. (TODO: a dubious violation of this rule is + * st_finalize_texture, which is a lazy operation that corresponds to a + * texture modification.) + */ + struct st_sampler_views *sampler_views; + + /* Old sampler views container objects that have not been freed yet because + * other threads/contexts may still be reading from them. */ - struct st_sampler_view *sampler_views; + struct st_sampler_views *sampler_views_old; /* True if this texture comes from the window system. Such a texture * cannot be reallocated and the format can only be changed with a sampler * view or a surface. */ GLboolean surface_based; /* If surface_based is true, this format should be used for all sampler * views and surfaces instead of pt->format. */ -- 2.11.0 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev