Ralph Metzler wrote:
> Andreas Oberritter writes:
> > On Tue, 2006-03-14 at 01:03 +0100, Oliver Endriss wrote:
> > > Andreas Oberritter wrote:
> > > > From: Andreas Oberritter <obi@xxxxxxxxxxx>
> > > >
> > > > Added variable 'error' to struct dvb_ringbuffer, which is set to zero on
> > > > init() and flush(). Also reset read an write pointers to zero on flush()
> > > > to get less fragmented data.
> > > >
> > > > Signed-off-by: Andreas Oberritter <obi@xxxxxxxxxxx>
> > > > ---
> > > >
> > > > A patch to make dmxdev use dvb_ringbuffer will follow.
> > > >
> > > > diff -r 427667c87c7b linux/drivers/media/dvb/dvb-core/dvb_ringbuffer.c
> > > > --- a/linux/drivers/media/dvb/dvb-core/dvb_ringbuffer.c Sun Mar 12 00:03:47 2006 -0300
> > > > +++ b/linux/drivers/media/dvb/dvb-core/dvb_ringbuffer.c Mon Mar 13 16:02:46 2006 +0100
> > > > ...
> > > > @@ -86,7 +87,8 @@ ssize_t dvb_ringbuffer_avail(struct dvb_
> > > >
> > > > void dvb_ringbuffer_flush(struct dvb_ringbuffer *rbuf)
> > > > {
> > > > - rbuf->pread = rbuf->pwrite;
> > > > + rbuf->pread = rbuf->pwrite = 0;
> > > +++
> > >
> > > Attention, this will convert dvb_ringbuffer_flush() into a writer!
> > >
> > > from dvb_ringbuffer.h:
> > > | ** (2) If there is exactly one reader and one writer, there is no need
> > > | ** to lock read or write operations.
> > > | ** Two or more readers must be locked against each other.
> > > | ** Flushing the buffer counts as a read operation.
> > > +++++++++++++++++++++++++++++++++++++++++++++++
> > > | ** Two or more writers must be locked against each other.
> > >
> > > With this patch flushing the ring buffer is a read _and_ a write
> > > operation. It might break existing code. Are you aware of that?
> >
> > Oliver, can you please take a look at the existing code? It's a
> > performance gain if it doesn't break.
>
> It will definitely break the code. Just look at the writing calls.
> If pwrite changes to 0 right in the middle of one, you will have problems.
> For this case (pwrite=0) you should not get segmentation faults
> but pwrite can end up in a wrong position (!=0) and there will be
> some bogus data at the beginning of the buffer.
>
> > Generally I'd expect a flush to return a buffer into its initial state.
>
> Then you will have to add mutexes and/or locks, at least in the
> write and flush calls. This will of course complicate the code and
> also harm performance. But maybe in your case less than the
> fragmentation you were talking about?
Ack. It will break the av7110 driver. I did not check other drivers.
We would have to add spin_lock/spin_unlock calls around each
ringbuffer_write which would reduce write performance.
How could setting the buffer ptr to 0 increase performance?
Usually it will save a single wrap of the buffer.
Anyway, if you really need this, you should add a new function which
implements this behaviour (dvb_ringbuffer_reset?).
Whenever it is used write locking will be required.
CU
Oliver
--
--------------------------------------------------------
VDR Remote Plugin available at
http://www.escape-edv.de/endriss/vdr/
--------------------------------------------------------
_______________________________________________
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
