Re: TSX lock elision for glibc v10

[Torvald Riegel <triegel at redhat dot com>]

On Tue, 2013-06-11 at 09:50 -0700, Andi Kleen wrote:
> Some bug fixes compared to the last version. Should be ready 
> for merging.

I disagree that this is ready for merging.


1) It breaks the semantics of PTHREAD_MUTEX_NORMAL because it doesn't
guarantee to block when trying to lock an already acquired mutex.  Rich
Felker has brought up convincing arguments why this can be use in
correct programs.  See http://sourceware.org/glibc/wiki/LockElisionGuide
and the comment in patch 3.


2) Explicitly enabling elision for a particular mutex (e.g., using the
initializers or by setting the PTHREAD_MUTEX_ELISION_NP bit) always
changes the semantics of the particular mutex.  Thus, there is no way
for users to just use the elision flags in an attempt to optimize
performance, without reviewing the code in detail.  This is *bad*.

The elision flags should be a *performance hint* that can be used based
just on some observation of performance counters, for example.  This
isn't possible right now -- programmers would always have to review that
the program is still correct when the semantics change due to suggesting
to use elision.  This covers the PTHREAD_MUTEX_NORMAL case above,
trylock() on mutexes, and the rwlock part.

If you want to allow programmers to use a type that is more friendly to
elision, create a new mutex type that fulfills that.  You could use the
C++11 mutex semantics, and this would automatically give you widely used
elision on C++11 too.
This is much cleaner than having a flag that modifies the meaning of
most existing types, and much easier to explain to users.  One could
think that PSHARED is a precedent for having a modifying flag, but the
difference with PSHARED is that what it expresses is *orthogonal* to the
what the mutex types express.  That's not the case for
PTHREAD_MUTEX_ELISION_NP as in the current patches because those change
the meaning of some mutex kinds, but don't modify
PTHREAD_MUTEX_DEFAULT's semantics, for example.

I'd also like to stress again that we should try to limit how much the
interfaces that we have to maintain forever are affected by what's
essentially a limitation of current hardware: If we could nest HLE
regions insides of RTM regions without getting an abort, then we
wouldn't be talking about changing mutex semantics *at all*.  I think we
should keep this in mind.

3) I dislike how the "upgrade" flag works because it seems backwards.
If elision should be used by default, you set this flag and the normal
elision flag (which does change the semantics) in several places such as
when a lock is acquired.  This leads to more complexity than required.

If you would have a new type for the mutexes for which programmers want
different semantics, then you could just use elision for the existing
mutex types in a way compatible with the current semantics.  That would
make the code cleaner, and it would probably lead to fewer code too on
the fast paths.


4) I still dislike that the elision flags (assuming they are performance
hints, as they should be) are exposed via the type.  But it's hard to do
it in a different way if we want to use initializers (ie, we'll have
them become part of the ABI anyway when they're part of initializers).


The freeze is soon, and I'd like to see some support for lock elision to
be included.  Given that we don't have consensus about the full set that
you proposed, I suggest we approach this in stages, to get at least
everything in that we actually agree on.

1) Elision without breaking semantics, without introducing new mutex
types with new semantics, and without tuning vars.  Elision is off by
default, but can be enabled at configure time.
- I believe that there is consensus to add this because this should
avoid all the contentious issues.

2) Like 1), but allow env vars for the elision tuning parameters.

3) Like 2), but also allow an env var to enable elision (in the sense of
turning it on by default for all mutexes, if their types' semantics
allows it).  Change the configure option of 1) to have a third state
that allows this (ie, besides the previous always on / always off
options).

4) Add the ELISION/NO_ELISION *performance hints* and the respective
initializers.

5) Add (a) new non-standard mutex type(s) that is/are more friendly to
elision.  I suggest to follow the C++11 / C11 mutex semantics, which
don't have all / most of the requirements that are problematic for how
you are implementing elision.  This gives us as much elision as possible
for the group of C++11/C11 users, and allows users to pick a specific
mutex type where necessary.

6) Add a tuning env var to ignore some of the mutex requirements that
are problematic for HTM-based lock elision.


I think we should be able to do 1) in any case.  Personally, I'd be fine
with 2) and even 3) -- but the env var discussion is still going on, so
we don't have consensus on this.  Note that up to and including 4), we
don't change any semantics nor add new types with different semantics.
5) and 6) are just suggestions, and it's something we'll have to discuss
further I think.


Torvald