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 03/11] Add elision to pthread_mutex_{try,timed,un,}lock


On 06/24/2013 02:24 PM, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Add elision paths to the basic mutex locks.
> 
> The normal path has a check for RTM and upgrades the lock
> to RTM when available. Trylocks cannot automatically upgrade,
> so they check for elision every time.
> 
> We use a 4 byte value in the mutex to store the lock
> elision adaptation state. This is separate from the adaptive
> spin state and uses a separate field.
> 
> Condition variables currently do not support elision.
> 
> Recursive mutexes and condition variables may be supported at some point,
> but are not in the current implementation. Also "trylock" will
> not automatically enable elision unless some other lock call
> has been already called on the lock.
> 
> This version does not use IFUNC, so it means every lock has one
> additional check for elision. Benchmarking showed the overhead
> to be negligible.

As we discussed, for the first pass we want to see anything that
changes semantics removed, that way we can get a core part of elision
into glibc and then look at what else is required.

> 2013-06-24  Andi Kleen  <ak@linux.intel.com>
>             Hongjiu Lu  <hongjiu.lu@intel.com>
> 
> 	* pthread_mutex_lock.c (adaptive_lock): Add
> 	(__pthread_mutex_lock): Add lock elision support.
> 	* pthread_mutex_timedlock.c (pthread_mutex_timedlock): dito.
> 	* pthread_mutex_trylock.c (__pthread_mutex_trylock): dito.
> 	* pthread_mutex_unlock.c (__pthread_mutex_unlock_usercnt): dito.
> 	* sysdeps/unix/sysv/linux/pthread_mutex_cond_lock.c: dito.
> 	* sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h: dito.
> 	* sysdeps/unix/sysv/linux/x86/Makefile: New file.
> 	* sysdeps/unix/sysv/linux/x86/force-elision.h: New file
> 	* sysdeps/unix/sysv/linux/x86/pthread_mutex_cond_lock.c: dito.
> 	* sysdeps/unix/sysv/linux/x86/pthread_mutex_lock.c: dito.
> 	* sysdeps/unix/sysv/linux/x86/pthread_mutex_timedlock.c: dito.
> 	* sysdeps/unix/sysv/linux/x86/pthread_mutex_trylock.c: dito.
> 	* sysdeps/unix/sysv/linux/x86/pthread_mutex_unlock.c: dito.
> 	* pthreadP.h: (PTHREAD_MUTEX_UPGRADED_ELISION_NP): Add.
> ---
>  nptl/pthreadP.h                                    |   4 +-
>  nptl/pthread_mutex_lock.c                          | 116 +++++++++++++++------
>  nptl/pthread_mutex_timedlock.c                     |  33 +++++-
>  nptl/pthread_mutex_trylock.c                       |  35 ++++++-
>  nptl/pthread_mutex_unlock.c                        |  20 +++-
>  nptl/sysdeps/pthread/pthread.h                     |   1 +
>  .../unix/sysv/linux/pthread_mutex_cond_lock.c      |   7 ++
>  .../unix/sysv/linux/x86/bits/pthreadtypes.h        |  13 ++-
>  nptl/sysdeps/unix/sysv/linux/x86/force-elision.h   |  32 ++++++
>  .../unix/sysv/linux/x86/pthread_mutex_cond_lock.c  |  21 ++++
>  .../unix/sysv/linux/x86/pthread_mutex_lock.c       |  21 ++++
>  .../unix/sysv/linux/x86/pthread_mutex_timedlock.c  |  20 ++++
>  .../unix/sysv/linux/x86/pthread_mutex_trylock.c    |  21 ++++
>  13 files changed, 303 insertions(+), 41 deletions(-)
>  create mode 100644 nptl/sysdeps/unix/sysv/linux/x86/force-elision.h
>  create mode 100644 nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_cond_lock.c
>  create mode 100644 nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_lock.c
>  create mode 100644 nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_timedlock.c
>  create mode 100644 nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_trylock.c
> 
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 72d5829..10b4be8 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -110,8 +110,10 @@ enum
>  
>  #define PTHREAD_MUTEX_TYPE(m) \
>    ((m)->__data.__kind & 127)
> +/* Don't include NO_ELISION, as that type is always the same
> +   as the underlying lock type.  */
>  #define PTHREAD_MUTEX_TYPE_ELISION(m) \
> -  ((m)->__data.__kind & (127|PTHREAD_MUTEX_ELISION_FLAGS_NP))
> +  ((m)->__data.__kind & (127|PTHREAD_MUTEX_ELISION_NP))

OK.

>  
>  #if LLL_PRIVATE == 0 && LLL_SHARED == 128
>  # define PTHREAD_MUTEX_PSHARED(m) \
> diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
> index fbedfd7..8add2b6 100644
> --- a/nptl/pthread_mutex_lock.c
> +++ b/nptl/pthread_mutex_lock.c
> @@ -25,6 +25,14 @@
>  #include <lowlevellock.h>
>  #include <stap-probe.h>
>  
> +#ifndef lll_lock_elision
> +#define lll_lock_elision(lock, try_lock, private)	({ \
> +      lll_lock (lock, private); 0; })
> +#endif
> +
> +#ifndef lll_trylock_elision
> +#define lll_trylock_elision(a,t,u) lll_trylock(a)
> +#endif

OK.

>  
>  #ifndef LLL_MUTEX_LOCK
>  # define LLL_MUTEX_LOCK(mutex) \
> @@ -34,12 +42,47 @@
>  # define LLL_ROBUST_MUTEX_LOCK(mutex, id) \
>    lll_robust_lock ((mutex)->__data.__lock, id, \
>  		   PTHREAD_ROBUST_MUTEX_PSHARED (mutex))
> +# define LLL_MUTEX_LOCK_ELISION(mutex) \
> +  lll_lock_elision ((mutex)->__data.__lock, (mutex)->__data.__elision, \
> +		   PTHREAD_MUTEX_PSHARED (mutex))
> +# define LLL_MUTEX_TRYLOCK_ELISION(mutex) \
> +  lll_trylock_elision((mutex)->__data.__lock, (mutex)->__data.__elision, \
> +		   PTHREAD_MUTEX_PSHARED (mutex))


OK.

>  #endif
>  
> +#ifndef FORCE_ELISION
> +#define FORCE_ELISION(m, s)
> +#endif
>  
>  static int __pthread_mutex_lock_full (pthread_mutex_t *mutex)
>       __attribute_noinline__;
>  
> +static inline __attribute__((always_inline)) void
> +adaptive_lock (pthread_mutex_t *mutex)
> +{
> +  if (LLL_MUTEX_TRYLOCK (mutex) != 0)
> +    {
> +      int cnt = 0;
> +      int max_cnt = MIN (MAX_ADAPTIVE_COUNT, mutex->__data.__spins * 2 + 10);
> +      do
> +        {
> +	  if (cnt++ >= max_cnt)
> +	    {
> +	      LLL_MUTEX_LOCK (mutex);
> +	      break;
> +	    }
> +
> +#ifdef BUSY_WAIT_NOP
> +	  BUSY_WAIT_NOP;
> +#endif
> +	}
> +      while (LLL_MUTEX_TRYLOCK (mutex) != 0);
> +
> +      mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
> +    }
> +  assert (mutex->__data.__owner == 0);
> +}
> +
>  
>  int
>  __pthread_mutex_lock (mutex)
> @@ -47,26 +90,37 @@ __pthread_mutex_lock (mutex)
>  {
>    assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
>  
> -  unsigned int type = PTHREAD_MUTEX_TYPE (mutex);
> +  unsigned int type = PTHREAD_MUTEX_TYPE_ELISION (mutex);

OK.

>    LIBC_PROBE (mutex_entry, 1, mutex);
>  
> -  if (__builtin_expect (type & ~PTHREAD_MUTEX_KIND_MASK_NP, 0))
> +  if (__builtin_expect (type & ~(PTHREAD_MUTEX_KIND_MASK_NP
> +				 | PTHREAD_MUTEX_ELISION_FLAGS_NP), 0))

OK.

>      return __pthread_mutex_lock_full (mutex);
>  
> -  pid_t id = THREAD_GETMEM (THREAD_SELF, tid);
> -
> -  if (__builtin_expect (type, PTHREAD_MUTEX_TIMED_NP)
> -      == PTHREAD_MUTEX_TIMED_NP)
> +  if (__builtin_expect (type == PTHREAD_MUTEX_TIMED_NP, 1))
>      {
> +      FORCE_ELISION (mutex, goto elision);
>      simple:
>        /* Normal mutex.  */
>        LLL_MUTEX_LOCK (mutex);
>        assert (mutex->__data.__owner == 0);
>      }
> +  else if (__builtin_expect (type == PTHREAD_MUTEX_TIMED_ELISION_NP, 1))
> +    {
> +  elision: __attribute__((unused))
> +      /* This case can never happen on a system without elision,
> +         as the mutex type initialization functions will not
> +	 allow to set the elision flags.  */
> +      /* Don't record owner or users for elision case. This is a
> +         tail call.  */
> +      return LLL_MUTEX_LOCK_ELISION (mutex);
> +    }
>    else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
>      {
>        /* Recursive mutex.  */
> +    recursive:;
> +      pid_t id = THREAD_GETMEM (THREAD_SELF, tid);
>  
>        /* Check whether we already hold the mutex.  */
>        if (mutex->__data.__owner == id)
> @@ -89,41 +143,41 @@ __pthread_mutex_lock (mutex)
>      }
>    else if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1))
>      {

As we've already discussed you can't elide PTHREAD_MUTEX_ADAPTIVE_NP
because linuxthreads says it behaves like NORMAL and requires that
relocking deadlock.

> -      if (! __is_smp)
> +      FORCE_ELISION (mutex, goto elision_adaptive);
> +      if (!__is_smp)
>  	goto simple;
> -
> -      if (LLL_MUTEX_TRYLOCK (mutex) != 0)
> -	{
> -	  int cnt = 0;
> -	  int max_cnt = MIN (MAX_ADAPTIVE_COUNT,
> -			     mutex->__data.__spins * 2 + 10);
> -	  do
> -	    {
> -	      if (cnt++ >= max_cnt)
> -		{
> -		  LLL_MUTEX_LOCK (mutex);
> -		  break;
> -		}
> -
> -#ifdef BUSY_WAIT_NOP
> -	      BUSY_WAIT_NOP;
> -#endif
> -	    }
> -	  while (LLL_MUTEX_TRYLOCK (mutex) != 0);
> -
> -	  mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
> -	}
> -      assert (mutex->__data.__owner == 0);
> +      adaptive_lock (mutex);
> +    }
> +  else if (type == PTHREAD_MUTEX_TIMED_ELISION_NP)
> +    goto elision;
> +  else if (type == PTHREAD_MUTEX_ADAPTIVE_ELISION_NP)

Slightly off-topic, but it was I who said it would be good to specify
elision as a bit in the type, specifically because it was a useful
way to represent a property that might belong to all locks and would
prevent the growth of the number of types available to users.

As part of the second half of this patch review, where we look at
adding new ABI/API bits to the core functioning elision support
we can talk about new lock types again like you had in your original
patches. In fact we might leave elision as a bit in the type and
simply expose the PTHREAD_MUTEX_TIMED_ELISION_NP type to the user
as the new type. Internally it would be PTHREAD_MUTEX_TIMED_NP |
PTHREAD_MUTEX_ELISION_NP and we could do easier tests for it than
having a uinque value for each new type.

This is a conversation for later.

> +    {
> +  elision_adaptive: __attribute__((unused))
> +      /* FIXME: This is a poor algorithm currently.  */

We don't normally try to leave FIXME's in the code.

Please describe why it's a poor algorithm, and a pointer
to how to make it better, or if you don't know just say
that.

> +      if (!LLL_MUTEX_TRYLOCK_ELISION (mutex))
> +        return 0;
> +      adaptive_lock (mutex);
> +      /* No owner for elision */
> +      return 0;
> +    }
> +  else if (PTHREAD_MUTEX_TYPE (mutex) == PTHREAD_MUTEX_RECURSIVE_NP)
> +    {
> +      /* In case the user set the elision flags here. 
> +         Elision not supported so far.  */
> +      goto recursive;
>      }
>    else
>      {
> -      assert (type == PTHREAD_MUTEX_ERRORCHECK_NP);
> +      pid_t id = THREAD_GETMEM (THREAD_SELF, tid);
> +      assert (PTHREAD_MUTEX_TYPE (mutex) == PTHREAD_MUTEX_ERRORCHECK_NP);

OK.

>        /* Check whether we already hold the mutex.  */
>        if (__builtin_expect (mutex->__data.__owner == id, 0))
>  	return EDEADLK;
>        goto simple;
>      }
>  
> +  pid_t id = THREAD_GETMEM (THREAD_SELF, tid);

OK. I had to think about this for a minute, but I realize that id
should only be read when it's needed and not before.

> +
>    /* Record the ownership.  */
>    mutex->__data.__owner = id;
>  #ifndef NO_INCR
> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
> index 3a36424..6c361a7 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -25,6 +25,17 @@
>  
>  #include <stap-probe.h>
>  
> +#ifndef lll_timedlock_elision
> +#define lll_timedlock_elision(a,dummy,b,c) lll_timedlock(a, b, c)
> +#endif
> +
> +#ifndef lll_trylock_elision
> +#define lll_trylock_elision(a,t,u) lll_trylock(a)
> +#endif
> +
> +#ifndef FORCE_ELISION
> +#define FORCE_ELISION(m, s)
> +#endif
>  
>  int
>  pthread_mutex_timedlock (mutex, abstime)
> @@ -40,10 +51,11 @@ pthread_mutex_timedlock (mutex, abstime)
>    /* We must not check ABSTIME here.  If the thread does not block
>       abstime must not be checked for a valid value.  */
>  
> -  switch (__builtin_expect (PTHREAD_MUTEX_TYPE (mutex),
> +  switch (__builtin_expect (PTHREAD_MUTEX_TYPE_ELISION (mutex),
>  			    PTHREAD_MUTEX_TIMED_NP))
>      {
>        /* Recursive mutex.  */
> +    case PTHREAD_MUTEX_RECURSIVE_NP|PTHREAD_MUTEX_ELISION_NP:
>      case PTHREAD_MUTEX_RECURSIVE_NP:
>        /* Check whether we already hold the mutex.  */
>        if (mutex->__data.__owner == id)
> @@ -78,13 +90,32 @@ pthread_mutex_timedlock (mutex, abstime)
>        /* FALLTHROUGH */
>  
>      case PTHREAD_MUTEX_TIMED_NP:
> +      FORCE_ELISION (mutex, goto elision);
>      simple:
>        /* Normal mutex.  */
>        result = lll_timedlock (mutex->__data.__lock, abstime,
>  			      PTHREAD_MUTEX_PSHARED (mutex));
>        break;
>  
> +    case PTHREAD_MUTEX_TIMED_ELISION_NP:
> +    elision: __attribute__((unused))
> +      /* Don't record ownership */
> +      return lll_timedlock_elision (mutex->__data.__lock,
> +				    mutex->__data.__spins,
> +				    abstime,
> +				    PTHREAD_MUTEX_PSHARED (mutex));
> +
> +
> +    case PTHREAD_MUTEX_ADAPTIVE_ELISION_NP:

Need to not elide ADAPTIVE per earlier comments.

> +    adaptive_elision: __attribute__((unused))
> +      if (!lll_trylock_elision (mutex->__data.__lock, mutex->__data.__elision, 0))
> +        return 0;
> +      goto adaptive;
> +
>      case PTHREAD_MUTEX_ADAPTIVE_NP:
> +      FORCE_ELISION (mutex, goto adaptive_elision);
> +
> +    adaptive:
>        if (! __is_smp)
>  	goto simple;
>  
> diff --git a/nptl/pthread_mutex_trylock.c b/nptl/pthread_mutex_trylock.c
> index 8f5279d..cd3d107 100644
> --- a/nptl/pthread_mutex_trylock.c
> +++ b/nptl/pthread_mutex_trylock.c
> @@ -22,6 +22,16 @@
>  #include "pthreadP.h"
>  #include <lowlevellock.h>
>  
> +#ifndef lll_trylock_elision
> +#define lll_trylock_elision(a,t,u) lll_trylock(a)
> +#endif
> +
> +#ifndef DO_ELISION
> +#define DO_ELISION(m) 0
> +#endif
> +
> +/* We don't force elision in trylock, because this can lead to inconsistent
> +   lock state if the lock was actually busy. */
>  
>  int
>  __pthread_mutex_trylock (mutex)
> @@ -30,10 +40,11 @@ __pthread_mutex_trylock (mutex)
>    int oldval;
>    pid_t id = THREAD_GETMEM (THREAD_SELF, tid);
>  
> -  switch (__builtin_expect (PTHREAD_MUTEX_TYPE (mutex),
> +  switch (__builtin_expect (PTHREAD_MUTEX_TYPE_ELISION (mutex),
>  			    PTHREAD_MUTEX_TIMED_NP))
>      {
>        /* Recursive mutex.  */
> +    case PTHREAD_MUTEX_RECURSIVE_NP|PTHREAD_MUTEX_ELISION_NP:
>      case PTHREAD_MUTEX_RECURSIVE_NP:
>        /* Check whether we already hold the mutex.  */
>        if (mutex->__data.__owner == id)
> @@ -57,10 +68,23 @@ __pthread_mutex_trylock (mutex)
>  	}
>        break;
>  
> -    case PTHREAD_MUTEX_ERRORCHECK_NP:
> +    case PTHREAD_MUTEX_TIMED_ELISION_NP:
> +    case PTHREAD_MUTEX_ADAPTIVE_ELISION_NP:
> +    elision:
> +      if (lll_trylock_elision (mutex->__data.__lock,
> +			       mutex->__data.__elision,
> +			       mutex->__data.__kind
> +			       	& PTHREAD_MUTEX_UPGRADED_ELISION_NP) != 0)
> +        break;
> +      /* Don't record the ownership. */
> +      return 0;
> +
>      case PTHREAD_MUTEX_TIMED_NP:
>      case PTHREAD_MUTEX_ADAPTIVE_NP:
> -      /* Normal mutex.  */
> +      if (DO_ELISION (mutex))
> +	goto elision;
> +      /*FALL THROUGH*/
> +    case PTHREAD_MUTEX_ERRORCHECK_NP:
>        if (lll_trylock (mutex->__data.__lock) != 0)
>  	break;
>  
> @@ -378,4 +402,9 @@ __pthread_mutex_trylock (mutex)
>  
>    return EBUSY;
>  }
> +
> +#ifndef __pthread_mutex_trylock
> +#ifndef pthread_mutex_trylock
>  strong_alias (__pthread_mutex_trylock, pthread_mutex_trylock)
> +#endif
> +#endif
> diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
> index c0249f7..9e2bf91 100644
> --- a/nptl/pthread_mutex_unlock.c
> +++ b/nptl/pthread_mutex_unlock.c
> @@ -23,6 +23,10 @@
>  #include <lowlevellock.h>
>  #include <stap-probe.h>
>  
> +#ifndef lll_unlock_elision
> +#define lll_unlock_elision(a,b) ({ lll_unlock (a,b); 0; })
> +#endif
> +
>  static int
>  internal_function
>  __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
> @@ -34,8 +38,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
>       pthread_mutex_t *mutex;
>       int decr;
>  {
> -  int type = PTHREAD_MUTEX_TYPE (mutex);
> -  if (__builtin_expect (type & ~PTHREAD_MUTEX_KIND_MASK_NP, 0))
> +  int type = PTHREAD_MUTEX_TYPE_ELISION (mutex);
> +  if (__builtin_expect (type &
> +		~(PTHREAD_MUTEX_KIND_MASK_NP|PTHREAD_MUTEX_ELISION_FLAGS_NP), 0))
>      return __pthread_mutex_unlock_full (mutex, decr);
>  
>    if (__builtin_expect (type, PTHREAD_MUTEX_TIMED_NP)
> @@ -55,6 +60,13 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
>  
>        return 0;
>      }
> +  else if (__builtin_expect (type == PTHREAD_MUTEX_TIMED_ELISION_NP, 1)
> +           || (type == PTHREAD_MUTEX_ADAPTIVE_ELISION_NP))

Need to not elide ADAPTIVE per earlier comments.

> +    {
> +      /* Don't reset the owner/users fields for elision.  */
> +      return lll_unlock_elision (mutex->__data.__lock,
> +				      PTHREAD_MUTEX_PSHARED (mutex));
> +    }
>    else if (__builtin_expect (type == PTHREAD_MUTEX_RECURSIVE_NP, 1))
>      {
>        /* Recursive mutex.  */
> @@ -66,7 +78,9 @@ __pthread_mutex_unlock_usercnt (mutex, decr)
>  	return 0;
>        goto normal;
>      }
> -  else if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1))
> +  type &= ~PTHREAD_MUTEX_ELISION_FLAGS_NP;
> +  if (__builtin_expect (type == PTHREAD_MUTEX_ADAPTIVE_NP, 1) ||
> +      __builtin_expect (type == PTHREAD_MUTEX_TIMED_NP, 1))

Likewise.

>      goto normal;
>    else
>      {
> diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h
> index 3dc552f..485a865 100644
> --- a/nptl/sysdeps/pthread/pthread.h
> +++ b/nptl/sysdeps/pthread/pthread.h
> @@ -48,6 +48,7 @@ enum
>  
>    PTHREAD_MUTEX_ELISION_NP    = 1024,
>    PTHREAD_MUTEX_NO_ELISION_NP = 2048,
> +  PTHREAD_MUTEX_UPGRADED_ELISION_NP = 4096,

Move to second patchset for ABI/API review.

>    PTHREAD_MUTEX_PSHARED_NP    = 128
>  
>  #if defined __USE_UNIX98 || defined __USE_XOPEN2K8
> diff --git a/nptl/sysdeps/unix/sysv/linux/pthread_mutex_cond_lock.c b/nptl/sysdeps/unix/sysv/linux/pthread_mutex_cond_lock.c
> index b417da5..7b6fbc1 100644
> --- a/nptl/sysdeps/unix/sysv/linux/pthread_mutex_cond_lock.c
> +++ b/nptl/sysdeps/unix/sysv/linux/pthread_mutex_cond_lock.c
> @@ -2,8 +2,15 @@
>  
>  #define LLL_MUTEX_LOCK(mutex) \
>    lll_cond_lock ((mutex)->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex))
> +
> +/* Not actually elided so far. Needed? */
> +#define LLL_MUTEX_LOCK_ELISION(mutex)  \
> +  ({ lll_cond_lock ((mutex)->__data.__lock, PTHREAD_MUTEX_PSHARED (mutex)); 0; })
> +
>  #define LLL_MUTEX_TRYLOCK(mutex) \
>    lll_cond_trylock ((mutex)->__data.__lock)
> +#define LLL_MUTEX_TRYLOCK_ELISION(mutex) LLL_MUTEX_TRYLOCK(mutex)
> +

OK.

>  #define LLL_ROBUST_MUTEX_LOCK(mutex, id) \
>    lll_robust_cond_lock ((mutex)->__data.__lock, id, \
>  			PTHREAD_ROBUST_MUTEX_PSHARED (mutex))
> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
> index ccd896c..1852e07 100644
> --- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
> +++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
> @@ -101,14 +101,23 @@ typedef union
>         binary compatibility.  */
>      int __kind;
>  #ifdef __x86_64__
> -    int __spins;
> +    short __spins;
> +    short __elision;
>      __pthread_list_t __list;
>  # define __PTHREAD_MUTEX_HAVE_PREV	1
> +# define __PTHREAD_MUTEX_HAVE_ELISION   1

OK.

>  #else
>      unsigned int __nusers;
>      __extension__ union
>      {
> -      int __spins;
> +      struct
> +      {
> +        short __espins;
> +	short __elision;
> +# define __spins d.__espins
> +# define __elision d.__elision
> +# define __PTHREAD_MUTEX_HAVE_ELISION   2

OK.

> +      } d;
>        __pthread_slist_t __list;
>      };
>  #endif
> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/force-elision.h b/nptl/sysdeps/unix/sysv/linux/x86/force-elision.h
> new file mode 100644
> index 0000000..22a634a
> --- /dev/null
> +++ b/nptl/sysdeps/unix/sysv/linux/x86/force-elision.h
> @@ -0,0 +1,32 @@
> +/* force-elision.h: Automatic enabling of elision for mutexes
> +   Copyright (C) 2013 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>. */
> +
> +/* Check for elision on this lock without upgrading.  */
> +#define DO_ELISION(m)							\
> +  (__pthread_force_elision						\
> +   && (m->__data.__kind & PTHREAD_MUTEX_NO_ELISION_NP) == 0)		\
> +
> +/* Automatically enable elision for existing user lock kinds.  */

Please comment under exactly what conditions we enable elision, and
why DO_ELISION is insufficient or not optimal.

> +#define FORCE_ELISION(m, s)						\
> +  if (__pthread_force_elision						\
> +      && (m->__data.__kind & PTHREAD_MUTEX_ELISION_FLAGS_NP) == 0)	\
> +    {									\
> +      mutex->__data.__kind |= PTHREAD_MUTEX_ELISION_NP			\
> +	    | PTHREAD_MUTEX_UPGRADED_ELISION_NP;			\
> +      s;								\
> +    }

OK.

> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_cond_lock.c b/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_cond_lock.c
> new file mode 100644
> index 0000000..962f101
> --- /dev/null
> +++ b/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_cond_lock.c
> @@ -0,0 +1,21 @@
> +/* Copyright (C) 2013 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>. */
> +
> +/* The cond lock is not actually elided yet, but we still need to handle
> +   already elided locks.  */
> +#include <elision-conf.h>
> +#include "sysdeps/unix/sysv/linux/pthread_mutex_cond_lock.c"

OK.

> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_lock.c b/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_lock.c
> new file mode 100644
> index 0000000..e639460
> --- /dev/null
> +++ b/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_lock.c
> @@ -0,0 +1,21 @@
> +/* Elided version of pthread_mutex_lock.
> +   Copyright (C) 2011, 2012, 2013 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>. */
> +#include <elision-conf.h>
> +#include "force-elision.h"
> +
> +#include "nptl/pthread_mutex_lock.c"

OK.

> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_timedlock.c b/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_timedlock.c
> new file mode 100644
> index 0000000..5115d03
> --- /dev/null
> +++ b/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_timedlock.c
> @@ -0,0 +1,20 @@
> +/* Elided version of pthread_mutex_timedlock.
> +   Copyright (C) 2011, 2012, 2013 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>. */
> +#include <elision-conf.h>
> +#include "force-elision.h"
> +#include "nptl/pthread_mutex_timedlock.c"

OK.

> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_trylock.c b/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_trylock.c
> new file mode 100644
> index 0000000..9bb4a65
> --- /dev/null
> +++ b/nptl/sysdeps/unix/sysv/linux/x86/pthread_mutex_trylock.c
> @@ -0,0 +1,21 @@
> +/* Elided version of pthread_mutex_trylock.
> +   Copyright (C) 2011, 2012, 2013 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>. */
> +#include <elision-conf.h>
> +#include "force-elision.h"
> +
> +#include "nptl/pthread_mutex_trylock.c"

OK.

Cheers,
Carlos.


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