This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
RE: [PATCH] Fix the atomic_compare_and_exchange_*_rel definitions.
- From: Steven Munroe <munroesj at linux dot vnet dot ibm dot com>
- To: Lei Xu <lei dot xu at nxp dot com>
- Cc: Tulio Magno Quites Machado Filho <tuliom at linux dot vnet dot ibm dot com>, "libc-alpha at sourceware dot org" <libc-alpha at sourceware dot org>, Scott Wood <scott dot wood at nxp dot com>, "carlos at systemhalted dot org" <carlos at systemhalted dot org>, "sjmunroe at us dot ibm dot com" <sjmunroe at us dot ibm dot com>
- Date: Tue, 14 Jun 2016 09:35:38 -0500
- Subject: RE: [PATCH] Fix the atomic_compare_and_exchange_*_rel definitions.
- Authentication-results: sourceware.org; auth=none
- References: <1465723849-12811-1-git-send-email-lei dot xu at nxp dot com> <87h9cx894b dot fsf at totoro dot br dot ibm dot com> <1465828404 dot 12944 dot 5 dot camel at oc7878010663> <AM3PR04MB129767F24DFE1EB7782338D690540 at AM3PR04MB1297 dot eurprd04 dot prod dot outlook dot com>
- Reply-to: munroesj at linux dot vnet dot ibm dot com
On Tue, 2016-06-14 at 06:00 +0000, Lei Xu wrote:
> 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.
>
Only if the changes are complaint with the PowerISA Server profile. If
it contains any Embedded profile specific instructions it must be
isolated to a Embedded specific path and override header file. ie
sysdeps/powerpc/powerpc64/e6500/
The E6500 part claims to be PowerISAâ Version 2.06 complaint, but does
not implement the full Server profile (for example decimal and vsx are
omitted) and does include a number of Embedded specific Book II
extensions not implemented for any server processor.
This is why we have --with-cpu configure option and the supporting
directory structure under ./sysdeps/powerpc/
> [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?
> >
>
>