[PATCH] Fix the atomic_compare_and_exchange_*_rel definitions.

[Lei Xu]

Hello, Steven, Tulio

Thank you and please see my comments below~


-----Original Message-----
From: Steven Munroe [mailto:munroesj@linux.vnet.ibm.com] 
Sent: Monday, June 13, 2016 10:33 PM
To: Tulio Magno Quites Machado Filho; Lei Xu
Cc: libc-alpha@sourceware.org; Scott Wood; carlos@systemhalted.org; sjmunroe@us.ibm.com
Subject: Re: [PATCH] Fix the atomic_compare_and_exchange_*_rel definitions.

On Mon, 2016-06-13 at 11:19 -0300, Tulio Magno Quites Machado Filho
wrote:
> Hi Lei,
> 
> Lei Xu <lei.xu@nxp.com> writes:
> 
> > In current code, atomic_compare_and_exchange_*_rel is defined as 
> > atomic_compare_and_exchange_*_acq, however this is wrong on power 
> > arch, especially __arch_*_rel has been defined on power arch.
> > This has caused segmentation fault issue when doing 
> > pthread_mutex_lock/unlock operations on PowerPC E6500.
> 
> I didn't expect the generic code to affect a PowerPC E6500 build.
> It should be using both of the files:
> sysdeps/powerpc/atomic-machine.h
> sysdeps/powerpc/powerpc32/atomic-machine.h
> 
E6500 is a 64-bit part which unfortunately follows the Embedded profile and so can not blindly use the power* server profile definitions and overrides.


I suspect he needs a sysdeps/powerpc/powerpc64/e6500/ with overrides and Implies to pick up common powerpc definitions
[XuLei] 
[XuLei] If we would not like to modify the generic files, another solution is to add the definitions directly in "sysdeps/powerpc/bits/atomic.h" file:
I am very happy to have all your comments, thanks.

[PATCH] Define "atomic_compare_and_exchange_bool_rel" for powerpc

Define the "atomic_compare_and_exchange_bool_rel" for powerpc,
otherwise it will use "atomic_compare_and_exchange_bool_acq" by default.

---
 sysdeps/powerpc/bits/atomic.h |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sysdeps/powerpc/bits/atomic.h b/sysdeps/powerpc/bits/atomic.h
index d71f64e..d2b91ba 100644
--- a/sysdeps/powerpc/bits/atomic.h
+++ b/sysdeps/powerpc/bits/atomic.h
@@ -80,6 +80,12 @@ typedef uintmax_t uatomic_max_t;
 #define atomic_full_barrier()	__asm ("sync" ::: "memory")
 #define atomic_write_barrier()	__asm ("eieio" ::: "memory")
 
+#ifdef __arch_compare_and_exchange_bool_32_rel
+#define atomic_compare_and_exchange_bool_rel(mem, newval, oldval)	      \
+  __atomic_bool_bysize (__arch_compare_and_exchange_bool,rel,		      \
+		        mem, newval, oldval)
+#endif
+
 #define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval)	      \
   ({									      \
       __typeof (*(mem)) __tmp;						      \

--------------------------------------[Xulei]

> These files define atomic_compare_and_exchange_val_rel(), which should 
> prevent from defining the atomic_compare_and_exchange_val_rel() as
> atomic_compare_and_exchange_val_acq() in the generic file.


> 
> So, there is a chance your build didn't use these files...
> How are you configuring your glibc build?
>