17 Oct
2008
17 Oct
'08
3:32 p.m.
On Fri, Oct 17, 2008 at 1:20 PM, Olivier Guilyardi <ml(a)xung.org> wrote:
Paul Coccoli wrote:
Looks like I don't have to "step back" or "read the whole thread for
links to relevant documents" then ;)
On Fri, Oct 17, 2008 at 8:46 AM, Paul Davis
<paul(a)linuxaudiosystems.com> wrote:
Well, it looks like you had a fast but great insight here. I've turned all
statements of this kind into one-liners, and the jack ringbuffer now
(apparently) passes the test.
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.
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:
rb->read_ptr += n1;
rb->read_ptr &= rb->size_mask;
Looks like a problem to me. What happens if there's a context switch
in between those 2 statements?
NB: I only took a cursory glance at the code. Here's the patch against jack1 r3007:
http://svn.samalyse.com/misc/rbtest/patches/jack-r3007-rb-fix.diff
And thanks everyone for the tests ! I've updated the test suite, it now contains
an additional test with this patch. Please continue testing :)
I didn't test your patch, but my own patch (which is the same thing)
works on dual Core2 duo system, whereas the original didn't.
No memory barriers needed *on x86* (volatile isn't needed anywhere).
Others system probably need memory barriers.