[Mesa-dev] [PATCH 2/3] mesa: Use a recursive mutex for the texture lock.

[Kristian Høgsberg]

On Tue, Aug 19, 2014 at 7:03 AM, Dave Airlie <airlied at gmail.com> wrote:
>> This avoids problems with things like meta operations calling functions
>> that want to take the lock while the lock is already held.  Basically,
>> the point is to guard against API reentrancy across threads...not to
>> guard against ourselves.
>
> I hate this on purely the grounds that I feel someone should always object
> to recursive mutexes. They are always due to a bad layering violation at some
> level. I won't cry if this goes in, but I'll guarantee someone will
> regret this at
> some point in their future.
>
> Ideally we'd split the APIs so the GL entrypoints take the locks, but
> meta uses pre-locked entrypoints,

I don't disagree with anything you say here; I know the party line
about recursive mutexes.  But I don't think the existing strategy of
lifting the lock when it gets in the way works well either.  This is
something that we already do, even before my meta fast clear patches.
I noted this section from your link below:

   "Recursive mutexes are a hack. There's nothing wrong with using them, but
    they're a crutch. Got a broken leg or library? Fine, use the crutch. But
    at least be aware that you're using a crutch, and why; and once in a
    while check out the leg (or library) to be sure you still need the crutch."

which applies pretty well to our situation...

 - Why do _mesa_TexParameter* not need to lock the texture object to
modify state while _mesa_GetTexParameteriv locks to just read out
state?

 - What protects _mesaBindTexture between looking up a texture object
and referencing it from racing with another thread deleting the
texture?

 - How can _mesa_meta_GenerateMipmap safely do what it does when it
releases the texure lock?  It's operating on the texobj after
releasing the lock, another thread could modify the number of mipmap
levels in that window, for example.

Maybe these are just bugs or maybe I just know how it works, but I do
feel like I'm missing the big picture on texture locking. It's one of
the most subtle things in mesa and I've found no documentation and
nobody has been able to explain it to me. What exactly does the
texture lock protect (meta data? contents?) and when are you supposed
to take it.  I'd really like to know how it's supposed to work,
because I don't see a clear pattern in the code.

I'll spend a couple of days or so trying to see if there's a better
way to do this.  Your suggestion of adding unlocked entry points that
meta can use, doesn't really work, since the lock
(_mesa_lock_context_textures for _mesa_update_state) is typically
taken well into any given entry point.  We may be able to add a new dd
entry point (ResolveTexture or such) that we call in all the right
places.

> or everyone would just use gallium and drop meta :-P

Oh hai, you must be new here.

Kristian

> I'll also just leave this here.
> http://www.zaval.org/resources/library/butenhof1.html
>
> Dave.