[vdr] Re: Video Disk Recorder version 1.0.1

[Andreas Schultz <aschultz@cs.uni-magdeburg.de>]

On Monday 22 April 2002 15:10, Clemens Kirchgatterer wrote:
> Andreas Schultz <aschultz@cs.uni-magdeburg.de> wrote:
> > > IMHO the variables head and tail shouldn't be changed without hold
> > > Lock during the changes (ringbuffer.c cRingBufferLinear::Get() and
> > > cRingBufferLinear::Put()).  This may fix the problem seen by some
> >
> > Theoreticly you're right, but Get() is only used by exactly one output
> > thread and Put is only used by exactly one input thread, always. tail
> > is only modified by Get() and head is only modified by Put(), the only
> > piece that needs protection is therefor the space calculation at the
> > beginning of Get() and Put().
> > Your change does not hurt, but it also does not changes anything, i
> > would be surprised when it could fix the brocken recording problem.
>
> i just studied the source and i think you are right. to put a lock on
> places where it is not needed, isn't that good IMHO. it suggestes that
> there IS a race condition to the reader. at least there should be a
> comment, that this should be removed, if it proves to be a waste of cpu
> cycles. ;-)

there is a race, if one of the methods is used by multiple threads. This 
currently does not happen, but future add-ons might wish to do exactly that. 
The code could be rewriten to work for multiple concurrent calls without 
lock's. One the other hand, if you look at my -rt patch, you will find the 
exact same locking there. I use it in conjuntion with conditional variables 
to reduce latency between Get() and Put(). (it would also reduce parallelism 
on SMP systems, but who cares about that for a vdr box?)

BTW: my original question was out of curiosity and not becasue i thought 
something was wrong with the change.

Have fun

   Andreas