Re: [PATCH 2/9] Add the low level infrastructure for pthreads lock elision with TSX

["Carlos O'Donell" <carlos at redhat dot com>]

On 05/12/2013 05:05 AM, Andreas Jaeger wrote:
> On 05/11/2013 12:41 AM, Andi Kleen wrote:
>>
>> Changes with the RTM mutexes:
>> -----------------------------
>> Lock elision in pthreads is generally compatible with existing programs.
>> There are some obscure exceptions, which are expected to be uncommon.
>> See the manual for more details.
>>
>> - A broken program that unlocks a free lock will crash.
>>    There are ways around this with some tradeoffs (more code in hot paths)
>>    This will also happen on systems without RTM with the patchkit.
>>    I'm still undecided on what approach to take here; have to wait for testing reports.

Buggy so OK.

>> - pthread_mutex_destroy of a lock mutex will not return EBUSY but 0.

Which is true so OK.

>> - mutex appears free when elided.
>>    pthread_mutex_lock(mutex);
>>    if (pthread_mutex_trylock(mutex) != 0) do_something
>>    will not do something when the lock elided.
>>    However note that if the check is an assert it works as expected because the
>>    assert failure aborts and the region is re-executed non transactionally,
>>    with the old behaviour.
>>    The same change applies to write locks for rwlocks.
>>    [This is now only done for mutexes that have elision explicitely enabled,
>>     standard mutexes abort in this situation]

Which is backwards compatible so OK.

>> - There's also a similar situation with trylock outside the mutex,
>>    "knowing" that the mutex must be held due to some other condition.
>>    In this case an assert failure cannot be recovered. This situation is
>>    usually an existing bug in the program.

This is only a problem if you explicitly enable elision which doesn't force the abort?

If the answer is yes, then that's backwards compatible and OK.

>> - Same applies to the rwlocks. Some of the return values changes
>>    (for example there is no EDEADLK for an elided lock, unless it aborts.
>>     However when elided it will also never deadlock of course)
> 
> The above changes are something I'd like to see some comments on from others - how critical are these changes?

We have had a lot of discussion about this between Torvald, Rich, Andi and myself.

Enough that I can look at this list and feel familiar with it :-)

I have reiterated my position for each issue above.

>> - Timing changes, so broken programs that make assumptions about specific timing
>>    may expose already existing latent problems.  Note that these broken programs will
>>    break in other situations too (loaded system, new faster hardware, compiler
>>    optimizations etc.)
>>
>> Currently elision is enabled by default on systems that support RTM,
>> unless explicitely disabled either in the program or by the user.

I'm asking to have elision disabled by default.

I don't think we can honestly ship 2.18 with this much change enabled by default.

I'm happy to enable it in Fedora Rawhide and feed results back to Andi.

Cheers,
Carlos.