From 9682a62743091463b8cfac540dbbf854d33128e5 Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Thu, 3 Nov 2016 00:47:22 -0700 Subject: [PATCH] Use proper atomics for the lockless ringbuffer --- Alc/alcRing.c | 91 ++++++++++++++++++++++++++++++++--------------------------- 1 file 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; -- 2.11.4.GIT