Thanks to all who provided feedback on the apr_atomic API
changes that I posted last week. Here's the updated patch
that I'm planning to commit; I'll wait a few days in case
more people want to comment.
Things that have changed since the original patch:
- Added a new function apr_atomic_xchg32 to exchange
two values
- Documented that the atomic functions will implement
a memory barrier internally
Changes not yet made:
- Removal of the old API (I saw two comments in favor
of this, including my own; I'll wait for an official
3 +1s and no -1s before I start deleting stuff)
- Return values for the add, subtract, and inc functions
(I haven't figured out a way to implement these
efficiently)
Brian
Index: include/apr_atomic.h
===================================================================
RCS file: /home/cvs/apr/include/apr_atomic.h,v
retrieving revision 1.52
diff -u -r1.52 apr_atomic.h
--- include/apr_atomic.h 14 Sep 2003 02:56:52 -0000 1.52
+++ include/apr_atomic.h 19 Sep 2003 04:36:17 -0000
@@ -104,24 +104,28 @@
* @param mem the pointer
* @warning on certain platforms this number is not stored
* directly in the pointer. in others it is
+ * @deprecated
*/
apr_uint32_t apr_atomic_read(volatile apr_atomic_t *mem);
/**
* set the value for atomic.
* @param mem the pointer
* @param val the value
+ * @deprecated
*/
void apr_atomic_set(volatile apr_atomic_t *mem, apr_uint32_t val);
/**
* Add 'val' to the atomic variable
* @param mem pointer to the atomic value
* @param val the addition
+ * @deprecated
*/
void apr_atomic_add(volatile apr_atomic_t *mem, apr_uint32_t val);
/**
* increment the atomic variable by 1
* @param mem pointer to the atomic value
+ * @deprecated
*/
void apr_atomic_inc(volatile apr_atomic_t *mem);
@@ -129,6 +133,7 @@
* decrement the atomic variable by 1
* @param mem pointer to the atomic value
* @return zero if the value is zero, otherwise non-zero
+ * @deprecated
*/
int apr_atomic_dec(volatile apr_atomic_t *mem);
@@ -140,10 +145,76 @@
* @param cmp the value to compare it to
* @return the old value of the atomic
* @warning do not mix apr_atomic's with the CAS function.
+ * @deprecated
* on some platforms they may be implemented by different mechanisms
*/
apr_uint32_t apr_atomic_cas(volatile apr_uint32_t *mem, long with, long cmp);
+
+/*
+ * 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_uint32_t apr_atomic_read32(volatile apr_uint32_t *mem);
+
+/**
+ * atomically set an apr_uint32_t in memory
+ * @param mem pointer to the object
+ */
+void apr_atomic_set32(volatile apr_uint32_t *mem, apr_uint32_t val);
+
+/**
+ * atomically add 'val' to an apr_uint32_t
+ * @param mem pointer to the object
+ * @param val amount to add
+ */
+void apr_atomic_add32(volatile apr_uint32_t *mem, apr_uint32_t val);
+
+/**
+ * atomically subtract 'val' from an apr_uint32_t
+ * @param mem pointer to the object
+ * @param val amount to subtract
+ */
+void apr_atomic_sub32(volatile apr_uint32_t *mem, apr_uint32_t val);
+
+/**
+ * atomically increment an apr_uint32_t by 1
+ * @param mem pointer to the object
+ */
+void apr_atomic_inc32(volatile apr_uint32_t *mem);
+
+/**
+ * atomically decrement an apr_uint32_t by 1
+ * @param mem pointer to the atomic value
+ * @return zero if the value becomes zero on decrement, otherwise non-zero
+ */
+int apr_atomic_dec32(volatile apr_uint32_t *mem);
+
+/**
+ * compare an apr_uint32_t's value with 'cmp'.
+ * If they are the same swap the value with 'with'
+ * @param mem pointer to the value
+ * @param with what to swap it with
+ * @param cmp the value to compare it to
+ * @return the old value of *mem
+ */
+apr_uint32_t apr_atomic_cas32(volatile apr_uint32_t *mem, apr_uint32_t with,
+ apr_uint32_t cmp);
+
+/**
+ * exchange an apr_uint32_t's value with 'val'.
+ * @param mem pointer to the value
+ * @param val what to swap it with
+ * @return the old value of *mem
+ */
+apr_uint32_t apr_atomic_xchg32(volatile apr_uint32_t *mem, apr_uint32_t val);
+
/**
* compare the pointer's value with cmp.
* If they are the same swap the value with 'with'
@@ -172,7 +243,7 @@
#define apr_atomic_set(mem, val) InterlockedExchange(mem, val)
#define apr_atomic_read(mem) (*mem)
#define apr_atomic_cas(mem,with,cmp) InterlockedCompareExchange(mem,with,cmp)
-#define apr_atomic_init(pool) APR_SUCCESS
+ /*#define apr_atomic_init(pool) APR_SUCCESS*/
#define apr_atomic_casptr(mem,with,cmp)
InterlockedCompareExchangePointer(mem,with,cmp)
#elif defined(NETWARE)
@@ -183,7 +254,7 @@
#define apr_atomic_inc(mem) atomic_inc(mem)
#define apr_atomic_set(mem, val) (*mem = val)
#define apr_atomic_read(mem) (*mem)
-#define apr_atomic_init(pool) APR_SUCCESS
+ /*#define apr_atomic_init(pool) APR_SUCCESS*/
#define apr_atomic_cas(mem,with,cmp) atomic_cmpxchg((unsigned long
*)(mem),(unsigned long)(cmp),(unsigned long)(with))
int apr_atomic_dec(apr_atomic_t *mem);
@@ -211,6 +282,12 @@
#define apr_atomic_set(mem, val) atomic_set_int(mem, val)
#define apr_atomic_read(mem) (*mem)
+#define apr_atomic_add32(mem, val) apr_atomic_add(mem, val)
+#define apr_atomic_dec32(mem) apr_atomic_dec(mem)
+#define apr_atomic_inc32(mem) apr_atomic_inc(mem)
+#define apr_atomic_set32(mem) apr_atomic_set(mem)
+#define apr_atomic_read32(mem) apr_atomic_read(mem)
+
#elif (defined(__linux__) || defined(__EMX__)) && defined(__i386__) &&
!APR_FORCE_ATOMIC_GENERIC
#define apr_atomic_t apr_uint32_t
@@ -228,6 +305,12 @@
: "m" (*(mem)), "r" (val) \
: "memory")
+#define apr_atomic_sub(mem, val) \
+ asm volatile ("lock; subl %1, %0" \
+ : \
+ : "m" (*(mem)), "r" (val) \
+ : "memory")
+
#define apr_atomic_dec(mem) \
({ int prev; \
asm volatile ("mov $0, %%eax;\n\t" \
@@ -247,7 +330,16 @@
#define apr_atomic_set(mem, val) (*(mem) = val)
#define apr_atomic_read(mem) (*(mem))
-#define apr_atomic_init(pool) APR_SUCCESS
+
+#define apr_atomic_cas32(mem, with, cmp) apr_atomic_cas(mem, with, cmp)
+#define apr_atomic_add32(mem, val) apr_atomic_add(mem, val)
+#define apr_atomic_sub32(mem, val) apr_atomic_sub(mem, val)
+#define apr_atomic_dec32(mem) apr_atomic_dec(mem)
+#define apr_atomic_inc32(mem) apr_atomic_inc(mem)
+#define apr_atomic_set32(mem, val) apr_atomic_set(mem, val)
+#define apr_atomic_read32(mem) apr_atomic_read(mem)
+
+/*#define apr_atomic_init(pool) APR_SUCCESS*/
#elif defined(__MVS__) /* OS/390 */
@@ -261,7 +353,7 @@
#define apr_atomic_inc(mem) apr_atomic_add(mem, 1)
#define apr_atomic_dec(mem) apr_atomic_add(mem, -1)
-#define apr_atomic_init(pool) APR_SUCCESS
+/*#define apr_atomic_init(pool) APR_SUCCESS*/
/* warning: the following two operations, _read and _set, are atomic
* if the memory variables are aligned (the usual case).
@@ -322,6 +414,41 @@
#if !defined(apr_atomic_cas) && !defined(APR_OVERRIDE_ATOMIC_CAS)
apr_uint32_t apr_atomic_cas(volatile apr_uint32_t *mem,long with,long cmp);
+#define APR_ATOMIC_NEED_DEFAULT_INIT 1
+#endif
+
+#if !defined(apr_atomic_read32) && !defined(APR_OVERRIDE_ATOMIC_READ32)
+#define apr_atomic_read32(p) *p
+#endif
+
+#if !defined(apr_atomic_set32) && !defined(APR_OVERRIDE_ATOMIC_SET32)
+void apr_atomic_set32(volatile apr_uint32_t *mem, apr_uint32_t val);
+#define APR_ATOMIC_NEED_DEFAULT_INIT 1
+#endif
+
+#if !defined(apr_atomic_add32) && !defined(APR_OVERRIDE_ATOMIC_ADD32)
+void apr_atomic_add32(volatile apr_uint32_t *mem, apr_uint32_t val);
+#define APR_ATOMIC_NEED_DEFAULT_INIT 1
+#endif
+
+#if !defined(apr_atomic_inc32) && !defined(APR_OVERRIDE_ATOMIC_INC32)
+void apr_atomic_inc32(volatile apr_uint32_t *mem);
+#define APR_ATOMIC_NEED_DEFAULT_INIT 1
+#endif
+
+#if !defined(apr_atomic_dec32) && !defined(APR_OVERRIDE_ATOMIC_DEC32)
+int apr_atomic_dec32(volatile apr_uint32_t *mem);
+#define APR_ATOMIC_NEED_DEFAULT_INIT 1
+#endif
+
+#if !defined(apr_atomic_cas32) && !defined(APR_OVERRIDE_ATOMIC_CAS32)
+apr_uint32_t apr_atomic_cas32(volatile apr_uint32_t *mem, apr_uint32_t with,
+ apr_uint32_t cmp);
+#define APR_ATOMIC_NEED_DEFAULT_INIT 1
+#endif
+
+#if !defined(apr_atomic_xchg32) && !defined(APR_OVERRIDE_ATOMIC_XCHG32)
+apr_uint32_t apr_atomic_xchg32(volatile apr_uint32_t *mem, apr_uint32_t val);
#define APR_ATOMIC_NEED_DEFAULT_INIT 1
#endif
Index: atomic/unix/apr_atomic.c
===================================================================
RCS file: /home/cvs/apr/atomic/unix/apr_atomic.c,v
retrieving revision 1.22
diff -u -r1.22 apr_atomic.c
--- atomic/unix/apr_atomic.c 1 Jan 2003 00:01:41 -0000 1.22
+++ atomic/unix/apr_atomic.c 19 Sep 2003 04:36:17 -0000
@@ -184,6 +184,139 @@
}
#endif /*!defined(apr_atomic_cas) && !defined(APR_OVERRIDE_ATOMIC_CAS) */
+#if !defined(apr_atomic_add32) && !defined(APR_OVERRIDE_ATOMIC_ADD32)
+void apr_atomic_add32(volatile apr_uint32_t *mem, apr_uint32_t val)
+{
+#if APR_HAS_THREADS
+ apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
+
+ if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
+ *mem += val;
+ apr_thread_mutex_unlock(lock);
+ }
+#else
+ *mem += val;
+#endif /* APR_HAS_THREADS */
+}
+#endif /*!defined(apr_atomic_sub32) && !defined(APR_OVERRIDE_ATOMIC_SUB32) */
+
+#if !defined(apr_atomic_sub32) && !defined(APR_OVERRIDE_ATOMIC_SUB32)
+void apr_atomic_sub32(volatile apr_uint32_t *mem, apr_uint32_t val)
+{
+#if APR_HAS_THREADS
+ apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
+
+ if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
+ *mem -= val;
+ apr_thread_mutex_unlock(lock);
+ }
+#else
+ *mem -= val;
+#endif /* APR_HAS_THREADS */
+}
+#endif /*!defined(apr_atomic_sub32) && !defined(APR_OVERRIDE_ATOMIC_SUB32) */
+
+#if !defined(apr_atomic_set32) && !defined(APR_OVERRIDE_ATOMIC_SET32)
+void apr_atomic_set32(volatile apr_uint32_t *mem, apr_uint32_t val)
+{
+#if APR_HAS_THREADS
+ apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
+
+ if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
+ *mem = val;
+ apr_thread_mutex_unlock(lock);
+ }
+#else
+ *mem = val;
+#endif /* APR_HAS_THREADS */
+}
+#endif /*!defined(apr_atomic_set32) && !defined(APR_OVERRIDE_ATOMIC_SET32) */
+
+#if !defined(apr_atomic_inc32) && !defined(APR_OVERRIDE_ATOMIC_INC32)
+void apr_atomic_inc32(volatile apr_uint32_t *mem)
+{
+#if APR_HAS_THREADS
+ apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
+
+ if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
+ (*mem)++;
+ apr_thread_mutex_unlock(lock);
+ }
+#else
+ (*mem)++;
+#endif /* APR_HAS_THREADS */
+}
+#endif /*!defined(apr_atomic_inc32) && !defined(APR_OVERRIDE_ATOMIC_INC32) */
+
+#if !defined(apr_atomic_dec32) && !defined(APR_OVERRIDE_ATOMIC_DEC32)
+int apr_atomic_dec32(volatile apr_uint32_t *mem)
+{
+#if APR_HAS_THREADS
+ apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
+ apr_uint32_t new;
+
+ if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
+ (*mem)--;
+ new = *mem;
+ apr_thread_mutex_unlock(lock);
+ return new;
+ }
+#else
+ (*mem)--;
+#endif /* APR_HAS_THREADS */
+ return *mem;
+}
+#endif /*!defined(apr_atomic_dec32) && !defined(APR_OVERRIDE_ATOMIC_DEC32) */
+
+#if !defined(apr_atomic_cas32) && !defined(APR_OVERRIDE_ATOMIC_CAS32)
+apr_uint32_t apr_atomic_cas32(volatile apr_uint32_t *mem, apr_uint32_t with,
+ apr_uint32_t cmp)
+{
+ apr_uint32_t prev;
+#if APR_HAS_THREADS
+ apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
+
+ if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
+ prev = *mem;
+ if (prev == cmp) {
+ *mem = with;
+ }
+ apr_thread_mutex_unlock(lock);
+ return prev;
+ }
+ return *mem;
+#else
+ prev = *mem;
+ if (prev == cmp) {
+ *mem = with;
+ }
+ return prev;
+#endif /* APR_HAS_THREADS */
+}
+#endif /*!defined(apr_atomic_cas32) && !defined(APR_OVERRIDE_ATOMIC_CAS32) */
+
+#if !defined(apr_atomic_xchg32) && !defined(APR_OVERRIDE_ATOMIC_XCHG32)
+apr_uint32_t apr_atomic_xchg32(volatile apr_uint32_t *mem, apr_uint32_t val)
+{
+ apr_uint32_t prev;
+#if APR_HAS_THREADS
+ apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
+
+ if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
+ prev = *mem;
+ *mem = val;
+ apr_thread_mutex_unlock(lock);
+ return val;
+ }
+ return *mem;
+#else
+ prev = *mem;
+ *mem = val;
+ return prev;
+#endif /* APR_HAS_THREADS */
+}
+#endif /*!defined(apr_atomic_xchg32) && !defined(APR_OVERRIDE_ATOMIC_XCHG32) */
+
#if !defined(apr_atomic_casptr) && !defined(APR_OVERRIDE_ATOMIC_CASPTR)
void *apr_atomic_casptr(volatile void **mem, void *with, const void *cmp)
{
@@ -208,4 +341,4 @@
return prev;
#endif /* APR_HAS_THREADS */
}
-#endif /*!defined(apr_atomic_cas) && !defined(APR_OVERRIDE_ATOMIC_CAS) */
+#endif /*!defined(apr_atomic_casptr) && !defined(APR_OVERRIDE_ATOMIC_CASPTR) */
Index: test/testatomic.c
===================================================================
RCS file: /home/cvs/apr/test/testatomic.c,v
retrieving revision 1.28
diff -u -r1.28 testatomic.c
--- test/testatomic.c 19 Sep 2003 04:15:52 -0000 1.28
+++ test/testatomic.c 19 Sep 2003 04:36:17 -0000
@@ -70,6 +70,7 @@
apr_pool_t *context;
apr_atomic_t y; /* atomic locks */
+apr_uint32_t y32;
static apr_status_t check_basic_atomics()
{
@@ -164,6 +165,75 @@
return APR_SUCCESS;
}
+static apr_status_t check_basic_atomics32()
+{
+ apr_uint32_t oldval;
+ apr_uint32_t casval = 0;
+
+ apr_atomic_set32(&y32, 2);
+ printf("%-60s", "testing apr_atomic_dec32");
+ if (apr_atomic_dec32(&y32) == 0) {
+ fprintf(stderr, "Failed\noldval =%d should not be zero\n",
+ apr_atomic_read32(&y32));
+ return APR_EGENERAL;
+ }
+ if (apr_atomic_dec32(&y32) != 0) {
+ fprintf(stderr, "Failed\noldval =%d should be zero\n",
+ apr_atomic_read32(&y32));
+ return APR_EGENERAL;
+ }
+ printf("OK\n");
+
+ printf("%-60s", "testing apr_atomic_cas32");
+ oldval = apr_atomic_cas32(&casval, 12, 0);
+ if (oldval != 0) {
+ fprintf(stderr, "Failed\noldval =%d should be zero\n", oldval);
+ return APR_EGENERAL;
+ }
+ printf("OK\n");
+ printf("%-60s", "testing apr_atomic_cas32 - match non-null");
+ oldval = apr_atomic_cas32(&casval, 23, 12);
+ if (oldval != 12) {
+ fprintf(stderr, "Failed\noldval =%d should be 12 y=%d\n",
+ oldval, casval);
+ return APR_EGENERAL;
+ }
+ printf("OK\n");
+ printf("%-60s", "testing apr_atomic_cas32 - no match");
+ oldval = apr_atomic_cas32(&casval, 23, 12);
+ if (oldval != 23) {
+ fprintf(stderr, "Failed\noldval =%d should be 23 y=%d\n",
+ oldval, casval);
+ return APR_EGENERAL;
+ }
+ printf("OK\n");
+
+ printf("%-60s", "testing apr_atomic_add32");
+ apr_atomic_set32(&y32, 23);
+ apr_atomic_add32(&y32, 4);
+ if ((oldval = apr_atomic_read32(&y32)) != 27) {
+ fprintf(stderr,
+ "Failed\nAtomic Add doesn't add up ;( expected 27 got %d\n",
+ oldval);
+ return APR_EGENERAL;
+ }
+
+ printf("OK\n");
+ printf("%-60s", "testing add32/inc32/sub32");
+ apr_atomic_set32(&y32, 0);
+ apr_atomic_add32(&y32, 20);
+ apr_atomic_inc32(&y32);
+ apr_atomic_sub32(&y32, 10);
+ if (apr_atomic_read32(&y32) != 11) {
+ fprintf(stderr, "Failed.\natomics do not add up: expected 11 got %d\n",
+ apr_atomic_read32(&y32));
+ return APR_EGENERAL;
+ }
+ fprintf(stdout, "OK\n");
+
+ return APR_SUCCESS;
+}
+
#if !APR_HAS_THREADS
int main(void)
{
@@ -189,6 +259,12 @@
fprintf(stderr, "Failed.\n");
exit(-1);
}
+
+ rv = check_basic_atomics32();
+ if (rv != APR_SUCCESS) {
+ fprintf(stderr, "Failed.\n");
+ exit(-1);
+ }
return 0;
}
#else /* !APR_HAS_THREADS */
@@ -301,6 +377,13 @@
exit(-1);
}
apr_atomic_set(&y, 0);
+
+ rv = check_basic_atomics32();
+ if (rv != APR_SUCCESS) {
+ fprintf(stderr, "Failed.\n");
+ exit(-1);
+ }
+
printf("%-60s", "Starting all the threads");
for (i = 0; i < NUM_THREADS; i++) {
