This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/8] Add the low level infrastructure for pthreads lockelision with TSX


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]