armel build failure (sync primitives)

[Matthias Klose]

Binary package hint: ceph

http://launchpadlibrarian.net/53331585/buildlog_ubuntu-maverick-armel.ceph_0.21-0ubuntu1_FAILEDTOBUILD.txt.gz

g++ -DHAVE_CONFIG_H -I. -Wall -D__CEPH__ -D_FILE_OFFSET_BITS=64 -D_REENTRANT -D_THREAD_SAFE -rdynamic -g -O2 -MT AuthAuthorizeHandler.o -MD -MP -MF .deps/AuthAuthorizeHandler.Tpo -c -o AuthAuthorizeHandler.o `test -f 'auth/AuthAuthorizeHandler.cc' || echo './'`auth/AuthAuthorizeHandler.cc
In file included from ./include/buffer.h:55,
                 from ./include/encoding.h:20,
                 from ./include/object.h:29,
                 from ./include/types.h:64,
                 from auth/Crypto.h:17,
                 from auth/Auth.h:18,
                 from auth/AuthAuthorizeHandler.cc:1:
./include/atomic.h: In member function 'size_t ceph::atomic_t::dec()':
./include/atomic.h:36: error: 'AO_fetch_and_sub1_write' was not declared in this scope
./include/atomic.h: In member function 'void ceph::atomic_t::sub(int)':
./include/atomic.h:43: error: 'AO_fetch_and_add_write' was not declared in this scope
make[3]: *** [AuthAuthorizeHandler.o] Error 1

[Sage Weil (sage-newdream)]

I'm the upstream Ceph maintainer, but I'm not sure where this error is coming from. AO_fetch_add_write should be defined by atomic_ops.h, which is included a few lines above. Is there an armel qemu available somewhere I can use to test the build?

[Matthias Klose (doko)]

please try to use the sync primitives from GCC when available, see the section "Built-in functions for atomic memory access" in the GCC manual.

for qemu, see
https://wiki.ubuntu.com/ARM/RootfsFromScratch/QemuDebootstrap

subscribing Loic for further information.

[Gregory Farnum (greg-gregs42)]

I've looked into switching to the gcc built-ins but from what I'm seeing there's no advantage to doing so. The instructions don't seem to allow such fine-grained memory ordering (and are usually way too expensive); they use function emulation for everything on ARM (despite some built-ins existing); and t's actually less portable since it requires sufficiently new versions of gcc and libgcc. And poking around the internet I see reports from at least one person that using the gcc built-ins produces a larger binary than using libatomic-ops when they compile it on ARM (presumably due to function emulation).

Unfortunately, we're having some difficulty configuring an ARM setup here, so I can't test this out myself. Still, I think I've discovered the problem and solution. Assuming you're comfortable hacking the header file, the following diff should make things work:

--- /home/gregf/atomic_ops.h 2010-09-14 11:57:21.000000000 -0700
+++ /usr/include/atomic_ops.h 2010-02-04 15:39:58.000000000 -0800
@@ -234,7 +234,6 @@
 # if defined(__arm__) && !defined(AO_USE_PTHREAD_DEFS)
 # include "atomic_ops/sysdeps/gcc/arm.h"
 # define AO_CAN_EMUL_CAS
-# define AO_GENERALIZE_TWICE
 # endif /* __arm__ */
 # if defined(__cris__) || defined(CRIS)
 # include "atomic_ops/sysdeps/gcc/cris.h"

I'm going to make contact with the upstream maintainers to discuss this issue as well.

[Gregory Farnum (greg-gregs42)]

Oh shoot, I generated that diff backwards. You want to insert that line; it doesn't exist and I believe it should.

[Matthias Klose (doko)]

thanks for the feedback. Current Ubuntu is built for armv7-a (thumb2 mode), while distributions still build for armv4 (arm mode). The implementations should work with both modes. more information:

  https://wiki.ubuntu.com/ARM/Thumb2
  https://wiki.ubuntu.com/ARM/Thumb2PortingHowto

[Ivan Maidanski (ivmai)]

There is a more generic fix for this bug (insert this code just before generalize.h inclusion):

/* The generalization section. */
#if !defined(AO_GENERALIZE_TWICE) && defined(AO_CAN_EMUL_CAS) \
    && !defined(AO_HAVE_compare_and_swap_full)
# define AO_GENERALIZE_TWICE
#endif

I'll fix it in the upstream tomorrow.

[Dave Martin (dave-martin-arm)]

So long as the implementation of the atomics in libatomic-ops is correct, I don't see a strong need to change it. My view is that people shouldn't roll their own atomics implementations unnecessarily, but it's OK to use a library for it, so long as the library is correct.

The actual build failure here looks more like an issue with the exported headers from libatomic-ops than an issue with the _implementation_ of the operations -- is this a correct conclusion?

[Dave Martin (dave-martin-arm)]

Note: I spotted a minor error in the inline asm for AO_nop_full().

The register argument to the MCR instruction is mistakenly listed as an output constraint. This probably means that the register will contain garbage instead of the required value 0.

(This is unlikely to be causing actual problems at present; on all processor implementations I know of, the processor ignores the value in the register... but strictly speaking it should be 0 for future compatiblity.)

[Launchpad Janitor (janitor)]

This bug was fixed in the package libatomic-ops - 7.2~alpha5+cvs20100601-1ubuntu1

---------------
libatomic-ops (7.2~alpha5+cvs20100601-1ubuntu1) maverick; urgency=low

  * atomic_ops.h: Define AO_GENERALIZE_TWICE for armel. LP: #635388.
  * Fix inline asm for for data memory barrier on arm (Dave Martin).
 -- Matthias Klose <email address hidden> Fri, 17 Sep 2010 15:18:51 +0200

[Matthias Klose (doko)]

ceph built with the fixed libatomic-ops