[PATCH 2 of 2] Core: add ngx_atomic_store() and ngx_atomic_load()

[Piotr Sikora]

# HG changeset patch
# User Piotr Sikora <piotrsikora at google.com>
# Date 1471265532 25200
#      Mon Aug 15 05:52:12 2016 -0700
# Node ID 40765d8ee4dd29089b0e60ed5b6099ac624e804e
# Parent  2f2ec92c3af93c11e195fb6d805df57518fede7c
Core: add ngx_atomic_store() and ngx_atomic_load().

Those functions must be used to prevent data races between
threads operating concurrently on the same variables.

No performance loss measured in microbenchmarks on x86_64.

No binary changes when compiled without __atomic intrinsics.

Found with ThreadSanitizer.

Signed-off-by: Piotr Sikora <piotrsikora at google.com>

diff -r 2f2ec92c3af9 -r 40765d8ee4dd auto/cc/conf
--- a/auto/cc/conf
+++ b/auto/cc/conf
@@ -178,7 +178,7 @@ if [ "$NGX_PLATFORM" != win32 ]; then
     fi
 
 
-    ngx_feature="gcc builtin atomic operations"
+    ngx_feature="gcc builtin __sync operations"
     ngx_feature_name=NGX_HAVE_GCC_ATOMIC
     ngx_feature_run=yes
     ngx_feature_incs=
@@ -195,6 +195,19 @@ if [ "$NGX_PLATFORM" != win32 ]; then
     . auto/feature
 
 
+    ngx_feature="gcc builtin __atomic operations"
+    ngx_feature_name=NGX_HAVE_GCC_ATOMIC_STORE_AND_LOAD
+    ngx_feature_run=yes
+    ngx_feature_incs=
+    ngx_feature_path=
+    ngx_feature_libs=
+    ngx_feature_test="long  n = 0;
+                      __atomic_store_n(&n, 1, __ATOMIC_RELEASE);
+                      if (__atomic_load_n(&n, __ATOMIC_ACQUIRE) != 1)
+                          return 1"
+    . auto/feature
+
+
     if [ "$NGX_CC_NAME" = "ccc" ]; then
         echo "checking for C99 variadic macros ... disabled"
     else
diff -r 2f2ec92c3af9 -r 40765d8ee4dd src/core/ngx_rwlock.c
--- a/src/core/ngx_rwlock.c
+++ b/src/core/ngx_rwlock.c
@@ -53,7 +53,7 @@ ngx_rwlock_rlock(ngx_atomic_t *lock)
     ngx_atomic_uint_t  readers;
 
     for ( ;; ) {
-        readers = *lock;
+        readers = ngx_atomic_load(lock);
 
         if (readers != NGX_RWLOCK_WLOCK
             && ngx_atomic_cmp_set(lock, readers, readers + 1))
@@ -69,7 +69,7 @@ ngx_rwlock_rlock(ngx_atomic_t *lock)
                     ngx_cpu_pause();
                 }
 
-                readers = *lock;
+                readers = ngx_atomic_load(lock);
 
                 if (readers != NGX_RWLOCK_WLOCK
                     && ngx_atomic_cmp_set(lock, readers, readers + 1))
@@ -89,10 +89,10 @@ ngx_rwlock_unlock(ngx_atomic_t *lock)
 {
     ngx_atomic_uint_t  readers;
 
-    readers = *lock;
+    readers = ngx_atomic_load(lock);
 
     if (readers == NGX_RWLOCK_WLOCK) {
-        *lock = 0;
+        ngx_atomic_store(lock, 0);
         return;
     }
 
@@ -102,7 +102,7 @@ ngx_rwlock_unlock(ngx_atomic_t *lock)
             return;
         }
 
-        readers = *lock;
+        readers = ngx_atomic_load(lock);
     }
 }
 
diff -r 2f2ec92c3af9 -r 40765d8ee4dd src/core/ngx_shmtx.c
--- a/src/core/ngx_shmtx.c
+++ b/src/core/ngx_shmtx.c
@@ -171,7 +171,7 @@ ngx_shmtx_wakeup(ngx_shmtx_t *mtx)
 
     for ( ;; ) {
 
-        wait = *mtx->wait;
+        wait = ngx_atomic_load(mtx->wait);
 
         if ((ngx_atomic_int_t) wait <= 0) {
             return;
diff -r 2f2ec92c3af9 -r 40765d8ee4dd src/core/ngx_thread_pool.c
--- a/src/core/ngx_thread_pool.c
+++ b/src/core/ngx_thread_pool.c
@@ -164,9 +164,9 @@ ngx_thread_pool_init(ngx_thread_pool_t *
 static void
 ngx_thread_pool_destroy(ngx_thread_pool_t *tp)
 {
-    ngx_uint_t           n;
-    ngx_thread_task_t    task;
-    volatile ngx_uint_t  lock;
+    ngx_uint_t         n;
+    ngx_atomic_t       lock;
+    ngx_thread_task_t  task;
 
     ngx_memzero(&task, sizeof(ngx_thread_task_t));
 
@@ -174,13 +174,13 @@ ngx_thread_pool_destroy(ngx_thread_pool_
     task.ctx = (void *) &lock;
 
     for (n = 0; n < tp->threads; n++) {
-        lock = 1;
+        ngx_atomic_store(&lock, 1);
 
         if (ngx_thread_task_post(tp, &task) != NGX_OK) {
             return;
         }
 
-        while (lock) {
+        while (ngx_atomic_load(&lock) != 0) {
             ngx_sched_yield();
         }
 
@@ -196,9 +196,9 @@ ngx_thread_pool_destroy(ngx_thread_pool_
 static void
 ngx_thread_pool_exit_handler(void *data, ngx_log_t *log)
 {
-    ngx_uint_t *lock = data;
+    ngx_atomic_uint_t  *lock = data;
 
-    *lock = 0;
+    ngx_atomic_store(lock, 0);
 
     pthread_exit(0);
 }
diff -r 2f2ec92c3af9 -r 40765d8ee4dd src/os/unix/ngx_atomic.h
--- a/src/os/unix/ngx_atomic.h
+++ b/src/os/unix/ngx_atomic.h
@@ -304,12 +304,25 @@ ngx_atomic_fetch_add(ngx_atomic_t *value
 #endif
 
 
+#if (NGX_HAVE_GCC_ATOMIC_STORE_AND_LOAD)
+
+#define ngx_atomic_store(x, value)  __atomic_store_n(x, value, __ATOMIC_RELEASE)
+#define ngx_atomic_load(x)          __atomic_load_n(x, __ATOMIC_ACQUIRE)
+
+#else
+
+#define ngx_atomic_store(x, value)  *(x) = value
+#define ngx_atomic_load(x)          *(x)
+
+#endif
+
+
 void ngx_spinlock(ngx_atomic_t *lock, ngx_atomic_int_t value, ngx_uint_t spin);
 
 #define ngx_trylock(lock, value)                                              \
-    (*(lock) == 0 && ngx_atomic_cmp_set(lock, 0, value))
+    (ngx_atomic_load(lock) == 0 && ngx_atomic_cmp_set(lock, 0, value))
 
-#define ngx_unlock(lock)    *(lock) = 0
+#define ngx_unlock(lock)    ngx_atomic_store(lock, 0)
 
 
 #endif /* _NGX_ATOMIC_H_INCLUDED_ */
diff -r 2f2ec92c3af9 -r 40765d8ee4dd src/os/win32/ngx_atomic.h
--- a/src/os/win32/ngx_atomic.h
+++ b/src/os/win32/ngx_atomic.h
@@ -60,12 +60,16 @@ typedef volatile ngx_atomic_uint_t  ngx_
 #endif
 
 
+#define ngx_atomic_store(x, value)  *(x) = value
+#define ngx_atomic_load(x)          *(x)
+
+
 void ngx_spinlock(ngx_atomic_t *lock, ngx_atomic_int_t value, ngx_uint_t spin);
 
 #define ngx_trylock(lock, value)                                              \
-    (*(lock) == 0 && ngx_atomic_cmp_set(lock, 0, value))
+    (ngx_atomic_load(lock) == 0 && ngx_atomic_cmp_set(lock, 0, value))
 
-#define ngx_unlock(lock)    *(lock) = 0
+#define ngx_unlock(lock)    ngx_atomic_store(lock, 0)
 
 
 #endif /* _NGX_ATOMIC_H_INCLUDED_ */