arm, hppa, m68k, sh, nios maintainers: Please check your emulation of atomic operations

arm, hppa, m68k, sh, nios maintainers: Please check your emulation of atomic operations

[Torvald Riegel]

These architectures seem to all use various sorts of emulation for
compare-and-exchange operations (CAS).  If these emulations do not stop
the world during a CAS, or do something else that prevents concurrent
write operations to the same memory location as the CAS, then atomic
store operations in glibc need to use a CAS to perform stores.
Otherwise, concurrent stores can happen between the load(s) and the
store that the CAS consists of.

If your architecture is affected by this, then it may be possible that
glibc's current concurrent code does not work correctly; this is because
we may still have plain stores in places where atomic stores should be
used.  Once we have completed the conversion to C11 atomics, all
concurrent stores will go through atomic_store_*().
See Chris Metcalf's patch for tilepro:
https://sourceware.org/ml/libc-alpha/2016-11/msg00414.html

If your architecture is not affected, I suggest adding a comment why
that is not the case.

Re: arm, hppa, m68k, sh, nios maintainers: Please check your emulation of atomic operations

[Andreas Schwab]

m68k is UP-only, so it requires only interrupt-safety which a simple
store already provides.  The CAS emulation is only used on coldfire, not
classic m68k.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: arm, hppa, m68k, sh, nios maintainers: Please check your emulation of atomic operations

[Ramana Radhakrishnan]

On Fri, Nov 11, 2016 at 5:13 AM, Torvald Riegel <triegel@redhat.com> wrote:
> These architectures seem to all use various sorts of emulation for
> compare-and-exchange operations (CAS).  If these emulations do not stop
> the world during a CAS, or do something else that prevents concurrent
> write operations to the same memory location as the CAS, then atomic
> store operations in glibc need to use a CAS to perform stores.
> Otherwise, concurrent stores can happen between the load(s) and the
> store that the CAS consists of.
>
> If your architecture is affected by this, then it may be possible that
> glibc's current concurrent code does not work correctly; this is because
> we may still have plain stores in places where atomic stores should be
> used.  Once we have completed the conversion to C11 atomics, all
> concurrent stores will go through atomic_store_*().
> See Chris Metcalf's patch for tilepro:
> https://sourceware.org/ml/libc-alpha/2016-11/msg00414.html


atomic-machine.h for arm was rewritten to use the __atomic builtins a
few years back. Thus if userland compiled these at architecture levels
less than the architecture levels where these could be inlined, the
emulation would be through calls to kernel helper pages. It is my
understanding the kernel helper routines if running on MP systems that
had the ldrex / strex instructions the kernel would use those to
implement CAS. Furthermore I don't know of any SMP systems that didn't
have the LDREX / STREX instructions thus I don't think that ARM is
affected by this. CC'ing Will and Catalin for a more definitive
answer.

Thanks,
Ramana
>
> If your architecture is not affected, I suggest adding a comment why
> that is not the case.
>

Re: arm, hppa, m68k, sh, nios maintainers: Please check your emulation of atomic operations

[Torvald Riegel]

On Fri, 2016-11-11 at 10:11 +0100, Andreas Schwab wrote:
> m68k is UP-only, so it requires only interrupt-safety which a simple
> store already provides.

The point is not that a simple store could be interrupted.  In a UP
scenario, the risk is that the CAS emulation gets interrupted between
the load and the store that will be part of it.  If you avoid that,
you're good.  If not, you need to either ensure that or route
atomic_store_ through CAS.

Re: arm, hppa, m68k, sh, nios maintainers: Please check your emulation of atomic operations

[Carlos O'Donell]

On 11/11/2016 12:13 AM, Torvald Riegel wrote:
> These architectures seem to all use various sorts of emulation for
> compare-and-exchange operations (CAS).  If these emulations do not stop
> the world during a CAS, or do something else that prevents concurrent
> write operations to the same memory location as the CAS, then atomic
> store operations in glibc need to use a CAS to perform stores.
> Otherwise, concurrent stores can happen between the load(s) and the
> store that the CAS consists of.
> 
> If your architecture is affected by this, then it may be possible that
> glibc's current concurrent code does not work correctly; this is because
> we may still have plain stores in places where atomic stores should be
> used.  Once we have completed the conversion to C11 atomics, all
> concurrent stores will go through atomic_store_*().
> See Chris Metcalf's patch for tilepro:
> https://sourceware.org/ml/libc-alpha/2016-11/msg00414.html
> 
> If your architecture is not affected, I suggest adding a comment why
> that is not the case.
 
This is an issue on hppa.

We need all concurrent stores to go through atomic_store_*().

The kernel light-weight syscall which emulates CAS can be interrupted by
another thread's store and thus break the CAS. The only thing we prevent
is a signal from the same thread interrupting the CAS, we do this by
not delivering the signal while the thread is in the CAS.

I'll fixup some of the atomic-machine.h bits to make sure we're doing
the most that we can to fix this.

-- 
Cheers,
Carlos.

Re: arm, hppa, m68k, sh, nios maintainers: Please check your emulation of atomic operations

[Will Deacon]

On Fri, Nov 11, 2016 at 11:33:38AM +0000, Ramana Radhakrishnan wrote:
> On Fri, Nov 11, 2016 at 5:13 AM, Torvald Riegel <triegel@redhat.com> wrote:
> > These architectures seem to all use various sorts of emulation for
> > compare-and-exchange operations (CAS).  If these emulations do not stop
> > the world during a CAS, or do something else that prevents concurrent
> > write operations to the same memory location as the CAS, then atomic
> > store operations in glibc need to use a CAS to perform stores.
> > Otherwise, concurrent stores can happen between the load(s) and the
> > store that the CAS consists of.
> >
> > If your architecture is affected by this, then it may be possible that
> > glibc's current concurrent code does not work correctly; this is because
> > we may still have plain stores in places where atomic stores should be
> > used.  Once we have completed the conversion to C11 atomics, all
> > concurrent stores will go through atomic_store_*().
> > See Chris Metcalf's patch for tilepro:
> > https://sourceware.org/ml/libc-alpha/2016-11/msg00414.html
> 
> 
> atomic-machine.h for arm was rewritten to use the __atomic builtins a
> few years back. Thus if userland compiled these at architecture levels
> less than the architecture levels where these could be inlined, the
> emulation would be through calls to kernel helper pages. It is my
> understanding the kernel helper routines if running on MP systems that
> had the ldrex / strex instructions the kernel would use those to
> implement CAS. Furthermore I don't know of any SMP systems that didn't
> have the LDREX / STREX instructions thus I don't think that ARM is
> affected by this. CC'ing Will and Catalin for a more definitive
> answer.

That's correct. The only other case is where you get pre-empted in the
critical section on a UP platform without LDREX/STREX, but the kernel
handles that.

Will

Re: arm, hppa, m68k, sh, nios maintainers: Please check your emulation of atomic operations

[Torvald Riegel]

On Fri, 2016-11-11 at 21:47 +0000, Will Deacon wrote:
> On Fri, Nov 11, 2016 at 11:33:38AM +0000, Ramana Radhakrishnan wrote:
> > On Fri, Nov 11, 2016 at 5:13 AM, Torvald Riegel <triegel@redhat.com> wrote:
> > > These architectures seem to all use various sorts of emulation for
> > > compare-and-exchange operations (CAS).  If these emulations do not stop
> > > the world during a CAS, or do something else that prevents concurrent
> > > write operations to the same memory location as the CAS, then atomic
> > > store operations in glibc need to use a CAS to perform stores.
> > > Otherwise, concurrent stores can happen between the load(s) and the
> > > store that the CAS consists of.
> > >
> > > If your architecture is affected by this, then it may be possible that
> > > glibc's current concurrent code does not work correctly; this is because
> > > we may still have plain stores in places where atomic stores should be
> > > used.  Once we have completed the conversion to C11 atomics, all
> > > concurrent stores will go through atomic_store_*().
> > > See Chris Metcalf's patch for tilepro:
> > > https://sourceware.org/ml/libc-alpha/2016-11/msg00414.html
> > 
> > 
> > atomic-machine.h for arm was rewritten to use the __atomic builtins a
> > few years back. Thus if userland compiled these at architecture levels
> > less than the architecture levels where these could be inlined, the
> > emulation would be through calls to kernel helper pages. It is my
> > understanding the kernel helper routines if running on MP systems that
> > had the ldrex / strex instructions the kernel would use those to
> > implement CAS. Furthermore I don't know of any SMP systems that didn't
> > have the LDREX / STREX instructions thus I don't think that ARM is
> > affected by this. CC'ing Will and Catalin for a more definitive
> > answer.
> 
> That's correct. The only other case is where you get pre-empted in the
> critical section on a UP platform without LDREX/STREX, but the kernel
> handles that.

It might be good to add this fact as a comment to atomic-machine.h, so
that future developers will know about this.