Properly handle forced elision in pthread_mutex_trylock

Message ID mvmzjawx2w2.fsf@hawking.suse.de
State Committed
Headers

Commit Message

Andreas Schwab Dec. 9, 2014, 3:24 p.m. UTC
  According to https://sourceware.org/bugzilla/show_bug.cgi?id=16657#c10
this is the correct solution.

Andreas.

	[BZ #16657]
	* nptl/pthread_mutex_trylock.c (__pthread_mutex_trylock): Use
	FORCE_ELISION instead of DO_ELISION.
	* sysdeps/unix/sysv/linux/x86/force-elision.h (DO_ELISION):
	Remove.
	* sysdeps/unix/sysv/linux/s390/force-elision.h (DO_ELISION):
	Likewise.
---
 nptl/pthread_mutex_trylock.c                 | 10 +++-------
 sysdeps/unix/sysv/linux/s390/force-elision.h |  5 -----
 sysdeps/unix/sysv/linux/x86/force-elision.h  |  5 -----
 3 files changed, 3 insertions(+), 17 deletions(-)
  

Comments

Ondrej Bilka Dec. 9, 2014, 10:07 p.m. UTC | #1
On Tue, Dec 09, 2014 at 04:24:13PM +0100, Andreas Schwab wrote:
> According to https://sourceware.org/bugzilla/show_bug.cgi?id=16657#c10
> this is the correct solution.
>
Should be ok as it was also acked by Andi in bugzilla, it would be
better if discussion was on mailing list.

 
> Andreas.
> 
> 	[BZ #16657]
> 	* nptl/pthread_mutex_trylock.c (__pthread_mutex_trylock): Use
> 	FORCE_ELISION instead of DO_ELISION.
> 	* sysdeps/unix/sysv/linux/x86/force-elision.h (DO_ELISION):
> 	Remove.
> 	* sysdeps/unix/sysv/linux/s390/force-elision.h (DO_ELISION):
> 	Likewise.
> ---
>  nptl/pthread_mutex_trylock.c                 | 10 +++-------
>  sysdeps/unix/sysv/linux/s390/force-elision.h |  5 -----
>  sysdeps/unix/sysv/linux/x86/force-elision.h  |  5 -----
>  3 files changed, 3 insertions(+), 17 deletions(-)
> 
> diff --git a/nptl/pthread_mutex_trylock.c b/nptl/pthread_mutex_trylock.c
> index 0293d0f..c4622aa 100644
> --- a/nptl/pthread_mutex_trylock.c
> +++ b/nptl/pthread_mutex_trylock.c
> @@ -26,13 +26,10 @@
>  #define lll_trylock_elision(a,t) lll_trylock(a)
>  #endif
>  
> -#ifndef DO_ELISION
> -#define DO_ELISION(m) 0
> +#ifndef FORCE_ELISION
> +#define FORCE_ELISION(m, s)
>  #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)
>       pthread_mutex_t *mutex;
> @@ -77,8 +74,7 @@ __pthread_mutex_trylock (mutex)
>        return 0;
>  
>      case PTHREAD_MUTEX_TIMED_NP:
> -      if (DO_ELISION (mutex))
> -	goto elision;
> +      FORCE_ELISION (mutex, goto elision);
>        /*FALL THROUGH*/
>      case PTHREAD_MUTEX_ADAPTIVE_NP:
>      case PTHREAD_MUTEX_ERRORCHECK_NP:
> diff --git a/sysdeps/unix/sysv/linux/s390/force-elision.h b/sysdeps/unix/sysv/linux/s390/force-elision.h
> index 8fd7684..31b3662 100644
> --- a/sysdeps/unix/sysv/linux/s390/force-elision.h
> +++ b/sysdeps/unix/sysv/linux/s390/force-elision.h
> @@ -17,11 +17,6 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #ifdef ENABLE_LOCK_ELISION
> -/* 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.  */
>  #define FORCE_ELISION(m, s)						\
>    if (__pthread_force_elision						\
> diff --git a/sysdeps/unix/sysv/linux/x86/force-elision.h b/sysdeps/unix/sysv/linux/x86/force-elision.h
> index 945f886..a767cf1 100644
> --- a/sysdeps/unix/sysv/linux/x86/force-elision.h
> +++ b/sysdeps/unix/sysv/linux/x86/force-elision.h
> @@ -16,11 +16,6 @@
>     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.  */
>  #define FORCE_ELISION(m, s)						\
>    if (__pthread_force_elision						\
> -- 
> 2.2.0
>
  

Patch

diff --git a/nptl/pthread_mutex_trylock.c b/nptl/pthread_mutex_trylock.c
index 0293d0f..c4622aa 100644
--- a/nptl/pthread_mutex_trylock.c
+++ b/nptl/pthread_mutex_trylock.c
@@ -26,13 +26,10 @@ 
 #define lll_trylock_elision(a,t) lll_trylock(a)
 #endif
 
-#ifndef DO_ELISION
-#define DO_ELISION(m) 0
+#ifndef FORCE_ELISION
+#define FORCE_ELISION(m, s)
 #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)
      pthread_mutex_t *mutex;
@@ -77,8 +74,7 @@  __pthread_mutex_trylock (mutex)
       return 0;
 
     case PTHREAD_MUTEX_TIMED_NP:
-      if (DO_ELISION (mutex))
-	goto elision;
+      FORCE_ELISION (mutex, goto elision);
       /*FALL THROUGH*/
     case PTHREAD_MUTEX_ADAPTIVE_NP:
     case PTHREAD_MUTEX_ERRORCHECK_NP:
diff --git a/sysdeps/unix/sysv/linux/s390/force-elision.h b/sysdeps/unix/sysv/linux/s390/force-elision.h
index 8fd7684..31b3662 100644
--- a/sysdeps/unix/sysv/linux/s390/force-elision.h
+++ b/sysdeps/unix/sysv/linux/s390/force-elision.h
@@ -17,11 +17,6 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #ifdef ENABLE_LOCK_ELISION
-/* 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.  */
 #define FORCE_ELISION(m, s)						\
   if (__pthread_force_elision						\
diff --git a/sysdeps/unix/sysv/linux/x86/force-elision.h b/sysdeps/unix/sysv/linux/x86/force-elision.h
index 945f886..a767cf1 100644
--- a/sysdeps/unix/sysv/linux/x86/force-elision.h
+++ b/sysdeps/unix/sysv/linux/x86/force-elision.h
@@ -16,11 +16,6 @@ 
    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.  */
 #define FORCE_ELISION(m, s)						\
   if (__pthread_force_elision						\