On Fri, 2008-10-17 at 14:33 -0400, Paul Coccoli wrote: > On Fri, Oct 17, 2008 at 2:06 PM, Joern Nettingsmeier > <nettings@xxxxxxxxxxxxxxxxxxxxxx> wrote: > > excuse my chiming in here, i'm not really much of a c programmer... but > > how can this > > - rb->read_ptr += n1; > > - rb->read_ptr &= rb->size_mask; > > + rb->read_ptr = (rb->read_ptr + n1) & rb->size_mask; > > fix anything? > > > > iiuc, both versions are equivalent. a context switch could happen just > > as well after the parenthesis has been computed..!? > > putting stuff on one line doesn't make it atomic. maybe you are now > > getting another compiler optimization that helps to hide the bug? > > > > They are not equivalent. The first version modifies rb->readptr > twice; the second version modifies once. I can't say with any > certainty that that fixes the problem, but it makes me feel better > (and happens to pass this test). I don't believe this logic is correct. As I explained before, and as Fons reiterated, the code is safe because of the combination of (a) single-writer, single-reader semantics and (b) any errors caused by context switching a the wrong time cause an under-estimate rather than an over-estimate of the current situation vis-a-vis reading or writing. The fact that in the revised version read_ptr is only read once changes nothing, since only thread ever modifies read_ptr. It doesn't matter how many times it accessed to do the computation - it will NEVER change its value during this computation because the computation happens in the reader thread and the reader thread is the only place where read_ptr is modified. So, its interesting that this revised version passes the test, but I don't believe that this is the reason why it does so. The test that Olivier has written is essentially a no-op on a uniprocessor x86 system, because there are no cache coherency issues. The code *is* thread safe on a uniprocessor because of the logic above, not because we rely on instruction reordering/memory barrier processor behaviour. However, on an SMP system, the logic that applies to the single reader/single writer design may not be sufficient to ensure correctness, hence the interest in adding memory barriers to the code. I believe that PortAudio is correct to do this. Before we do this, though, we should determine why the current code can fail, and whether the "fix" mentioned above is actually a fix or simply changes the code enough to hide the real problem more frequently (which I suspect is the case). --p _______________________________________________ Linux-audio-user mailing list Linux-audio-user@xxxxxxxxxxxxxxxxxxxx http://lists.linuxaudio.org/mailman/listinfo/linux-audio-user
