Re: [PATCH] Fix elision in nested trylock.

[Andi Kleen <andi at firstfloor dot org>]

On Thu, Jun 27, 2013 at 01:10:03AM +0200, Torvald Riegel wrote:
> Attached is a patch that fixes a race condition in how Andi's current
> patches use elision in trylock.  The elision code path in trylock aborts
> if it is executed in a transaction, but the non-elision code path does
> no such thing.  Thus, we need to avoid that a thread acquires a lock by
> using elision and then executes a nested trylock that doesn't use
> elision (the nested trylock would violate the POSIX requirements in this
> case). 

The POSIX requirement is on the same lock, not different locks.

Elision enforce the invariant that either all parallel users on
a lock are eliding or all users are locking. This is done by putting
the lock value into the read-set, so any locker will abort any parallel
transactions.

So when a non elided lock tries to try-lock (without elision) 
it will always abort all parallel transactions first, and those
will in term reacquire their locks (or wait).  So all the standard
semantics are followed.

Between non elided trylocks there are always the standard semantics
of course too.

Given that I don't see how the problem you're describing can happen.

> To do that, we recheck our elision decision in the transaction,
> which ensures that the nested trylock will also execute the elision
> path.  Another option to solve that would be to abort any transaction in
> the non-elision code path too, but that might affect non-glibc-elision
> transactions that use locks even more.

Also "try_lock" has nothing to do with trylocks, it just points
to the adaption state field.  Adaptation state is always only a hint,
so if you look at it for a correctness problem it's always not 
correct.

I agree the naming is a bit unfortunate.

-Andi