Use proper atomics for the lockless ringbuffer

1 files changed, 49 insertions, 42 deletions

diff --git a/Alc/alcRing.c b/Alc/alcRing.c
index 5994f196..8a77dda1 100644
--- a/Alc/alcRing.c
+++ b/Alc/alcRing.c
@@ -34,8 +34,8 @@
  * size or count is in 'elements', not bytes. Additionally, it only supports
  * single-consumer/single-provider operation. */
 struct ll_ringbuffer {
-    volatile size_t write_ptr;
-    volatile size_t read_ptr;
+    ATOMIC(size_t) write_ptr;
+    ATOMIC(size_t) read_ptr;
     size_t size;
     size_t size_mask;
     size_t elem_size;
@@ -58,11 +58,11 @@ ll_ringbuffer_t *ll_ringbuffer_create(size_t sz, size_t elem_sz)
     rb = al_malloc(16, sizeof(*rb) + power_of_two*elem_sz);
     if(!rb) return NULL;
 
+    ATOMIC_INIT(&rb->write_ptr, 0);
+    ATOMIC_INIT(&rb->read_ptr, 0);
     rb->size = power_of_two;
     rb->size_mask = rb->size - 1;
     rb->elem_size = elem_sz;
-    rb->write_ptr = 0;
-    rb->read_ptr = 0;
     rb->mlocked = 0;
     return rb;
 }
@@ -84,7 +84,7 @@ void ll_ringbuffer_free(ll_ringbuffer_t *rb)
 int ll_ringbuffer_mlock(ll_ringbuffer_t *rb)
 {
 #ifdef USE_MLOCK
-    if(!rb->locked && mlock(rb, sizeof(*rb) + rb->size*rb->elem_size))
+    if(!rb->mlocked && mlock(rb, sizeof(*rb) + rb->size*rb->elem_size))
         return -1;
 #endif /* USE_MLOCK */
     rb->mlocked = 1;
@@ -94,8 +94,8 @@ int ll_ringbuffer_mlock(ll_ringbuffer_t *rb)
 /* Reset the read and write pointers to zero. This is not thread safe. */
 void ll_ringbuffer_reset(ll_ringbuffer_t *rb)
 {
-    rb->read_ptr = 0;
-    rb->write_ptr = 0;
+    ATOMIC_STORE(&rb->write_ptr, 0, almemory_order_relaxed);
+    ATOMIC_STORE(&rb->read_ptr, 0, almemory_order_release);
     memset(rb->buf, 0, rb->size*rb->elem_size);
 }
 
@@ -103,16 +103,16 @@ void ll_ringbuffer_reset(ll_ringbuffer_t *rb)
  * elements in front of the read pointer and behind the write pointer. */
 size_t ll_ringbuffer_read_space(const ll_ringbuffer_t *rb)
 {
-    size_t w = rb->write_ptr;
-    size_t r = rb->read_ptr;
+    size_t w = ATOMIC_LOAD(&rb->write_ptr, almemory_order_acquire);
+    size_t r = ATOMIC_LOAD(&rb->read_ptr, almemory_order_relaxed);
     return (rb->size+w-r) & rb->size_mask;
 }
 /* Return the number of elements available for writing. This is the number of
  * elements in front of the write pointer and behind the read pointer. */
 size_t ll_ringbuffer_write_space(const ll_ringbuffer_t *rb)
 {
-    size_t w = rb->write_ptr;
-    size_t r = rb->read_ptr;
+    size_t w = ATOMIC_LOAD(&rb->write_ptr, almemory_order_acquire);
+    size_t r = ATOMIC_LOAD(&rb->read_ptr, almemory_order_relaxed);
     return (rb->size+r-w-1) & rb->size_mask;
 }
 
@@ -120,6 +120,7 @@ size_t ll_ringbuffer_write_space(const ll_ringbuffer_t *rb)
  * Returns the actual number of elements copied. */
 size_t ll_ringbuffer_read(ll_ringbuffer_t *rb, char *dest, size_t cnt)
 {
+    size_t read_ptr;
     size_t free_cnt;
     size_t cnt2;
     size_t to_read;
@@ -129,10 +130,12 @@ size_t ll_ringbuffer_read(ll_ringbuffer_t *rb, char *dest, size_t cnt)
     if(free_cnt == 0) return 0;
 
     to_read = (cnt > free_cnt) ? free_cnt : cnt;
-    cnt2 = rb->read_ptr + to_read;
+    read_ptr = ATOMIC_LOAD(&rb->read_ptr, almemory_order_relaxed) & rb->size_mask;
+
+    cnt2 = read_ptr + to_read;
     if(cnt2 > rb->size)
     {
-        n1 = rb->size - rb->read_ptr;
+        n1 = rb->size - read_ptr;
         n2 = cnt2 & rb->size_mask;
     }
     else
@@ -141,13 +144,14 @@ size_t ll_ringbuffer_read(ll_ringbuffer_t *rb, char *dest, size_t cnt)
         n2 = 0;
     }
 
-    memcpy(dest, &(rb->buf[rb->read_ptr*rb->elem_size]), n1*rb->elem_size);
-    rb->read_ptr = (rb->read_ptr + n1) & rb->size_mask;
+    memcpy(dest, &rb->buf[read_ptr*rb->elem_size], n1*rb->elem_size);
+    read_ptr = (read_ptr + n1) & rb->size_mask;
     if(n2)
     {
-        memcpy(dest + n1*rb->elem_size, &(rb->buf[rb->read_ptr*rb->elem_size]), n2*rb->elem_size);
-        rb->read_ptr = (rb->read_ptr + n2) & rb->size_mask;
+        memcpy(dest + n1*rb->elem_size, &rb->buf[read_ptr*rb->elem_size], n2*rb->elem_size);
+        read_ptr += n2;
     }
+    ATOMIC_STORE(&rb->read_ptr, read_ptr, almemory_order_release);
     return to_read;
 }
 
@@ -160,17 +164,18 @@ size_t ll_ringbuffer_peek(ll_ringbuffer_t *rb, char *dest, size_t cnt)
     size_t cnt2;
     size_t to_read;
     size_t n1, n2;
-    size_t tmp_read_ptr;
+    size_t read_ptr;
 
-    tmp_read_ptr = rb->read_ptr;
     free_cnt = ll_ringbuffer_read_space(rb);
     if(free_cnt == 0) return 0;
 
     to_read = (cnt > free_cnt) ? free_cnt : cnt;
-    cnt2 = tmp_read_ptr + to_read;
+    read_ptr = ATOMIC_LOAD(&rb->read_ptr, almemory_order_relaxed) & rb->size_mask;
+
+    cnt2 = read_ptr + to_read;
     if(cnt2 > rb->size)
     {
-        n1 = rb->size - tmp_read_ptr;
+        n1 = rb->size - read_ptr;
         n2 = cnt2 & rb->size_mask;
     }
     else
@@ -179,10 +184,10 @@ size_t ll_ringbuffer_peek(ll_ringbuffer_t *rb, char *dest, size_t cnt)
         n2 = 0;
     }
 
-    memcpy(dest, &(rb->buf[tmp_read_ptr*rb->elem_size]), n1*rb->elem_size);
-    tmp_read_ptr = (tmp_read_ptr + n1) & rb->size_mask;
+    memcpy(dest, &rb->buf[read_ptr*rb->elem_size], n1*rb->elem_size);
+    read_ptr = (read_ptr + n1) & rb->size_mask;
     if(n2)
-        memcpy(dest + n1*rb->elem_size, &(rb->buf[tmp_read_ptr*rb->elem_size]), n2*rb->elem_size);
+        memcpy(dest + n1*rb->elem_size, &rb->buf[read_ptr*rb->elem_size], n2*rb->elem_size);
     return to_read;
 }
 
@@ -190,6 +195,7 @@ size_t ll_ringbuffer_peek(ll_ringbuffer_t *rb, char *dest, size_t cnt)
  * Returns the actual number of elements copied. */
 size_t ll_ringbuffer_write(ll_ringbuffer_t *rb, const char *src, size_t cnt)
 {
+    size_t write_ptr;
     size_t free_cnt;
     size_t cnt2;
     size_t to_write;
@@ -199,10 +205,12 @@ size_t ll_ringbuffer_write(ll_ringbuffer_t *rb, const char *src, size_t cnt)
     if(free_cnt == 0) return 0;
 
     to_write = (cnt > free_cnt) ? free_cnt : cnt;
-    cnt2 = rb->write_ptr + to_write;
+    write_ptr = ATOMIC_LOAD(&rb->write_ptr, almemory_order_relaxed) & rb->size_mask;
+
+    cnt2 = write_ptr + to_write;
     if(cnt2 > rb->size)
     {
-        n1 = rb->size - rb->write_ptr;
+        n1 = rb->size - write_ptr;
         n2 = cnt2 & rb->size_mask;
     }
     else
@@ -211,28 +219,27 @@ size_t ll_ringbuffer_write(ll_ringbuffer_t *rb, const char *src, size_t cnt)
         n2 = 0;
     }
 
-    memcpy(&(rb->buf[rb->write_ptr*rb->elem_size]), src, n1*rb->elem_size);
-    rb->write_ptr = (rb->write_ptr + n1) & rb->size_mask;
+    memcpy(&rb->buf[write_ptr*rb->elem_size], src, n1*rb->elem_size);
+    write_ptr = (write_ptr + n1) & rb->size_mask;
     if(n2)
     {
-        memcpy(&(rb->buf[rb->write_ptr*rb->elem_size]), src + n1*rb->elem_size, n2*rb->elem_size);
-        rb->write_ptr = (rb->write_ptr + n2) & rb->size_mask;
+        memcpy(&rb->buf[write_ptr*rb->elem_size], src + n1*rb->elem_size, n2*rb->elem_size);
+        write_ptr += n2;
     }
+    ATOMIC_STORE(&rb->write_ptr, write_ptr, almemory_order_release);
     return to_write;
 }
 
 /* Advance the read pointer `cnt' places. */
 void ll_ringbuffer_read_advance(ll_ringbuffer_t *rb, size_t cnt)
 {
-    size_t tmp = (rb->read_ptr + cnt) & rb->size_mask;
-    rb->read_ptr = tmp;
+    ATOMIC_ADD(size_t, &rb->read_ptr, cnt, almemory_order_acq_rel);
 }
 
 /* Advance the write pointer `cnt' places. */
 void ll_ringbuffer_write_advance(ll_ringbuffer_t *rb, size_t cnt)
 {
-    size_t tmp = (rb->write_ptr + cnt) & rb->size_mask;
-    rb->write_ptr = tmp;
+    ATOMIC_ADD(size_t, &rb->write_ptr, cnt, almemory_order_acq_rel);
 }
 
 /* The non-copying data reader. `vec' is an array of two places. Set the values
@@ -244,8 +251,8 @@ void ll_ringbuffer_get_read_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data
     size_t cnt2;
     size_t w, r;
 
-    w = rb->write_ptr;
-    r = rb->read_ptr;
+    w = ATOMIC_LOAD(&rb->write_ptr, almemory_order_acquire) & rb->size_mask;
+    r = ATOMIC_LOAD(&rb->read_ptr, almemory_order_relaxed) & rb->size_mask;
     free_cnt = (rb->size+w-r) & rb->size_mask;
 
     cnt2 = r + free_cnt;
@@ -253,7 +260,7 @@ void ll_ringbuffer_get_read_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data
     {
         /* Two part vector: the rest of the buffer after the current write ptr,
          * plus some from the start of the buffer. */
-        vec[0].buf = (char*)&(rb->buf[r*rb->elem_size]);
+        vec[0].buf = (char*)&rb->buf[r*rb->elem_size];
         vec[0].len = rb->size - r;
         vec[1].buf = (char*)rb->buf;
         vec[1].len = cnt2 & rb->size_mask;
@@ -261,7 +268,7 @@ void ll_ringbuffer_get_read_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data
     else
     {
         /* Single part vector: just the rest of the buffer */
-        vec[0].buf = (char*)&(rb->buf[r*rb->elem_size]);
+        vec[0].buf = (char*)&rb->buf[r*rb->elem_size];
         vec[0].len = free_cnt;
         vec[1].buf = NULL;
         vec[1].len = 0;
@@ -277,8 +284,8 @@ void ll_ringbuffer_get_write_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_dat
     size_t cnt2;
     size_t w, r;
 
-    w = rb->write_ptr;
-    r = rb->read_ptr;
+    w = ATOMIC_LOAD(&rb->write_ptr, almemory_order_acquire) & rb->size_mask;
+    r = ATOMIC_LOAD(&rb->read_ptr, almemory_order_relaxed) & rb->size_mask;
     free_cnt = (rb->size+r-w-1) & rb->size_mask;
 
     cnt2 = w + free_cnt;
@@ -286,14 +293,14 @@ void ll_ringbuffer_get_write_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_dat
     {
         /* Two part vector: the rest of the buffer after the current write ptr,
          * plus some from the start of the buffer. */
-        vec[0].buf = (char*)&(rb->buf[w*rb->elem_size]);
+        vec[0].buf = (char*)&rb->buf[w*rb->elem_size];
         vec[0].len = rb->size - w;
         vec[1].buf = (char*)rb->buf;
         vec[1].len = cnt2 & rb->size_mask;
     }
     else
     {
-        vec[0].buf = (char*)&(rb->buf[w*rb->elem_size]);
+        vec[0].buf = (char*)&rb->buf[w*rb->elem_size];
         vec[0].len = free_cnt;
         vec[1].buf = NULL;
         vec[1].len = 0;