This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/8] Add the low level infrastructure for pthreads lockelision with TSX
- From: Andi Kleen <andi at firstfloor dot org>
- To: Carlos O'Donell <carlos at redhat dot com>
- Cc: Andi Kleen <andi at firstfloor dot org>, Torvald Riegel <triegel at redhat dot com>,libc-alpha at sourceware dot org, Andi Kleen <ak at linux dot intel dot com>
- Date: Wed, 6 Mar 2013 00:20:04 +0100
- Subject: Re: [PATCH 1/8] Add the low level infrastructure for pthreads lockelision with TSX
- References: <1359671370-19270-1-git-send-email-andi@firstfloor.org><1359671370-19270-2-git-send-email-andi@firstfloor.org><513672AE.5030700@redhat.com>
On Tue, Mar 05, 2013 at 05:33:18PM -0500, Carlos O'Donell wrote:
> It is no longer true that you can enable or disable using new
> lock types? Is this simply a cut-and-paste error from your previous
> version of the text where this was true?
I'm somewhat sloppy with terminology here. Internally it's still
a lock type, but on the API level it's modified as two separate flags
(elision forced on and elision forced off)
> > See the manual for more details.
>
> The more and more that I review this the more I think that for
> the *first* implementation we are going to need to be conservative.
> This includes no semantic changes to the existing POSIX API in the
> default configuration e.g. no forced elision.
Just to be clear on terminology.
With forced elision I mean the program sets the type explicitely in C
code. Everything else is not forced.
The only difference it makes is the nested trylock semantics.
> If I understand correctly then:
>
> (a) If elision is forced on then trylock does not abort the transaction
> and simply returns that the lock us unlocked.
Forced on in the program source.
> (b) Elision forced on is not the default and must be set via the environment
> variables.
The program can set it by lock flag. There is no environment variable
that changes POSIX semantics
> (c) The default will continue to support trylock with the existing
> POSIX semantics.
Correct, unless the program requested it explicitly.
>
> > - 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.
>
> Have you provided an example of this for review? I'm having a hard
> time coming up with this example.
It's practically always a data race, so incorrect code.
For example you set a flag before a lock that takes a long time (but
does not block). Some other thread reads the flag, waits a bit
and reads the lock and assumes its locked.
The problem would have eventually occurred anyways on a larger/faster/
more out of order machine, but elision can make it potentially
appear quicker.
>
> > Currently elision is enabled by default on systems that support RTM,
> > unless explicitely disabled either in the program or by the user.
>
> Enabled by default is not considered forced on?
Forced on means having code in the program that sets the force elided lock
flag. It does not mean setting a environment variable.
> Once forced on the user has requested a change in semantic for the POSIX API?
nested trylock will behave differently when forced.
> > +#include <pthreadP.h>
> > +#include <sys/fcntl.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <init-arch.h>
> > +#include "elision-conf.h"
>
> Is there any reason you don't use <elision-conf.h> and allow the sysdeps
> selection mechanism to choose elision-conf.h as appropriate? This would
> allow later developers to perhaps override this with their own copies
> using a ports style addon.
No reason. Will fix.
> > +extern int __pthread_mutex_trylock (pthread_mutex_t *);
> > +
> > +#define SUPPORTS_ELISION 1
>
> Please use `USE_ELISION'.
The flag is obsolete anyways I think, will remove.
Thanks for the review.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.