2/3: implement DMX_SET_BUFFER_SIZE for dvr

This patch implements the ioctl DMX_SET_BUFFER_SIZE for the dvr.

The ioctl used to be supported but not yet implemented.

The way it works is that it replaces the ringbuffer with a new one.
Beforehand it flushes the old buffer.
This means that part of the stream is lost, and reading errors (like overflow) are cleaned. The flushing is not a problem since this operation usually occurs at beginning before start reading.

Since the dvr is *always* up and running this change has to be done while the buffer is written:

1) On the userspace side, it is as safe as dvb_dvr_read is now:
- dvb_dvr_set_buffer_size locks the mutex
- dvb_dvr_read does *not* lock the mutex (the code is there commented out)

So as long as one does not call read simultaneously it works properly. Maybe the mutex should be enforced in dvb_dvr_read.

2) On the kernel side
The only thing I am not 100% sure about is whether the spin_lock I've used is enough to guarantee synchronization between the new function dvb_dvr_set_buffer_size (which uses spin_lock_irq) and dvb_dmxdev_ts_callback and dvb_dmxdev_section_callback (using spin_lock).
But this looks to me the same as all other functions do.

I would like to change documentation about DMX_SET_BUFFER_SIZE, but I could not find the source of
http://www.linuxtv.org/docs/dvbapi/DVB_Demux_Device.html

Andrea

diff -r 1886a5ea2f84 linux/drivers/media/dvb/dvb-core/dmxdev.c
@@ -259,6 +259,37 @@ static ssize_t dvb_dvr_read(struct file 
 	return ret;
 }
 
+static int dvb_dvr_set_buffer_size(struct dmxdev *dmxdev,
+				      unsigned long size)
+{
+	struct dvb_ringbuffer *buf = &dmxdev->dvr_buffer;
+	void *mem;
+
+	if (buf->size == size)
+		return 0;
+
+	dprintk("function : %s\n", __FUNCTION__);
+
+	spin_lock(&dmxdev->lock);
+	mem = buf->data;
+	buf->data = NULL;
+	buf->size = size;
+	dvb_ringbuffer_flush(buf);
+	spin_unlock(&dmxdev->lock);
+	vfree(mem);
+
+	if (buf->size) {
+		mem = vmalloc(dmxdev->dvr_buffer.size);
+		if (!mem)
+			return -ENOMEM;
+		spin_lock(&dmxdev->lock);
+		buf->data = mem;
+		spin_unlock(&dmxdev->lock);
+	}
+
+	return 0;
+}
+
 static inline void dvb_dmxdev_filter_state_set(struct dmxdev_filter
 					       *dmxdevfilter, int state)
 {

@@ -1009,6 +1040,7 @@ static int dvb_dvr_do_ioctl(struct inode
 {
 	struct dvb_device *dvbdev = file->private_data;
 	struct dmxdev *dmxdev = dvbdev->priv;
+	unsigned long arg = (unsigned long)parg;
 	int ret;
 
 	if (mutex_lock_interruptible(&dmxdev->mutex))

@@ -1016,8 +1048,7 @@ static int dvb_dvr_do_ioctl(struct inode
 
 	switch (cmd) {
 	case DMX_SET_BUFFER_SIZE:
-		// FIXME: implement
-		ret = 0;
+		ret = dvb_dvr_set_buffer_size(dmxdev, arg);
 		break;
 
 	default:

_______________________________________________
linux-dvb mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Please see my other mail, too

I see some problems here:
- Do not release the lock before the ringbuffer is consistent again.
- We should not free the old buffer before we got a new one.
- As the ring buffer can be written from an ISR, we have to use spin_lock_irqsave/spin_unlock_irqrestore here.

What about this fragment:

	...
	if (!size)
		return -EINVAL;

	mem = vmalloc(size);
	if (!mem)
		return -ENOMEM;

	mem2 = buf->data;

        spin_lock_irqsave(&dmxdev->lock);
        buf->pread = buf->pwrite = 0;
	buf->data = mem;
	buf->size = size;
	spin_unlock_irqrestore(&dmxdev->lock);

	vfree(mem2);
	return 0;

Any comemnts? I hope that someone else also has a look at this because I don't have much time atm.

CU
Oliver

----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
----------------------------------------------------------------

_______________________________________________
linux-dvb mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb