On 08/03/2019 22:52, Brian Paul wrote:
After a while of running google-chrome and viewing Bing maps, we'd see "context mismatch in svga_sampler_view_destroy()" messages and eventually crash because we leaked too many sampler views (the kernel module would have too many sampler views).When a texture object is being deleted, we call st_texture_release_all_sampler_views() to release all the sampler views. In the list, there may sampler views which were created from other contexts. Previously, we called pipe_sampler_view_release(pipe, view) which would use the given pipe context to destroy the view if the refcount hit zero. The svga error was triggered because we were calling pipe->sampler_view_destroy(pipe, view) and the pipe context didn't match the view's parent context. Instead, call pipe_sampler_reference(&view, NULL). That way, if the refcount hits zero, we'll use the view's parent context to destroy the view. That's what we want. The pipe_sampler_view_release() function was introduced years ago to avoid freeing a sampler view with a context that was already deleted. But since then we've improved sampler view and context tracking. When a context is destroyed, the state tracker walks over all texture objects and frees all sampler views which belong to that context. So, we should never end up deleting a sampler view after its context is deleted. After this, we can remove all calls to pipe_sampler_view_release() in the drivers. Finally, it appears that we need to implement a similar tear-down mechanism for shaders and programs since we may generate per-context shader variants. Testing done: google chrome, misc GL demos, games --- src/mesa/state_tracker/st_context.c | 3 +-- src/mesa/state_tracker/st_sampler_view.c | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c index 2898279..a7464fd 100644 --- a/src/mesa/state_tracker/st_context.c +++ b/src/mesa/state_tracker/st_context.c @@ -278,8 +278,7 @@ st_destroy_context_priv(struct st_context *st, bool destroy_pipe) st_destroy_bound_image_handles(st);for (i = 0; i < ARRAY_SIZE(st->state.frag_sampler_views); i++) {- pipe_sampler_view_release(st->pipe, - &st->state.frag_sampler_views[i]); + pipe_sampler_view_reference(&st->state.frag_sampler_views[i], NULL); }/* free glReadPixels cache data */diff --git a/src/mesa/state_tracker/st_sampler_view.c b/src/mesa/state_tracker/st_sampler_view.c index e4eaf39..650a2b0 100644 --- a/src/mesa/state_tracker/st_sampler_view.c +++ b/src/mesa/state_tracker/st_sampler_view.c @@ -74,7 +74,7 @@ st_texture_set_sampler_view(struct st_context *st, if (sv->view) { /* check if the context matches */ if (sv->view->context == st->pipe) { - pipe_sampler_view_release(st->pipe, &sv->view); + pipe_sampler_view_reference(&sv->view, NULL); goto found; } } else { @@ -94,13 +94,13 @@ st_texture_set_sampler_view(struct st_context *st,if (new_max < views->max ||new_max > (UINT_MAX - sizeof(*views)) / sizeof(views->views[0])) { - pipe_sampler_view_release(st->pipe, &view); + pipe_sampler_view_reference(&view, NULL); goto out; }struct st_sampler_views *new_views = malloc(new_size);if (!new_views) { - pipe_sampler_view_release(st->pipe, &view); + pipe_sampler_view_reference(&view, NULL); goto out; }@@ -225,7 +225,7 @@ st_texture_release_all_sampler_views(struct st_context *st,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); + pipe_sampler_view_reference(&views->views[i].view, NULL); simple_mtx_unlock(&stObj->validate_mutex); }
With your upcoming change that prevents the sampler view from ever being released with the wrong context (which I suppose should go before this one?), this series is
Reviewed-By: Jose Fonseca <jfons...@vmware.com> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
