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

[Ian Romanick]

On 06/12/2013 04:08 PM, Dave Airlie wrote:
> On Thu, Jun 13, 2013 at 3:33 AM, Eric Anholt <eric at anholt.net> wrote:
>> 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
>
> I suspect you'd need a reservation type scheme like the one Maarten is
> writing for
> the kernel, and based on the one TTM uses.
>
> Where you get a list of objects you want to lock and back off all locks when
> you hit a contended point.
>
> Dave.

Other OpenGL drivers solve this a different way, and it has been 
something on my todo list since forever.  Basically, you have N+1 sets 
of state per object: the global state and a mirror per-context.

Once a texture is bound, all reads access the local mirror.  Writes hit 
the local mirror and the global state.  When glBindTexture is called, 
the mirror synchronizes from the global state.  Accesses to the 
per-context state never need a lock (since they're implicitly locked by 
when the thread calls MakeCurrent).

This eliminates all of the ABBA problems I'm aware of.

  - From Eric's example, thread 1 never needs to lock texture A, and 
thread 2 never needs to lock texture B.

  - Other cases where multiple textures are involved together (e.g., MRT 
rendering) are only modifying texture contents, not texture state.  No 
lock is needed in these cases.

The other follow-on is that we need to separately reference count the 
storage for the image data.  There are probably other issues.  Clients 
that have the texture bound will continue to access the same images even 
if another client has called glTexImage or glCopyTexImage.  Once the 
last client unbinds (or rebinds) the texture the old images are freed.

It's a big pile of work that will touch things all over the place in 
Mesa.  That, alas, is why I've never gotten around to doing it.

I think adding the ref counting to the images and adding the per-context 
mirror state (with the single big lock) would be good first steps.