[Bug 503185] Re: fails to build on armel/lucid

[Dave Martin]

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.

-- 
fails to build on armel/lucid
https://bugs.launchpad.net/bugs/503185
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.

-- 
ubuntu-bugs mailing list
ubuntu-bugs at lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs