[Mesa-dev] Using the right context in st_texture_release_all_sampler_views()
Brian Paul
brianp at vmware.com
Wed Jul 22 17:00:29 PDT 2015
On 07/22/2015 05:31 PM, Jose Fonseca wrote:
> On 22/07/15 23:32, Brian Paul wrote:
>> Hi Marek,
>>
>> This is regarding your commit "st/mesa: use pipe_sampler_view_release
>> for releasing sampler views" from last October.
>>
>> Basically, we have:
>>
>> void
>> st_texture_release_all_sampler_views(struct st_context *st,
>> struct st_texture_object *stObj)
>> {
>> GLuint i;
>>
>> /* XXX This should use sampler_views[i]->pipe, not st->pipe */
>> for (i = 0; i < stObj->num_sampler_views; ++i)
>> pipe_sampler_view_release(st->pipe, &stObj->sampler_views[i]);
>> }
>>
>> Our VMware/svga driver has an issue when
>> pipe_context::sampler_view_destroy() is called with one context and a
>> sampler view which was created with another context. I can hack around
>> it in our driver code, but it would be nice to fix this in the state
>> tracker.
>>
>> Ideally, the above code should be something like:
>>
>> for (i = 0; i < stObj->num_sampler_views; ++i)
>> pipe_sampler_view_reference(&stObj->sampler_views[i], NULL);
>>
>> The current code which uses the st->pipe context came from the bug
>> https://bugs.freedesktop.org/show_bug.cgi?id=81680
>>
>> AFAICT, you were just working around an R600 driver issue. Any chance
>> we could fix the state tracker and re-test Firefox on R600?
>
> Freeing a view from a different context with another context is wrong.
>
> But freeing a view with a context that might be current on another
> thread is also wrong, as pipe_context are not thread safe.
>
>
> If that was the previous behavior, then maybe Firefox crashed due to the
> race conditions. Looking at the stack trace, I suspect that what
> happened was that one context was freeing the st_texture, and all its
> views, while the other context was destroying itself, and all its own
> views. In short, st_texture_release_sampler_view was being called
> simultanouesly from two different threads.
>
>
> The proper fix IMO is to let the pipe context that owns the view to
> destroyed from whatever thread it is current (or whenever it is made
> current.)
OK, in our off-list discussion it wasn't clear to me that
multi-threading was your main concern until your last message. I see
what you're saying now.
So, when we destroy a texture object which may be shared by multiple
contexts, we theoretically need to move the texture's sampler views to
new, per-context lists. At some later time, when the context is used,
we'd check if the list of sampler views to be destroyed was non-empty
and free them.
Unfortunately, that involves one context/thread reaching into another
context. And that sounds messy.
The ultimate solution may be to get rid of the per-texture list of
sampler views and instead store sampler views in a context-private data
structure. I'll have to think that through.
-Brian
More information about the mesa-dev
mailing list