On 23/07/15 01:08, Jose Fonseca wrote:
On 23/07/15 01:00, Brian Paul wrote:
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.
Yes. We could have a sepeare mutex for this list.
Still, it's hard to prevent dead-locks (e.g, two contexts trying to add
a deferred view to each-others defferred list.)
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.
Yes, that's another possibility. I haven't thought it through either.
One way or another, you'll still want to notify a context to "garbagge
collect" its views, even if it's just an atomic flag on each context,
otherwise a texture that was shared by two contexts might end up
lingering around indefinitely because contexts are holding to its views.
What about this:
- we keep the surface views in a LRU-ordered linked list in each context
(whenever a view is unbounded, it's moved to the end of the list the
time-stamp/frame-sequence number noted)
- and we dereference the old views whenever they haven't been refered
for a while -- either in time (e.g, 1 second) or frames (e.g, 1 or 2
frames.)
This has several benefits:
- the context that destroys the texture object doesn't need to worry
about views from other contexts -- those context will automatically
destroy them on their time
- there's no need for any context to ever need to refer other context's
data.
- drivers for which views are heavy-weight (e.g, contain auxiliary
resources with their own storage) will reclaim that storage earlier if
those views stop being needed
The aspect I'm less clear is how to effeciently find the views -- in
addition of the LRU list, we'll probably need an additional data
structure (an hash, or a table of lists indexed by texture object) to
quickly find the views.
Jose
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
