This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [committed] Use __kernel_cmpxchg for __sync_lock_release
- From: Ramana Radhakrishnan <ramana dot gcc at googlemail dot com>
- To: Mikael Pettersson <mikpelinux at gmail dot com>
- Cc: John David Anglin <dave dot anglin at bell dot net>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Ramana Radhakrishnan <ramana dot radhakrishnan at arm dot com>, Richard Earnshaw <richard dot earnshaw at arm dot com>
- Date: Sat, 19 Jul 2014 09:44:18 +0100
- Subject: Re: [committed] Use __kernel_cmpxchg for __sync_lock_release
- Authentication-results: sourceware.org; auth=none
- References: <BLU436-SMTP97BE92D7629BBF76B5D07697F40 at phx dot gbl> <21449 dot 1231 dot 812618 dot 674579 at gargle dot gargle dot HOWL>
- Reply-to: ramrad01 at arm dot com
On Fri, Jul 18, 2014 at 12:28 PM, Mikael Pettersson
<mikpelinux@gmail.com> wrote:
> John David Anglin writes:
> > Because the atomic sync functions in config/pa/linux-atomic.c are not
> > lock free, we need to use
> > __kernel_cmpxchg for the __sync_lock_release. This was found in
> > glibc's pthread_spin_unlock
> > implementation.
> >
> > Tested on hppa-unknown-linux-gnu. Committed to trunk.
>
> It seems to me that ARM's linux-atomic.c has the same problem.
> CC:ing some ARM folks for clarification.
Prima-facie I'm worried about the 8 byte store case as that's not
guaranteed atomic on all MP implementations (you need LPAE IIRC to
guarantee this).
All other cases look sane to me as the 1,2, 4 byte accesses should be atomic.
Ramana
>
> /Mikael
>
> >
> > Dave
> > --
> > John David Anglin dave.anglin@bell.net
> >
> >
> >
> >
> > ----------------------------------------------------------------------
> > 2014-07-17 John David Anglin <danglin@gcc.gnu.org>
> >
> > * config/pa/linux-atomic.c (__sync_lock_release_4): New.
> > (SYNC_LOCK_RELEASE): Update to use __kernel_cmpxchg for release.
> > Don't use SYNC_LOCK_RELEASE for int type.
> >
> > Index: config/pa/linux-atomic.c
> > ===================================================================
> > --- config/pa/linux-atomic.c (revision 210671)
> > +++ config/pa/linux-atomic.c (working copy)
> > @@ -293,13 +293,34 @@
> > SUBWORD_TEST_AND_SET (unsigned short, 2)
> > SUBWORD_TEST_AND_SET (unsigned char, 1)
> >
> > +void HIDDEN
> > +__sync_lock_release_4 (int *ptr)
> > +{
> > + int failure, oldval;
> > +
> > + do {
> > + oldval = *ptr;
> > + failure = __kernel_cmpxchg (oldval, 0, ptr);
> > + } while (failure != 0);
> > +}
> > +
> > #define SYNC_LOCK_RELEASE(TYPE, WIDTH) \
> > void HIDDEN \
> > __sync_lock_release_##WIDTH (TYPE *ptr) \
> > { \
> > - *ptr = 0; \
> > + int failure; \
> > + unsigned int oldval, newval, shift, mask; \
> > + int *wordptr = (int *) ((unsigned long) ptr & ~3); \
> > + \
> > + shift = (((unsigned long) ptr & 3) << 3) ^ INVERT_MASK_##WIDTH; \
> > + mask = MASK_##WIDTH << shift; \
> > + \
> > + do { \
> > + oldval = *wordptr; \
> > + newval = oldval & ~mask; \
> > + failure = __kernel_cmpxchg (oldval, newval, wordptr); \
> > + } while (failure != 0); \
> > }
> >
> > -SYNC_LOCK_RELEASE (int, 4)
> > SYNC_LOCK_RELEASE (short, 2)
> > SYNC_LOCK_RELEASE (char, 1)
>
> --