V4L/DVB: v4l2: add core serialization lock

Drivers can optionally set a pointer to a mutex in struct video_device.
The core will use that to lock before calling open, read, write, unlocked_ioctl,
poll, mmap or release.

Updated the documentation as well and ensure that v4l2-event knows about the
lock: it will unlock it before doing a blocking wait on an event and relock it
afterwards.

Ensure that the 'video_is_registered' check is done when the lock is held:
a typical disconnect will take the lock as well before unregistering the
device nodes, so to prevent race conditions the video_is_registered check
should also be done with the lock held.

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

4 files changed, 89 insertions, 19 deletions

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 9b1d81c26b7d..a128e012a45c 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -453,6 +453,10 @@ You should also set these fields:
 - ioctl_ops: if you use the v4l2_ioctl_ops to simplify ioctl maintenance
   (highly recommended to use this and it might become compulsory in the
   future!), then set this to your v4l2_ioctl_ops struct.
+- lock: leave to NULL if you want to do all the locking in the driver.
+  Otherwise you give it a pointer to a struct mutex_lock and before any
+  of the v4l2_file_operations is called this lock will be taken by the
+  core and released afterwards.
 - parent: you only set this if v4l2_device was registered with NULL as
   the parent device struct. This only happens in cases where one hardware
   device has multiple PCI devices that all share the same v4l2_device core.
@@ -469,6 +473,22 @@ If you use v4l2_ioctl_ops, then you should set either .unlocked_ioctl or
 The v4l2_file_operations struct is a subset of file_operations. The main
 difference is that the inode argument is omitted since it is never used.
 
+v4l2_file_operations and locking
+--------------------------------
+
+You can set a pointer to a mutex_lock in struct video_device. Usually this
+will be either a top-level mutex or a mutex per device node. If you want
+finer-grained locking then you have to set it to NULL and do you own locking.
+
+If a lock is specified then all file operations will be serialized on that
+lock. If you use videobuf then you must pass the same lock to the videobuf
+queue initialize function: if videobuf has to wait for a frame to arrive, then
+it will temporarily unlock the lock and relock it afterwards. If your driver
+also waits in the code, then you should do the same to allow other processes
+to access the device node while the first process is waiting for something.
+
+The implementation of a hotplug disconnect should also take the lock before
+calling v4l2_device_disconnect and video_unregister_device.
 
 video_device registration
 -------------------------
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 5a54eabd4c42..a7702e3d149c 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -187,33 +187,50 @@ static ssize_t v4l2_read(struct file *filp, char __user *buf,
                 size_t sz, loff_t *off)
 {
         struct video_device *vdev = video_devdata(filp);
+        int ret = -EIO;
 
         if (!vdev->fops->read)
                 return -EINVAL;
-        if (!video_is_registered(vdev))
-                return -EIO;
-        return vdev->fops->read(filp, buf, sz, off);
+        if (vdev->lock)
+                mutex_lock(vdev->lock);
+        if (video_is_registered(vdev))
+                ret = vdev->fops->read(filp, buf, sz, off);
+        if (vdev->lock)
+                mutex_unlock(vdev->lock);
+        return ret;
 }
 
 static ssize_t v4l2_write(struct file *filp, const char __user *buf,
                 size_t sz, loff_t *off)
 {
         struct video_device *vdev = video_devdata(filp);
+        int ret = -EIO;
 
         if (!vdev->fops->write)
                 return -EINVAL;
-        if (!video_is_registered(vdev))
-                return -EIO;
-        return vdev->fops->write(filp, buf, sz, off);
+        if (vdev->lock)
+                mutex_lock(vdev->lock);
+        if (video_is_registered(vdev))
+                ret = vdev->fops->write(filp, buf, sz, off);
+        if (vdev->lock)
+                mutex_unlock(vdev->lock);
+        return ret;
 }
 
 static unsigned int v4l2_poll(struct file *filp, struct poll_table_struct *poll)
 {
         struct video_device *vdev = video_devdata(filp);
+        int ret = DEFAULT_POLLMASK;
 
-        if (!vdev->fops->poll || !video_is_registered(vdev))
-                return DEFAULT_POLLMASK;
-        return vdev->fops->poll(filp, poll);
+        if (!vdev->fops->poll)
+                return ret;
+        if (vdev->lock)
+                mutex_lock(vdev->lock);
+        if (video_is_registered(vdev))
+                ret = vdev->fops->poll(filp, poll);
+        if (vdev->lock)
+                mutex_unlock(vdev->lock);
+        return ret;
 }
 
 static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
@@ -224,7 +241,11 @@ static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
         if (!vdev->fops->ioctl)
                 return -ENOTTY;
         if (vdev->fops->unlocked_ioctl) {
+                if (vdev->lock)
+                        mutex_lock(vdev->lock);
                 ret = vdev->fops->unlocked_ioctl(filp, cmd, arg);
+                if (vdev->lock)
+                        mutex_unlock(vdev->lock);
         } else if (vdev->fops->ioctl) {
                 /* TODO: convert all drivers to unlocked_ioctl */
                 lock_kernel();
@@ -239,10 +260,17 @@ static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 static int v4l2_mmap(struct file *filp, struct vm_area_struct *vm)
 {
         struct video_device *vdev = video_devdata(filp);
+        int ret = -ENODEV;
 
-        if (!vdev->fops->mmap || !video_is_registered(vdev))
-                return -ENODEV;
-        return vdev->fops->mmap(filp, vm);
+        if (!vdev->fops->mmap)
+                return ret;
+        if (vdev->lock)
+                mutex_lock(vdev->lock);
+        if (video_is_registered(vdev))
+                ret = vdev->fops->mmap(filp, vm);
+        if (vdev->lock)
+                mutex_unlock(vdev->lock);
+        return ret;
 }
 
 /* Override for the open function */
@@ -254,17 +282,24 @@ static int v4l2_open(struct inode *inode, struct file *filp)
         /* Check if the video device is available */
         mutex_lock(&videodev_lock);
         vdev = video_devdata(filp);
-        /* return ENODEV if the video device has been removed
-           already or if it is not registered anymore. */
-        if (vdev == NULL || !video_is_registered(vdev)) {
+        /* return ENODEV if the video device has already been removed. */
+        if (vdev == NULL) {
                 mutex_unlock(&videodev_lock);
                 return -ENODEV;
         }
         /* and increase the device refcount */
         video_get(vdev);
         mutex_unlock(&videodev_lock);
-        if (vdev->fops->open)
-                ret = vdev->fops->open(filp);
+        if (vdev->fops->open) {
+                if (vdev->lock)
+                        mutex_lock(vdev->lock);
+                if (video_is_registered(vdev))
+                        ret = vdev->fops->open(filp);
+                else
+                        ret = -ENODEV;
+                if (vdev->lock)
+                        mutex_unlock(vdev->lock);
+        }
 
         /* decrease the refcount in case of an error */
         if (ret)
@@ -278,8 +313,13 @@ static int v4l2_release(struct inode *inode, struct file *filp)
         struct video_device *vdev = video_devdata(filp);
         int ret = 0;
 
-        if (vdev->fops->release)
+        if (vdev->fops->release) {
+                if (vdev->lock)
+                        mutex_lock(vdev->lock);
                 vdev->fops->release(filp);
+                if (vdev->lock)
+                        mutex_unlock(vdev->lock);
+        }
 
         /* decrease the refcount unconditionally since the release()
            return value is ignored. */
diff --git a/drivers/media/video/v4l2-event.c b/drivers/media/video/v4l2-event.c
index de74ce07b5e2..69fd343d4774 100644
--- a/drivers/media/video/v4l2-event.c
+++ b/drivers/media/video/v4l2-event.c
@@ -134,15 +134,22 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event,
         if (nonblocking)
                 return __v4l2_event_dequeue(fh, event);
 
+        /* Release the vdev lock while waiting */
+        if (fh->vdev->lock)
+                mutex_unlock(fh->vdev->lock);
+
         do {
                 ret = wait_event_interruptible(events->wait,
                                                events->navailable != 0);
                 if (ret < 0)
-                        return ret;
+                        break;
 
                 ret = __v4l2_event_dequeue(fh, event);
         } while (ret == -ENOENT);
 
+        if (fh->vdev->lock)
+                mutex_lock(fh->vdev->lock);
+
         return ret;
 }
 EXPORT_SYMBOL_GPL(v4l2_event_dequeue);
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index ba236ff35c8a..15802a067a12 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -94,6 +94,9 @@ struct video_device
 
         /* ioctl callbacks */
         const struct v4l2_ioctl_ops *ioctl_ops;
+
+        /* serialization lock */
+        struct mutex *lock;
 };
 
 /* dev to video-device */