Re: [PATCH] Lock elision: Give PTHREAD_MUTEX_NORMAL-like mutexes a new internal type.

[Torvald Riegel <triegel at redhat dot com>]

On Tue, 2013-06-25 at 08:49 +0200, Dominik Vogt wrote:
> On Sat, Jun 22, 2013 at 01:04:58AM +0200, Torvald Riegel wrote:
> > Shortcomings:
> > * Mutexes _explicitly_ initialized to PTHREAD_MUTEX_DEFAULT via
> > pthread_mutex_settype are treated like PTHREAD_MUTEX_NORMAL.  This is
> > correct because DEFAULT provides strictly weaker guarantees, but means
> > that we can't use elision for those mutexes.  However, setting this
> > explicitly is redundant, and probably used rarely.
> 
> Is it really acceptable if mutexes that are _implicitly_
> initilized to PTHREAD_MUTEX_DEFAULT use different semantics than
> mutexes initialized _explicitly_ to PTHREAD_MUTEX_DEFAULT ->
> PTHREAD_MUTEX_NORMAL?

Yes. The semantics as far as the POSIX requirements (ie, what Roland
called the "semantic guarantees") are concerned are the same.  DEFAULT
mutexes allow undefined behavior in the "relock" case, so this includes
the deadlock that is required for NORMAL.

> > Things that could be added / changed:
> > * We could try to not subsume explicitly initialized DEFAULT mutexes
> > under the NORMAL mutexes.  But this seems to require bumping symbol
> > versions.  So might not be worth it.
> 
> .
> 
> > The main motivation for this patch, besides solving the technical
> > problem it addresses, is to carve out the parts of Andi's patches that
> > we can hopefully reach consensus on *now*.
> 
> Actually, I don't understand this pressure to get _something_ into
> 2.18 when it's clear that there will be no *well tested* elision
> patches

We can't get a lot of real-world testing done if we don't expose it to
real users.  I understand Andi says he has done a decent amount of
testing himself; I agree with him that wider-scale testing would be good
to have as the next step.

> and there's a risk to shoot your own foot.

Right.  But I think we can minimize this risk, and that's the motivation
for several of the objections at least I had regarding Andi's patches.
If you look at this patch, the outline of the patches that I posted as
reply to v12, and Carlos requests, you will see that:
* Elision is off by default.  Distributions etc. can enable it
selectively.
* We don't change the external interface nor the semantic guarantees
(e.g., I'm using an internal type for explicitly-initialized NORMAL
mutexes instead of an external type, and Carlos asked Andi for a rework
of his patches that doesn't expose the ELISION_NP and NO_ELISION_NP
flags publicly).

Thus, if someone enables elision, there is risk that performance might
be worse, but that's about it.

> > but this gives us the 90% that
> > we're interested in (ie, enabling elision for most mutexes including in
> > existing binaries),
> 
> How can we know what we're interested with zero performance
> testing up to now.

Sorry if it wasn't clear who the "we" in this sentence referred to.  I
meant the group of people that seemed to have interest in having elision
in 2.18 so far.  So, Andi, Carlos, myself, and potentially others as
well.

> Really, I'm not questioning this just for fun
> but because I've been testing transactional memory and lock
> elision on z/archirecture for several month, and because of the
> test results I'm much less optimistic that any _existing_
> application will get any relevant benefit from elision for free.

Well, this certainly depends a lot on the hardware.  I haven't done the
tests that you have done, but I trust Andi that the testing that he has
done shows that we can get a performance benefit.

But elision is off by default, and you have to explicitly enable it, and
we just have patches for x86 so far.  I think it would be good to make
this available for wider testing.  Anything that we find out for x86 is
probably also of interest to lock elision implementations on other
architectures.

> I rather expect that some applications can be (and have to be)
> carefully reworked to yield these benefits, while other
> applications will never get them.  Please keep in mind that lock
> elision is not a magical wand that fixes every lock related
> performance problem without ever understanding what the problem
> was in the first place.

There are no silver bullets, of course.  But I also don't see a reason
to not let people experiment with it, provided we don't build up any
baggage that we can't get rid off.  With the approach that I have
outlined and that this patch here is part of, in the worst case we have
to deprecate the configure-time switch to enable elision; there is no
external interface change, and even ignoring a --enable-elision=on
setting is fine because it's all just about performance.  So, in the
worst case, we can just rip all of this out in a next release.