This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [RFC] [PATCH] powerpc: Fix missing barriers in atomic_exchange_and_add_{acq,rel}
- From: Torvald Riegel <triegel at redhat dot com>
- To: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Wed, 26 Nov 2014 10:00:05 +0100
- Subject: Re: [RFC] [PATCH] powerpc: Fix missing barriers in atomic_exchange_and_add_{acq,rel}
- Authentication-results: sourceware.org; auth=none
- References: <1413921274 dot 8483 dot 65 dot camel at triegel dot csb> <54749B3D dot 5010806 at linux dot vnet dot ibm dot com> <1416929990 dot 1771 dot 211 dot camel at triegel dot csb> <5474C5D5 dot 5080407 at linux dot vnet dot ibm dot com> <5474DE4F dot 8050401 at linux dot vnet dot ibm dot com>
On Tue, 2014-11-25 at 17:53 -0200, Adhemerval Zanella wrote:
> On 25-11-2014 16:09, Adhemerval Zanella wrote:
> > On 25-11-2014 13:39, Torvald Riegel wrote:
> >> On Tue, 2014-11-25 at 13:07 -0200, Adhemerval Zanella wrote:
> >>> Hi Torvald,
> >>>
> >>> On 21-10-2014 17:54, Torvald Riegel wrote:
> >>>> \
> >>>> })
> >>>> +#define atomic_exchange_and_add_acq(mem, value) \
> >>>> + ({ \
> >>>> + __typeof (*(mem)) __result2; \
> >>>> + __result2 = atomic_exchange_and_add (mem, value); \
> >>>> + atomic_read_barrier (); \
> >>>> + __result2;
> >>> Although it is not wrong by using a 'atomic_read_barrier' (lwsync), it adds a more
> >>> expensive synchronization than required (isync). I would prefer if we use the
> >>> already defined __arch_compare_and_exchange_val_[32|64]_[acq|rel] operations on powerpc.
> >> That's fine with me. Do you want to go adapt and commit the patch
> >> (given that you can test this easily I guess), or should I?
> >>
> > I will do it, thanks.
> >
> This is what I intend to commit. Comments?
>
> --
> * csu/tst-atomic.c (do_test): Add atomic_exchange_and_add_{acq,rel}
> tests.
Thanks for adding the tests.
> * sysdeps/powerpc/bits/atomic.h
> (__arch_atomic_exchange_and_add_32_acq): Add definition.
> (__arch_atomic_exchange_and_add_32_rel): Likewise.
> (atomic_exchange_and_add_acq): Likewise.
> (atomic_exchange_and_add_rel): Likewise.
> * sysdeps/powerpc/powerpc32/bits/atomic.h
> (__arch_atomic_exchange_and_add_64_acq): Add definition.
> (__arch_atomic_exchange_and_add_64_rel): Likewise.
> * sysdeps/powerpc/powerpc64/bits/atomic.h
> (__arch_atomic_exchange_and_add_64_acq): Add definition.
> (__arch_atomic_exchange_and_add_64_rel): Likewise.
Looks good to me.