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]

[PATCH] Remove atomic_compare_and_exchange_bool_rel.


Removing this operation and the matching (unused) catomic_ operation
seemed to be easier than fixing powerpc's definition of it, only for it
to be removed anyway in the future.  There were just three call sites of
it.

Tested on x86_64-linux.  Lei Xu, could you test on Power?

commit d7623a113c608af3e4a0a1afaa0510c0ec5c48dc
Author: Torvald Riegel <triegel@redhat.com>
Date:   Tue Jun 14 15:12:00 2016 +0200

    Remove atomic_compare_and_exchange_bool_rel.
    
    atomic_compare_and_exchange_bool_rel and
    catomic_compare_and_exchange_bool_rel are removed and replaced with the
    new C11-like atomic_compare_exchange_weak_release.  The concurrent code
    in nscd/cache.c has not been reviewed yet, so this patch does not add
    detailed comments.
    
    	* nscd/cache.c (cache_add): Use new C11-like atomic operation instead
    	of atomic_compare_and_exchange_bool_rel.
    	* nptl/pthread_mutex_unlock.c (__pthread_mutex_unlock_full): Likewise.
    	* include/atomic.h (atomic_compare_and_exchange_bool_rel,
    	catomic_compare_and_exchange_bool_rel): Remove.
    	* sysdeps/aarch64/atomic-machine.h
    	(atomic_compare_and_exchange_bool_rel): Likewise.
    	* sysdeps/alpha/atomic-machine.h
    	(atomic_compare_and_exchange_bool_rel): Likewise.
    	* sysdeps/arm/atomic-machine.h
    	(atomic_compare_and_exchange_bool_rel): Likewise.
    	* sysdeps/mips/atomic-machine.h
    	(atomic_compare_and_exchange_bool_rel): Likewise.
    	* sysdeps/tile/atomic-machine.h
    	(atomic_compare_and_exchange_bool_rel): Likewise.

diff --git a/include/atomic.h b/include/atomic.h
index 5e8bfff..ad3db25 100644
--- a/include/atomic.h
+++ b/include/atomic.h
@@ -159,23 +159,6 @@
 #endif
 
 
-#ifndef catomic_compare_and_exchange_bool_rel
-# ifndef atomic_compare_and_exchange_bool_rel
-#  define catomic_compare_and_exchange_bool_rel(mem, newval, oldval)	      \
-  catomic_compare_and_exchange_bool_acq (mem, newval, oldval)
-# else
-#  define catomic_compare_and_exchange_bool_rel(mem, newval, oldval)	      \
-  atomic_compare_and_exchange_bool_rel (mem, newval, oldval)
-# endif
-#endif
-
-
-#ifndef atomic_compare_and_exchange_bool_rel
-# define atomic_compare_and_exchange_bool_rel(mem, newval, oldval) \
-  atomic_compare_and_exchange_bool_acq (mem, newval, oldval)
-#endif
-
-
 /* Store NEWVALUE in *MEM and return the old value.  */
 #ifndef atomic_exchange_acq
 # define atomic_exchange_acq(mem, newvalue) \
diff --git a/nptl/pthread_mutex_unlock.c b/nptl/pthread_mutex_unlock.c
index 334ce38..82aaa95 100644
--- a/nptl/pthread_mutex_unlock.c
+++ b/nptl/pthread_mutex_unlock.c
@@ -237,15 +237,24 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
       int private = (robust
 		     ? PTHREAD_ROBUST_MUTEX_PSHARED (mutex)
 		     : PTHREAD_MUTEX_PSHARED (mutex));
-      if ((mutex->__data.__lock & FUTEX_WAITERS) != 0
-	  || atomic_compare_and_exchange_bool_rel (&mutex->__data.__lock, 0,
-						   THREAD_GETMEM (THREAD_SELF,
-								  tid)))
+      /* Unlock the mutex using a CAS unless there are futex waiters or our
+	 TID is not the value of __lock anymore, in which case we let the
+	 kernel take care of the situation.  Use release MO in the CAS to
+	 synchronize with acquire MO in lock acquisitions.  */
+      int l = atomic_load_relaxed (&mutex->__data.__lock);
+      do
 	{
-	  INTERNAL_SYSCALL_DECL (__err);
-	  INTERNAL_SYSCALL (futex, __err, 2, &mutex->__data.__lock,
-			    __lll_private_flag (FUTEX_UNLOCK_PI, private));
+	  if (((l & FUTEX_WAITERS) != 0)
+	      || (l != THREAD_GETMEM (THREAD_SELF, tid)))
+	    {
+	      INTERNAL_SYSCALL_DECL (__err);
+	      INTERNAL_SYSCALL (futex, __err, 2, &mutex->__data.__lock,
+				__lll_private_flag (FUTEX_UNLOCK_PI, private));
+	      break;
+	    }
 	}
+      while (!atomic_compare_exchange_weak_release (&mutex->__data.__lock,
+						    &l, 0));
 
       THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
       break;
@@ -278,15 +287,16 @@ __pthread_mutex_unlock_full (pthread_mutex_t *mutex, int decr)
 	/* One less user.  */
 	--mutex->__data.__nusers;
 
-      /* Unlock.  */
-      int newval, oldval;
+      /* Unlock.  Use release MO in the CAS to synchronize with acquire MO in
+	 lock acquisitions.  */
+      int newval;
+      int oldval = atomic_load_relaxed (&mutex->__data.__lock);
       do
 	{
-	  oldval = mutex->__data.__lock;
 	  newval = oldval & PTHREAD_MUTEX_PRIO_CEILING_MASK;
 	}
-      while (atomic_compare_and_exchange_bool_rel (&mutex->__data.__lock,
-						   newval, oldval));
+      while (!atomic_compare_exchange_weak_release (&mutex->__data.__lock,
+						    &oldval, newval));
 
       if ((oldval & ~PTHREAD_MUTEX_PRIO_CEILING_MASK) > 1)
 	lll_futex_wake (&mutex->__data.__lock, 1,
diff --git a/nscd/cache.c b/nscd/cache.c
index 3021abd..daa0b2b 100644
--- a/nscd/cache.c
+++ b/nscd/cache.c
@@ -178,12 +178,12 @@ cache_add (int type, const void *key, size_t len, struct datahead *packet,
   assert ((newp->packet & BLOCK_ALIGN_M1) == 0);
 
   /* Put the new entry in the first position.  */
-  do
-    newp->next = table->head->array[hash];
-  while (atomic_compare_and_exchange_bool_rel (&table->head->array[hash],
-					       (ref_t) ((char *) newp
-							- table->data),
-					       (ref_t) newp->next));
+  /* TODO Review concurrency.  Use atomic_exchange_release.  */
+  newp->next = atomic_load_relaxed (&table->head->array[hash]);
+  while (!atomic_compare_exchange_weak_release (&table->head->array[hash],
+						(ref_t *) &newp->next,
+						(ref_t) ((char *) newp
+							 - table->data)));
 
   /* Update the statistics.  */
   if (packet->notfound)
diff --git a/sysdeps/aarch64/atomic-machine.h b/sysdeps/aarch64/atomic-machine.h
index 28c80dc..6708b9b 100644
--- a/sysdeps/aarch64/atomic-machine.h
+++ b/sysdeps/aarch64/atomic-machine.h
@@ -115,10 +115,6 @@ typedef uintmax_t uatomic_max_t;
 
 /* Compare and exchange with "release" semantics, ie barrier before.  */
 
-# define atomic_compare_and_exchange_bool_rel(mem, new, old)	\
-  __atomic_bool_bysize (__arch_compare_and_exchange_bool, int,	\
-			mem, new, old, __ATOMIC_RELEASE)
-
 # define atomic_compare_and_exchange_val_rel(mem, new, old)	 \
   __atomic_val_bysize (__arch_compare_and_exchange_val, int,    \
                        mem, new, old, __ATOMIC_RELEASE)
diff --git a/sysdeps/alpha/atomic-machine.h b/sysdeps/alpha/atomic-machine.h
index d96cb7a..882d800 100644
--- a/sysdeps/alpha/atomic-machine.h
+++ b/sysdeps/alpha/atomic-machine.h
@@ -210,10 +210,6 @@ typedef uintmax_t uatomic_max_t;
 
 /* Compare and exchange with "release" semantics, ie barrier before.  */
 
-#define atomic_compare_and_exchange_bool_rel(mem, new, old)	\
-  __atomic_bool_bysize (__arch_compare_and_exchange_bool, int,	\
-		        mem, new, old, __MB, "")
-
 #define atomic_compare_and_exchange_val_rel(mem, new, old)	\
   __atomic_val_bysize (__arch_compare_and_exchange_val, int,	\
 		       mem, new, old, __MB, "")
diff --git a/sysdeps/arm/atomic-machine.h b/sysdeps/arm/atomic-machine.h
index dd5e714..916c09a 100644
--- a/sysdeps/arm/atomic-machine.h
+++ b/sysdeps/arm/atomic-machine.h
@@ -87,10 +87,6 @@ void __arm_link_error (void);
 
 /* Compare and exchange with "release" semantics, ie barrier before.  */
 
-# define atomic_compare_and_exchange_bool_rel(mem, new, old)    \
-  __atomic_bool_bysize (__arch_compare_and_exchange_bool, int,  \
-                        mem, new, old, __ATOMIC_RELEASE)
-
 # define atomic_compare_and_exchange_val_rel(mem, new, old)      \
   __atomic_val_bysize (__arch_compare_and_exchange_val, int,    \
                        mem, new, old, __ATOMIC_RELEASE)
diff --git a/sysdeps/mips/atomic-machine.h b/sysdeps/mips/atomic-machine.h
index a60e4fb..771166b 100644
--- a/sysdeps/mips/atomic-machine.h
+++ b/sysdeps/mips/atomic-machine.h
@@ -149,10 +149,6 @@ typedef uintmax_t uatomic_max_t;
 
 /* Compare and exchange with "release" semantics, ie barrier before.  */
 
-# define atomic_compare_and_exchange_bool_rel(mem, new, old)	\
-  __atomic_bool_bysize (__arch_compare_and_exchange_bool, int,	\
-			mem, new, old, __ATOMIC_RELEASE)
-
 # define atomic_compare_and_exchange_val_rel(mem, new, old)	 \
   __atomic_val_bysize (__arch_compare_and_exchange_val, int,    \
                        mem, new, old, __ATOMIC_RELEASE)
@@ -330,10 +326,6 @@ typedef uintmax_t uatomic_max_t;
 
 /* Compare and exchange with "release" semantics, ie barrier before.  */
 
-# define atomic_compare_and_exchange_bool_rel(mem, new, old)	\
-  __atomic_bool_bysize (__arch_compare_and_exchange_bool, int,	\
-		        mem, new, old, MIPS_SYNC_STR, "")
-
 # define atomic_compare_and_exchange_val_rel(mem, new, old)	\
   __atomic_val_bysize (__arch_compare_and_exchange_val, int,	\
 		       mem, new, old, MIPS_SYNC_STR, "")
diff --git a/sysdeps/tile/atomic-machine.h b/sysdeps/tile/atomic-machine.h
index 651e0d9..336518c 100644
--- a/sysdeps/tile/atomic-machine.h
+++ b/sysdeps/tile/atomic-machine.h
@@ -55,11 +55,6 @@ typedef uintmax_t uatomic_max_t;
     atomic_full_barrier ();                                     \
     atomic_compare_and_exchange_val_acq ((mem), (n), (o));      \
   })
-#define atomic_compare_and_exchange_bool_rel(mem, n, o)         \
-  ({                                                            \
-    atomic_full_barrier ();                                     \
-    atomic_compare_and_exchange_bool_acq ((mem), (n), (o));     \
-  })
 #define atomic_exchange_rel(mem, n)                             \
   ({                                                            \
     atomic_full_barrier ();                                     \

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