From 178b44f32ea82b30a0bccf75ecc47a28a08dbf96 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Sun, 26 Sep 2010 08:47:38 -0300 Subject: [PATCH] --- yaml --- r: 219194 b: refs/heads/master c: ee6869afc922a9849979e49bb3bbcad794872fcb h: refs/heads/master v: v3 --- [refs] | 2 +- .../video4linux/v4l2-framework.txt | 20 +++++ trunk/drivers/media/video/v4l2-dev.c | 76 ++++++++++++++----- trunk/drivers/media/video/v4l2-event.c | 9 ++- trunk/include/media/v4l2-dev.h | 3 + 5 files changed, 90 insertions(+), 20 deletions(-) diff --git a/[refs] b/[refs] index 23c0e797838d..89994ab829e5 100644 --- a/[refs] +++ b/[refs] @@ -1,2 +1,2 @@ --- -refs/heads/master: c29fcff3daafbf46d64a543c1950bbd206ad8c1c +refs/heads/master: ee6869afc922a9849979e49bb3bbcad794872fcb diff --git a/trunk/Documentation/video4linux/v4l2-framework.txt b/trunk/Documentation/video4linux/v4l2-framework.txt index 9b1d81c26b7d..a128e012a45c 100644 --- a/trunk/Documentation/video4linux/v4l2-framework.txt +++ b/trunk/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/trunk/drivers/media/video/v4l2-dev.c b/trunk/drivers/media/video/v4l2-dev.c index 5a54eabd4c42..a7702e3d149c 100644 --- a/trunk/drivers/media/video/v4l2-dev.c +++ b/trunk/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/trunk/drivers/media/video/v4l2-event.c b/trunk/drivers/media/video/v4l2-event.c index de74ce07b5e2..69fd343d4774 100644 --- a/trunk/drivers/media/video/v4l2-event.c +++ b/trunk/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/trunk/include/media/v4l2-dev.h b/trunk/include/media/v4l2-dev.h index ba236ff35c8a..15802a067a12 100644 --- a/trunk/include/media/v4l2-dev.h +++ b/trunk/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 */