Re: [LAD] Atomic Operations

[Olivier Guilyardi]

On 12/17/2009 04:00 PM, Tim Goetze wrote:
> [Olivier Guilyardi]
> 
>> On 12/17/2009 01:03 PM, Tim Blechmann wrote:
>>>> +#if defined(__APPLE__)
>>>> +#include <libkern/OSAtomic.h>
>>>> +#define MEMORY_BARRIER() OSMemoryBarrier()
>>>> +#elif (__GNUC__ > 4) || (__GNUC__ == 4 && __GNUC_MINOR__ >= 1)
>>>> +#define MEMORY_BARRIER() __sync_synchronize()
>>>> +#else
>>>> +#warning SMP Danger: memory barriers are not supported on this system
>>>> +#endif
> 
> I may be wrong but if neither __APPLE__ nor __GNUC__ >= 4.1 are true
> shouldn't MEMORY_BARRIER() at least be an empty define to keep cpp 
> happy?  
> 
> After all, your #warning statement seems to indicate you don't want 
> compilation to fail.

Yes indeed. Fixed.

--
  Olivier




Index: libjack/ringbuffer.c
===================================================================
--- libjack/ringbuffer.c        (revision 3862)
+++ libjack/ringbuffer.c        (working copy)
@@ -29,6 +29,16 @@
 #endif /* USE_MLOCK */
 #include <jack/ringbuffer.h>
 
+#if defined(__APPLE__)
+#include <libkern/OSAtomic.h>
+#define MEMORY_BARRIER() OSMemoryBarrier()
+#elif (__GNUC__ > 4) || (__GNUC__ == 4 && __GNUC_MINOR__ >= 1)
+#define MEMORY_BARRIER() __sync_synchronize()
+#else
+#warning SMP Danger: memory barriers are not supported on this system
+#define MEMORY_BARRIER()
+#endif
+
 /* Create a new ringbuffer to hold at least `sz' bytes of data. The
    actual buffer size is rounded up to the next power of two.  */
 
@@ -146,6 +156,7 @@
        size_t cnt2;
        size_t to_read;
        size_t n1, n2;
+       size_t new_ptr;
 
        if ((free_cnt = jack_ringbuffer_read_space (rb)) == 0) {
                return 0;
@@ -164,13 +175,17 @@
        }
 
        memcpy (dest, &(rb->buf[rb->read_ptr]), n1);
-       rb->read_ptr = (rb->read_ptr + n1) & rb->size_mask;
+       new_ptr = (rb->read_ptr + n1) & rb->size_mask;
 
        if (n2) {
-               memcpy (dest + n1, &(rb->buf[rb->read_ptr]), n2);
-               rb->read_ptr = (rb->read_ptr + n2) & rb->size_mask;
+               memcpy (dest + n1, &(rb->buf[new_ptr]), n2);
+               new_ptr = (new_ptr + n2) & rb->size_mask;
        }
 
+       /* Ensure that the read pointer gets updated after copying the data */
+       MEMORY_BARRIER();
+       rb->read_ptr = new_ptr;
+
        return to_read;
 }
 
@@ -226,6 +241,7 @@
        size_t cnt2;
        size_t to_write;
        size_t n1, n2;
+       size_t new_ptr;
 
        if ((free_cnt = jack_ringbuffer_write_space (rb)) == 0) {
                return 0;
@@ -244,13 +260,17 @@
        }
 
        memcpy (&(rb->buf[rb->write_ptr]), src, n1);
-       rb->write_ptr = (rb->write_ptr + n1) & rb->size_mask;
+       new_ptr = (rb->write_ptr + n1) & rb->size_mask;
 
        if (n2) {
-               memcpy (&(rb->buf[rb->write_ptr]), src + n1, n2);
-               rb->write_ptr = (rb->write_ptr + n2) & rb->size_mask;
+               memcpy (&(rb->buf[new_ptr]), src + n1, n2);
+               new_ptr = (new_ptr + n2) & rb->size_mask;
        }
 
+       /* Ensure that the write pointer gets updated after copying the data */
+       MEMORY_BARRIER();
+       rb->write_ptr = new_ptr;
+
        return to_write;
 }
 
@@ -260,6 +280,8 @@
 jack_ringbuffer_read_advance (jack_ringbuffer_t * rb, size_t cnt)
 {
        size_t tmp = (rb->read_ptr + cnt) & rb->size_mask;
+       /* Ensure that the read pointer gets updated after external data change 
*/
+       MEMORY_BARRIER();
        rb->read_ptr = tmp;
 }
 
@@ -269,6 +291,8 @@
 jack_ringbuffer_write_advance (jack_ringbuffer_t * rb, size_t cnt)
 {
        size_t tmp = (rb->write_ptr + cnt) & rb->size_mask;
+       /* Ensure that the write pointer gets updated after external data 
change */
+       MEMORY_BARRIER();
        rb->write_ptr = tmp;
 }
 

_______________________________________________
Linux-audio-dev mailing list
Linux-audio-dev@lists.linuxaudio.org
http://lists.linuxaudio.org/mailman/listinfo/linux-audio-dev