18 Oct
2008
18 Oct
'08
4:17 p.m.
On Fri, Oct 17, 2008 at 04:39:25PM +0200, Olivier Guilyardi wrote:
Paul Coccoli wrote:
Why do you suspect you need memory barriers? My concern with
ringbuffer.c is the non-atomic ops on the read and write pointers.
They're marked volatile, but what I think you really want is make all
ops on those fields atomic. Stuff like this:
[...]
As I said, the Portaudio ringbuffer passes the test even with memory barriers
deactivated. But the lack of memory barriers in jack code was the reason I wrote
this test, please read the whole thread for links to relevant documents.
Yeah, at PulseAudio they say that the way jack ringbuffer uses volatile is wrong.
Anyway, I've read a lot of different (and very opposed) point of views on these
matters, and IMO there's nothing like thorough testing to sort this out.
To figure whether memory barriers are needed or not, we need to run the test on
various architectures.
For this purpose, I've set up a small svn repo with everything to run the test
easily. There's no dependency, you just need pthread, gcc and sh.
Here's how to run the test:
svn co http://svn.samalyse.com/misc/rbtest
cd rbtest
make test
It tests the ringbuffers from (all included) jack, portaudio and portaudio
without barriers. That can take up to 6 minutes. It has to be a bit long due to
the probabilistic nature of failures.
LAUsers could you please run this test on various architectures? Especially
PowerPC single-cpu and SMP, x86 single-cpu, SMP, in various flavors, etc...
cc -Wall -I. -I./jack -lpthread -o test-int-array-jack \
test-int-array.c jack/ringbuffer.c
test-int-array.c: In function ‘main’:
test-int-array.c:113: warning: format ‘%d’ expects type ‘int’, but argument 2 has type
‘long unsigned int’
cc -Wall -I. -I./portaudio -lpthread -o test-int-array-portaudio \
test-int-array.c portaudio/ringbuffer.c portaudio/pa_ringbuffer.c
test-int-array.c: In function ‘main’:
test-int-array.c:113: warning: format ‘%d’ expects type ‘int’, but argument 2 has type
‘long unsigned int’
cc -Wall -I. -I./portaudio -lpthread -o test-int-array-portaudio-nobarrier \
-DNO_MEMORY_BARRIER \
test-int-array.c portaudio/ringbuffer.c portaudio/pa_ringbuffer.c
test-int-array.c: In function ‘main’:
test-int-array.c:113: warning: format ‘%d’ expects type ‘int’, but argument 2 has type
‘long unsigned int’
cc -Wall -I. -I./jack -lpthread -o test-int-array-jack-fix1 \
test-int-array.c jack/ringbuffer-fix1.c
test-int-array.c: In function ‘main’:
test-int-array.c:113: warning: format ‘%d’ expects type ‘int’, but argument 2 has type
‘long unsigned int’
./alltests.sh
Starting ringbuffer tests (buffer size: 512)
=== Jack ringbuffer test ===
starting ringbuffer stress test (2 minutes max)
buffer size (bytes): 512
array size (bytes): 256
./alltests.sh: line 9: 7742 Segmentation fault ./test-int-array-jack $BUFFER_SIZE
=== Jack ringbuffer test (with fix 1) ===
starting ringbuffer stress test (2 minutes max)
buffer size (bytes): 512
array size (bytes): 256
./alltests.sh: line 13: 7745 Segmentation fault ./test-int-array-jack-fix1
$BUFFER_SIZE
=== Portaudio ringbuffer test ===
starting ringbuffer stress test (2 minutes max)
buffer size (bytes): 512
array size (bytes): 256
./alltests.sh: line 17: 7748 Segmentation fault ./test-int-array-portaudio
$BUFFER_SIZE
=== Portaudio ringbuffer test (without memory barriers) ===
starting ringbuffer stress test (2 minutes max)
buffer size (bytes): 512
array size (bytes): 256
./alltests.sh: line 21: 7751 Segmentation fault ./test-int-array-portaudio-nobarrier
$BUFFER_SIZE
make: *** [test] Error 139
Info on the system this was run on, is here:
http://restivo.nfshost.com/projects/asus/system-details/
-ken
On Fri, Oct 17, 2008 at 8:46 AM, Paul Davis
<paul(a)linuxaudiosystems.com> wrote:
> On Fri, 2008-10-17 at 14:13 +0200, Olivier Guilyardi wrote:
>
>> Okay, I wrote such a test. It fails with Jack's ringbuffer (jack1 r3004) but
>> succeeds with Portaudio's one (r1240).
> Nice work. Nobody ask why we didn't do this 5 years ago!
Thanks Paul :)
The Portaudio code looks more and more robust to
me. It's also surprisingly
short. Maybe that the best would be to replace jack's ringbuffer with it? I
think it should be possible to keep the jack_ringbuffer api unchanged.
I'd
rather add the memory barriers to the JACK code, but this could be a
race to see who does what first. A memory barrier is typically single
instruction. The complication tends to be defining them in a
sufficiently portable way.