From: Hans Verkuil Date: Sun, 26 Sep 2010 11:47:38 +0000 (-0300) Subject: V4L/DVB: v4l2: add core serialization lock X-Git-Tag: v4.13~17041^2~345 X-Git-Url: https://git.proxmox.com/?a=commitdiff_plain;h=ee6869afc922a9849979e49bb3bbcad794872fcb;p=mirror_ubuntu-bionic-kernel.git 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 Signed-off-by: Mauro Carvalho Chehab --- 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 */