V4L/DVB (6538): em28xx: fix locking to allow accesses from 2 different threads at the same time
The attached patch modifies the em28xx driver so that there can be ioctls from
multiple different threads.
This is necessary for capture apps like MPlayer that use different threads for
capturing and channel tuning.
Now the locking is only done for the ioctls that change properties of the
device or access the i2c bus.
It also removes some locks that look unnecessary:
In em28xx_init_dev:
the videodevice is not registered yet so nothing can access the hardware
meanwhile, the device struct is not assigned to the interface yet so no race
with disconnect is possible
In em28xx_release_resources:
it gets only called when dev->lock is already held
Signed-off-by: Sascha Sommer <saschasommer@freenet.de>
Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>
diff --git a/drivers/media/video/em28xx/em28xx-video.c b/drivers/media/video/em28xx/em28xx-video.c
index 18b8568..bc495a1 100644
--- a/drivers/media/video/em28xx/em28xx-video.c
+++ b/drivers/media/video/em28xx/em28xx-video.c
@@ -126,8 +126,6 @@
static struct usb_driver em28xx_usb_driver;
-static DEFINE_MUTEX(em28xx_sysfs_lock);
-static DECLARE_RWSEM(em28xx_disconnect);
/********************* v4l2 interface ******************************************/
@@ -253,22 +251,18 @@
em28xx_videodbg("open minor=%d type=%s users=%d\n",
minor,v4l2_type_names[dev->type],dev->users);
- if (!down_read_trylock(&em28xx_disconnect))
- return -ERESTARTSYS;
+ mutex_lock(&dev->lock);
if (dev->users) {
em28xx_warn("this driver can be opened only once\n");
- up_read(&em28xx_disconnect);
+ mutex_unlock(&dev->lock);
return -EBUSY;
}
- mutex_init(&dev->fileop_lock); /* to 1 == available */
spin_lock_init(&dev->queue_lock);
init_waitqueue_head(&dev->wait_frame);
init_waitqueue_head(&dev->wait_stream);
- mutex_lock(&dev->lock);
-
if (dev->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
em28xx_set_alternate(dev);
@@ -306,7 +300,6 @@
err:
mutex_unlock(&dev->lock);
- up_read(&em28xx_disconnect);
return errCode;
}
@@ -317,7 +310,6 @@
*/
static void em28xx_release_resources(struct em28xx *dev)
{
- mutex_lock(&em28xx_sysfs_lock);
/*FIXME: I2C IR should be disconnected */
@@ -329,7 +321,6 @@
video_unregister_device(dev->vbi_dev);
em28xx_i2c_unregister(dev);
usb_put_dev(dev->udev);
- mutex_unlock(&em28xx_sysfs_lock);
/* Mark device as unused */
@@ -389,6 +380,8 @@
int ret = 0;
struct em28xx *dev = filp->private_data;
+ mutex_lock(&dev->lock);
+
if (dev->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
em28xx_videodbg("V4l2_Buf_type_videocapture is set\n");
}
@@ -396,47 +389,46 @@
em28xx_videodbg("V4L2_BUF_TYPE_VBI_CAPTURE is set\n");
em28xx_videodbg("not supported yet! ...\n");
if (copy_to_user(buf, "", 1)) {
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -EFAULT;
}
+ mutex_unlock(&dev->lock);
return (1);
}
if (dev->type == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE) {
em28xx_videodbg("V4L2_BUF_TYPE_SLICED_VBI_CAPTURE is set\n");
em28xx_videodbg("not supported yet! ...\n");
if (copy_to_user(buf, "", 1)) {
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -EFAULT;
}
+ mutex_unlock(&dev->lock);
return (1);
}
- if (mutex_lock_interruptible(&dev->fileop_lock))
- return -ERESTARTSYS;
-
if (dev->state & DEV_DISCONNECTED) {
em28xx_videodbg("device not present\n");
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -ENODEV;
}
if (dev->state & DEV_MISCONFIGURED) {
em28xx_videodbg("device misconfigured; close and open it again\n");
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -EIO;
}
if (dev->io == IO_MMAP) {
em28xx_videodbg ("IO method is set to mmap; close and open"
" the device again to choose the read method\n");
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -EINVAL;
}
if (dev->io == IO_NONE) {
if (!em28xx_request_buffers(dev, EM28XX_NUM_READ_FRAMES)) {
em28xx_errdev("read failed, not enough memory\n");
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -ENOMEM;
}
dev->io = IO_READ;
@@ -445,13 +437,13 @@
}
if (!count) {
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return 0;
}
if (list_empty(&dev->outqueue)) {
if (filp->f_flags & O_NONBLOCK) {
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -EAGAIN;
}
ret = wait_event_interruptible
@@ -459,11 +451,11 @@
(!list_empty(&dev->outqueue)) ||
(dev->state & DEV_DISCONNECTED));
if (ret) {
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return ret;
}
if (dev->state & DEV_DISCONNECTED) {
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -ENODEV;
}
}
@@ -482,12 +474,12 @@
count = f->buf.length;
if (copy_to_user(buf, f->bufmem, count)) {
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -EFAULT;
}
*f_pos += count;
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return count;
}
@@ -501,8 +493,7 @@
unsigned int mask = 0;
struct em28xx *dev = filp->private_data;
- if (mutex_lock_interruptible(&dev->fileop_lock))
- return POLLERR;
+ mutex_lock(&dev->lock);
if (dev->state & DEV_DISCONNECTED) {
em28xx_videodbg("device not present\n");
@@ -527,13 +518,13 @@
if (!list_empty(&dev->outqueue))
mask |= POLLIN | POLLRDNORM;
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return mask;
}
}
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return POLLERR;
}
@@ -575,25 +566,24 @@
struct em28xx *dev = filp->private_data;
- if (mutex_lock_interruptible(&dev->fileop_lock))
- return -ERESTARTSYS;
+ mutex_lock(&dev->lock);
if (dev->state & DEV_DISCONNECTED) {
em28xx_videodbg("mmap: device not present\n");
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -ENODEV;
}
if (dev->state & DEV_MISCONFIGURED) {
em28xx_videodbg ("mmap: Device is misconfigured; close and "
"open it again\n");
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -EIO;
}
if (dev->io != IO_MMAP || !(vma->vm_flags & VM_WRITE) ||
size != PAGE_ALIGN(dev->frame[0].buf.length)) {
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -EINVAL;
}
@@ -603,7 +593,7 @@
}
if (i == dev->num_frames) {
em28xx_videodbg("mmap: user supplied mapping address is out of range\n");
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -EINVAL;
}
@@ -615,7 +605,7 @@
while (size > 0) { /* size is page-aligned */
if (vm_insert_page(vma, start, vmalloc_to_page(pos))) {
em28xx_videodbg("mmap: vm_insert_page failed\n");
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return -EAGAIN;
}
start += PAGE_SIZE;
@@ -627,7 +617,7 @@
vma->vm_private_data = &dev->frame[i];
em28xx_vm_open(vma);
- mutex_unlock(&dev->fileop_lock);
+ mutex_unlock(&dev->lock);
return 0;
}
@@ -1084,7 +1074,9 @@
}
}
}
+ mutex_lock(&dev->lock);
em28xx_i2c_call_clients(dev,cmd,qc);
+ mutex_unlock(&dev->lock);
if (qc->type)
return 0;
else
@@ -1098,7 +1090,9 @@
if (!dev->has_msp34xx)
retval=em28xx_get_ctrl(dev, ctrl);
if (retval==-EINVAL) {
+ mutex_lock(&dev->lock);
em28xx_i2c_call_clients(dev,cmd,arg);
+ mutex_unlock(&dev->lock);
return 0;
} else return retval;
}
@@ -1106,21 +1100,26 @@
{
struct v4l2_control *ctrl = arg;
u8 i;
+ mutex_lock(&dev->lock);
if (!dev->has_msp34xx){
for (i = 0; i < ARRAY_SIZE(em28xx_qctrl); i++) {
if (ctrl->id == em28xx_qctrl[i].id) {
+ int retval=-EINVAL;
if (ctrl->value <
em28xx_qctrl[i].minimum
|| ctrl->value >
em28xx_qctrl[i].maximum)
return -ERANGE;
- return em28xx_set_ctrl(dev, ctrl);
+ retval = em28xx_set_ctrl(dev, ctrl);
+ mutex_unlock(&dev->lock);
+ return retval;
}
}
}
em28xx_i2c_call_clients(dev,cmd,arg);
+ mutex_unlock(&dev->lock);
return 0;
}
/* --- tuner ioctls ------------------------------------------ */
@@ -1220,12 +1219,16 @@
|| dev->io != IO_MMAP)
return -EINVAL;
+ mutex_lock(&dev->lock);
if (dev->stream == STREAM_ON) {
em28xx_videodbg ("VIDIOC_STREAMOFF: interrupting stream\n");
- if ((ret = em28xx_stream_interrupt(dev)))
+ if ((ret = em28xx_stream_interrupt(dev))){
+ mutex_unlock(&dev->lock);
return ret;
+ }
}
em28xx_empty_framequeues(dev);
+ mutex_unlock(&dev->lock);
return 0;
}
@@ -1291,11 +1294,23 @@
return 0;
}
case VIDIOC_G_FMT:
- return em28xx_get_fmt(dev, (struct v4l2_format *) arg);
+ {
+ int retval;
+ mutex_lock(&dev->lock);
+ retval = em28xx_get_fmt(dev, (struct v4l2_format *) arg);
+ mutex_unlock(&dev->lock);
+ return retval;
+ }
case VIDIOC_TRY_FMT:
case VIDIOC_S_FMT:
- return em28xx_set_fmt(dev, cmd, (struct v4l2_format *)arg);
+ {
+ int retval;
+ mutex_lock(&dev->lock);
+ retval = em28xx_set_fmt(dev, cmd, (struct v4l2_format *)arg);
+ mutex_unlock(&dev->lock);
+ return retval;
+ }
case VIDIOC_REQBUFS:
{
@@ -1320,10 +1335,13 @@
return -EINVAL;
}
+ mutex_lock(&dev->lock);
if (dev->stream == STREAM_ON) {
em28xx_videodbg("VIDIOC_REQBUFS: interrupting stream\n");
- if ((ret = em28xx_stream_interrupt(dev)))
+ if ((ret = em28xx_stream_interrupt(dev))){
+ mutex_unlock(&dev->lock);
return ret;
+ }
}
em28xx_empty_framequeues(dev);
@@ -1338,6 +1356,7 @@
em28xx_videodbg ("VIDIOC_REQBUFS: setting io method to mmap: num bufs %i\n",
rb->count);
dev->io = rb->count ? IO_MMAP : IO_NONE;
+ mutex_unlock(&dev->lock);
return 0;
}
case VIDIOC_QUERYBUF:
@@ -1439,26 +1458,19 @@
int ret = 0;
struct em28xx *dev = filp->private_data;
- if (mutex_lock_interruptible(&dev->fileop_lock))
- return -ERESTARTSYS;
-
if (dev->state & DEV_DISCONNECTED) {
em28xx_errdev("v4l2 ioctl: device not present\n");
- mutex_unlock(&dev->fileop_lock);
return -ENODEV;
}
if (dev->state & DEV_MISCONFIGURED) {
em28xx_errdev
("v4l2 ioctl: device is misconfigured; close and open it again\n");
- mutex_unlock(&dev->fileop_lock);
return -EIO;
}
ret = video_usercopy(inode, filp, cmd, arg, em28xx_video_do_ioctl);
- mutex_unlock(&dev->fileop_lock);
-
return ret;
}
@@ -1519,8 +1531,6 @@
return -ENOMEM;
}
- mutex_lock(&dev->lock);
-
/* register i2c bus */
em28xx_i2c_register(dev);
@@ -1530,8 +1540,6 @@
/* configure the device */
em28xx_config_i2c(dev);
- mutex_unlock(&dev->lock);
-
for (i = 0; i < TVNORMS; i++)
if (em28xx_boards[dev->model].norm == tvnorms[i].mode)
break;
@@ -1599,8 +1607,18 @@
list_add_tail(&dev->devlist,&em28xx_devlist);
+
+ if (dev->has_msp34xx) {
+ /* Send a reset to other chips via gpio */
+ em28xx_write_regs_req(dev, 0x00, 0x08, "\xf7", 1);
+ msleep(3);
+ em28xx_write_regs_req(dev, 0x00, 0x08, "\xff", 1);
+ msleep(3);
+
+ }
+ video_mux(dev, 0);
+
/* register v4l2 device */
- mutex_lock(&dev->lock);
if ((retval = video_register_device(dev->vdev, VFL_TYPE_GRABBER,
video_nr[dev->devno]))) {
em28xx_errdev("unable to register video device (error=%i).\n",
@@ -1627,18 +1645,6 @@
printk("registered VBI\n");
}
- if (dev->has_msp34xx) {
- /* Send a reset to other chips via gpio */
- em28xx_write_regs_req(dev, 0x00, 0x08, "\xf7", 1);
- msleep(3);
- em28xx_write_regs_req(dev, 0x00, 0x08, "\xff", 1);
- msleep(3);
-
- }
- video_mux(dev, 0);
-
- mutex_unlock(&dev->lock);
-
em28xx_info("V4L2 device registered as /dev/video%d and /dev/vbi%d\n",
dev->vdev->minor-MINOR_VFL_TYPE_GRABBER_MIN,
dev->vbi_dev->minor-MINOR_VFL_TYPE_VBI_MIN);
@@ -1762,18 +1768,19 @@
*/
static void em28xx_usb_disconnect(struct usb_interface *interface)
{
- struct em28xx *dev = usb_get_intfdata(interface);
+ struct em28xx *dev;
+
+ dev = usb_get_intfdata(interface);
usb_set_intfdata(interface, NULL);
if (!dev)
return;
- down_write(&em28xx_disconnect);
-
- mutex_lock(&dev->lock);
-
em28xx_info("disconnecting %s\n", dev->vdev->name);
+ /* wait until all current v4l2 io is finished then deallocate resources */
+ mutex_lock(&dev->lock);
+
wake_up_interruptible_all(&dev->open);
if (dev->users) {
@@ -1792,6 +1799,7 @@
em28xx_release_resources(dev);
}
+
mutex_unlock(&dev->lock);
if (!dev->users) {
@@ -1799,7 +1807,6 @@
kfree(dev);
}
- up_write(&em28xx_disconnect);
}
static struct usb_driver em28xx_usb_driver = {