linuxaudio.org

On Fri, 2008-10-17 at 14:33 -0400, Paul Coccoli wrote:

quoted text
> On Fri, Oct 17, 2008 at 2:06 PM, Joern Nettingsmeier

>  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@lists.linuxaudio.org

http://lists.linuxaudio.org/mailman/listinfo/linux-audio-user