Brian, One question also for my own education inline below:
On Friday, 8 March 2019 23:52:09 CET 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. That sounds plausible so far. What I wonder, is calling into an 'arbitrary' different context and do work there thread safe for any gallium driver? Especially since pipe_sampler_view_reference in its documentation says that the views provided need to originate from the same context and that this must be the 'current'. I have been taking a quick look into iris and radeonsi and both seem to be safe in terms of threads for dereferencing the views finally. But either the documentation of pipe_sampler_view_reference should be updated or I may miss an other piece of the puzzle to see that it is still save to do so. Can you tell me? thanks Mathias > > 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); > } > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev