Hi

> [...] It would seem that ARMv7 no longer includes swp/swpne
> instructions. It's relatively trivial to conditionally
> replace this with appropriate ldrex/strex instructions

See https://wiki.ubuntu.com/ARM/Thumb2 for a bit more background on this.  ARMv7 still supports SWP, but it's inefficient and not guaranteed to work correctly on all platforms in the future, especially in the case of multi-core platforms.  In addition, you're getting build errors because lucid is building for the Thumb-2 instruction set by default now, which doesn't support SWP at all.


It looks like most of the functions in the header should be straightforward to implement using the GCC atomics:  See lucid gcc-4.4 info pages: node Atomic Builtins.  These are already supported on various architectures, not just ARM.

The best way to detect support for these is to add a test in your configure script which tries to compile and link calls to a couple of the atomic builtins.

This is the best approach, since the calling code can be kept cleaner and more portable--- also you don't need to worry too much about memory barriers because the builtins generally do the appropriate things to ensure SMP-safety.



For cri_atomic_read and cri_atomic_write, some extra care is likely to be needed for SMP-safety.  Memory ordering barriers are generally needed for these operations to have the desired effects in multithreaded environments.

The existing __asm__ __volatile__("": : :"memory") statements (which provide a barrier at the C language level only) should be replaced with a call to __sync_synchronize, if the GCC atomic intrinsics are detected as available.  This should work for Ubuntu and any distro based on GCC >= 4.4 generally.



> 1) CR_KCODE___kuser_cmpxchg clearly isn't defined at build
> time here.  The test doesn't get run because we're only
> building userspace parts at this point. Is that correct?
> Are we always expecting to be in the #else of
> libcr/arch/arm/cr_atomic.h when building userspace bits? I
> can't help but feel I've missed something in my reading of
> this somehow though.

The kuser helpers are some functions exported by the kernel and callable by userspace programs.  These are called at fixed addresses, rather than via the dynamic linker.
The helpers provied some fundamental atomic operations appropriate to the platform you're running on, in a portable way, though the kuser helpers are obscure and not well known.

The GCC atomic intrinsics are built round these, so it's better to use the intrinsics rather than calling the kuser helpers directly.


I'm not familiar with blcr--- if this code was pasted from another package, it may be that the original package knew how to define CR_KCODE___kuser_cmpxchg in its configure scripts but maybe blcr does not.  The issue is kernel version --- 2.6.16 and greater have the helpers; earlier kernels do not.  We can ignore this at least for Ubuntu, since all kernels are new enough.

> 2) Is a 2nd alternative for
> cri_atomic_inc/cri_atomic_dec_and_test/cri_cmp_swap the
> correct way to fix this? The comments in the source seem to
> suggest that using ldrex/strex would be correct and better
> where they exist. They also imply that we shouldn be using
> __kernel_cmpxchg on newer kernels anyway though.

I suggest:

1. If GCC atomic intrinsics are available, use those
2. (optionally) otherwise, the kuser helpers are present (Linux 2.6.16 or later) use those
3. Otherwise, fall back to the old swp-based implementation.

(2) is not really necessarily, but since the code is already there you could leave it.

I think this should cover most eventualities.