[Mesa-dev] [PATCH] mesa: per-texture locking

[Eric Anholt]

Frank Henigman <fjhenigman at google.com> writes:

> On Tue, Jun 11, 2013 at 1:10 PM, Eric Anholt <eric at anholt.net> wrote:
>
>> Frank Henigman <fjhenigman at google.com> writes:
>>
>> > Replace the one texture lock with a lock per texture.  This allows
>> > uploading textures from one thread concurrently with drawing in another
>> > thread.  _mesa_lock_context_textures() was used to check for texture
>> > updates from other contexts and also to acquire the texture lock.
>> > It's been replaced with _mesa_check_context_textures() which only does
>> > the checking.  Code sections that were between
>> > _mesa_lock_context_textures() and _mesa_unlock_context_textures()
>> > have been updated to lock individual textures as needed.
>>
>> When someone's doing something like glCopyTexSubImage() from an FBO
>> backed by a texture to another texture, how is the locking supposed to
>> work?  How about copies from one texture to the same texture?
>>
>
> Right now glCopyTexSubImage locks the destination texture before copying
> to it, but doesn't lock the source texture.  This was safe because locking
> any
> texture effectively locked them all.  With my change that's no longer true
> so
> now we're copying from an unlocked texture.  Is that your concern?
> So we just need to have it lock the source texture too?
> We'll have to check for source == destination so we don't try to lock twice.

That's an example of my concern.

> I'm pretty sure that our current locking doesn't cover nearly as much as
>> it needs to if one wants to make thread-per-context shareCtx support
>> actually work, so I'm really concerned that this change may make the
>> locking unfixable.
>>
>
> I agree there probably are problems with locking currently, and there seems
> to be zero coverage for context sharing in piglit.  But I don't understand
> how
> my change makes anything unfixable.  Can you elaborate?

Basic ABBA locking problems.  Someone does a copyteximage from texture A
to fbo-wrapped texture B, at the same time someone does copytexsubimage
From texture B to fbo-wrapped texture A.

The timeline for ABBA failure is:

thread 1:                  thread 2:
lock texture A
                           lock texture B
block locking texture B
                           block locking texture A

(You may object that "who would copy from A to B at the same time as
copying from B to A?", Well, it's legal, and you can probably find some
other lock-two-textures operation to replace one of these copyteximages
with.  Generally when I say "that would be insane, I can't imagine
anyone doing that", I just have too weak of an imagination).

> The idea behind the test was to mimic an actual use case and show
> that my patch increases concurrency.  I had thought to actually measure
> concurrency in the test (maybe by timing how long the threads wait)
> but realized this would fail on single core machines so I left that out,
> though I can still tell the patch helps because the test completes
> significantly
> faster with it.

The goal with piglit is to find bugs in drivers and demonstrate them as
simply as possible, as opposed to demoing features or testing
performance.

> use of shared contexts will probably include some synchronization between
> the users.  If you think there's value in a test of concurrent access to a
> texture I'll write one (including your glCopyTexSubImage scenarios).
> I'm interested in any suggestions as to the most useful tests to have at
> this point.   Thanks.

Things I'm afraid of with shareCtx are basically everything with
not-externally-mutexed concurrent access:

- MSAA window system buffer resolves.
- Concurrent operations on something after a depth or a color clear
  to it.
- Reference counting races around intel_miptree_reference/release()
- Reference counting races around intel_region_reference/release()
  (check out EGLImage hooks for totally unprotected cases!)
- Concurrent intel_finalize_mipmap_tree().
- Concurrent brw_workaround_depthstencil_alignment()
- While there's no zero-copy PBO support any more, we need to
  reintroduce it, so concurrent texture operations that might reference
  or COW from a PBO.

glCopyTexSubImage was just an example of "reads from texture and writes
to texture, thanks to FBOs" -- you've also got the normal drawing path,
glCopyPixels, glDrawPixels, glBitmap, glBlitFramebuffer.  If zero-copy
PBOs are reintroduced, then glReadPixels() and glGetTexImage() are
concerns.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130612/7cb8b1b4/attachment.pgp>