You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Daniel Lescohier <da...@cbsi.com> on 2013/11/29 16:38:28 UTC

apr_atomic and memory barriers

apr_atomic.h says:

/*
 * Atomic operations on 32-bit values
 * Note: Each of these functions internally implements a memory barrier
 * on platforms that require it
 */

/**
 * atomically read an apr_uint32_t from memory
 * @param mem the pointer
 */
APR_DECLARE(apr_uint32_t) apr_atomic_read32(volatile apr_uint32_t *mem);

However, atomic/unix/builtins.c implementation is:

APR_DECLARE(apr_uint32_t) apr_atomic_read32(volatile apr_uint32_t *mem)
{
    return *mem;
}

Doesn't the implementation have to be the following in order to have a
memory barrier?

APR_DECLARE(apr_uint32_t) apr_atomic_read32(volatile apr_uint32_t *mem)
{
    __sync_synchronize();
    return *mem;
}

volatile only protects against compiler reordering; adding the
__sync_synchronize() also protects against processor reordering.

For background, see this email thread from January in dev@httpd.apache.org.
In this archive, oldest message is listed last:

http://marc.info/?t=135733584100001&r=1&w=2

In message 9 I outline a problem where the read without a barrier is a
problem.  In message 8, Stefan Fritsch mentioned the documentation I quoted
at the top of this email that documents that apr_atomic_read is supposed to
be "with a memory barrier."

Intel architecture has a pretty strong memory ordering guarantee, so in
practice it's not a problem on that architecture: but it could be a problem
on other architectures like ARM and PowerPC (see message 4 in the thread,
and Stefan's response in message 3).  The documentation of the current
apr_atomic API says that this function should have a memory barrier, so I
think it needs to be added to apr_atomic_read32.