All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 00/26] Remove the V4L2_FL_LOCK_ALL_FOPS flag
@ 2012-06-24 11:25 Hans Verkuil
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
  0 siblings, 1 reply; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:25 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov

Hi all,

In the 3.5 kernel a change was made regarding core locking (i.e. how locking
is done if the 'lock' field is set in struct video_device). Before kernel 3.5
all file operations would be locked in that case. But this turned out to be
problematic, adding unnecessary latencies in some cases (poll) and potentially
introducing a deadlock in mmap: the kernel takes the mm semaphore before calling
the driver's file operation, where the core lock is taken, whereas other file
ops take the core lock first, and in case of a page fault the mm semaphore
will be taken. This scenario is very unlikely, but the lockdep checker will
complain about it.

So in kernel 3.5 we decided not to take the core lock anymore for file
operations other than unlocked_ioctl. Instead the driver will have to do
the locking. As a temporary measure the flag V4L2_FL_LOCK_ALL_FOPS was
introduced that, if set, would force the core to take the lock anyway for
all file operations. In other words, drivers that were not converted yet
to do their own locking for non-unlocked_ioctl file operations would just
set this flag and keep the old behavior.

This patch series goes through all those drivers and pushed the locking
down into the driver and removes this legacy flag.

These patches just push the locking down and do not do anything smart (except
for some additional dead code removal in saa7146, or if I was 100% certain no
locking was needed for a particular file operation). In particular for mmap
it will still take the core lock, it just does it in the driver now, making it
easier to change in a future patch.

I have already tested ivtv, saa7146, cpia2, usbvision, em28xx, tm6000,
mem2mem_testdev and cx231xx. I hope to test vpif_capture/display tomorrow.

The others need to be tested and/or reviewed by others (i.e. you!).

I'd really want to get rid of this flag as soon as possible as it makes the
v4l2 core lock handling unnecessarily complex, and it also complicates the
core and vb2 enhancements patch series I am working on:

http://www.spinics.net/lists/linux-media/msg49299.html

So please take a look, test if you can, and let me know if it is OK.

Regards,

	Hans


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 [RFC PATCH 00/26] Remove the V4L2_FL_LOCK_ALL_FOPS flag Hans Verkuil
@ 2012-06-24 11:25 ` Hans Verkuil
  2012-06-24 11:25   ` [RFC PATCH 02/26] saa7146: " Hans Verkuil
                     ` (24 more replies)
  0 siblings, 25 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:25 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/ivtv/ivtv-fileops.c |   52 +++++++++++++++++++++++++------
 drivers/media/video/ivtv/ivtv-streams.c |    4 ---
 2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/drivers/media/video/ivtv/ivtv-fileops.c b/drivers/media/video/ivtv/ivtv-fileops.c
index 9ff69b5..88bce90 100644
--- a/drivers/media/video/ivtv/ivtv-fileops.c
+++ b/drivers/media/video/ivtv/ivtv-fileops.c
@@ -505,14 +505,17 @@ ssize_t ivtv_v4l2_read(struct file * filp, char __user *buf, size_t count, loff_
 	struct ivtv_open_id *id = fh2id(filp->private_data);
 	struct ivtv *itv = id->itv;
 	struct ivtv_stream *s = &itv->streams[id->type];
-	int rc;
+	ssize_t rc;
 
 	IVTV_DEBUG_HI_FILE("read %zd bytes from %s\n", count, s->name);
 
+	if (mutex_lock_interruptible(&itv->serialize_lock))
+		return -ERESTARTSYS;
 	rc = ivtv_start_capture(id);
-	if (rc)
-		return rc;
-	return ivtv_read_pos(s, buf, count, pos, filp->f_flags & O_NONBLOCK);
+	if (!rc)
+		rc = ivtv_read_pos(s, buf, count, pos, filp->f_flags & O_NONBLOCK);
+	mutex_unlock(&itv->serialize_lock);
+	return rc;
 }
 
 int ivtv_start_decoding(struct ivtv_open_id *id, int speed)
@@ -540,7 +543,7 @@ int ivtv_start_decoding(struct ivtv_open_id *id, int speed)
 	return 0;
 }
 
-ssize_t ivtv_v4l2_write(struct file *filp, const char __user *user_buf, size_t count, loff_t *pos)
+static ssize_t ivtv_write(struct file *filp, const char __user *user_buf, size_t count, loff_t *pos)
 {
 	struct ivtv_open_id *id = fh2id(filp->private_data);
 	struct ivtv *itv = id->itv;
@@ -712,6 +715,19 @@ retry:
 	return bytes_written;
 }
 
+ssize_t ivtv_v4l2_write(struct file *filp, const char __user *user_buf, size_t count, loff_t *pos)
+{
+	struct ivtv_open_id *id = fh2id(filp->private_data);
+	struct ivtv *itv = id->itv;
+	ssize_t res;
+
+	if (mutex_lock_interruptible(&itv->serialize_lock))
+		return -ERESTARTSYS;
+	res = ivtv_write(filp, user_buf, count, pos);
+	mutex_unlock(&itv->serialize_lock);
+	return res;
+}
+
 unsigned int ivtv_v4l2_dec_poll(struct file *filp, poll_table *wait)
 {
 	struct ivtv_open_id *id = fh2id(filp->private_data);
@@ -760,7 +776,9 @@ unsigned int ivtv_v4l2_enc_poll(struct file *filp, poll_table *wait)
 			(req_events & (POLLIN | POLLRDNORM))) {
 		int rc;
 
+		mutex_lock(&itv->serialize_lock);
 		rc = ivtv_start_capture(id);
+		mutex_unlock(&itv->serialize_lock);
 		if (rc) {
 			IVTV_DEBUG_INFO("Could not start capture for %s (%d)\n",
 					s->name, rc);
@@ -863,6 +881,8 @@ int ivtv_v4l2_close(struct file *filp)
 
 	IVTV_DEBUG_FILE("close %s\n", s->name);
 
+	mutex_lock(&itv->serialize_lock);
+
 	/* Stop radio */
 	if (id->type == IVTV_ENC_STREAM_TYPE_RAD &&
 			v4l2_fh_is_singular_file(filp)) {
@@ -892,10 +912,8 @@ int ivtv_v4l2_close(struct file *filp)
 	v4l2_fh_exit(fh);
 
 	/* Easy case first: this stream was never claimed by us */
-	if (s->fh != &id->fh) {
-		kfree(id);
-		return 0;
-	}
+	if (s->fh != &id->fh)
+		goto close_done;
 
 	/* 'Unclaim' this stream */
 
@@ -913,11 +931,13 @@ int ivtv_v4l2_close(struct file *filp)
 	} else {
 		ivtv_stop_capture(id, 0);
 	}
+close_done:
 	kfree(id);
+	mutex_unlock(&itv->serialize_lock);
 	return 0;
 }
 
-int ivtv_v4l2_open(struct file *filp)
+static int ivtv_open(struct file *filp)
 {
 	struct video_device *vdev = video_devdata(filp);
 	struct ivtv_stream *s = video_get_drvdata(vdev);
@@ -1020,6 +1040,18 @@ int ivtv_v4l2_open(struct file *filp)
 	return 0;
 }
 
+int ivtv_v4l2_open(struct file *filp)
+{
+	struct video_device *vdev = video_devdata(filp);
+	int res;
+
+	if (mutex_lock_interruptible(vdev->lock))
+		return -ERESTARTSYS;
+	res = ivtv_open(filp);
+	mutex_unlock(vdev->lock);
+	return res;
+}
+
 void ivtv_mute(struct ivtv *itv)
 {
 	if (atomic_read(&itv->capturing))
diff --git a/drivers/media/video/ivtv/ivtv-streams.c b/drivers/media/video/ivtv/ivtv-streams.c
index 6738592..7ea5ca7 100644
--- a/drivers/media/video/ivtv/ivtv-streams.c
+++ b/drivers/media/video/ivtv/ivtv-streams.c
@@ -228,10 +228,6 @@ static int ivtv_prep_dev(struct ivtv *itv, int type)
 	s->vdev->release = video_device_release;
 	s->vdev->tvnorms = V4L2_STD_ALL;
 	s->vdev->lock = &itv->serialize_lock;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &s->vdev->flags);
 	set_bit(V4L2_FL_USE_FH_PRIO, &s->vdev->flags);
 	ivtv_set_funcs(s->vdev);
 	return 0;
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 02/26] saa7146: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
@ 2012-06-24 11:25   ` Hans Verkuil
  2012-06-24 11:25   ` [RFC PATCH 03/26] cpia2: " Hans Verkuil
                     ` (23 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:25 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

I also removed some dead code in the form of the saa7146_devices list and
saa7146_devices_lock mutex: these were used once but that was a long time
ago.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/common/saa7146_core.c |    8 -----
 drivers/media/common/saa7146_fops.c |   55 +++++++++++++++++++++++++----------
 include/media/saa7146.h             |    4 ---
 3 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/drivers/media/common/saa7146_core.c b/drivers/media/common/saa7146_core.c
index d6b1cf6..bb6ee51 100644
--- a/drivers/media/common/saa7146_core.c
+++ b/drivers/media/common/saa7146_core.c
@@ -23,9 +23,6 @@
 #include <media/saa7146.h>
 #include <linux/module.h>
 
-LIST_HEAD(saa7146_devices);
-DEFINE_MUTEX(saa7146_devices_lock);
-
 static int saa7146_num;
 
 unsigned int saa7146_debug;
@@ -482,8 +479,6 @@ static int saa7146_init_one(struct pci_dev *pci, const struct pci_device_id *ent
 	   set it explicitly. */
 	pci_set_drvdata(pci, &dev->v4l2_dev);
 
-	INIT_LIST_HEAD(&dev->item);
-	list_add_tail(&dev->item,&saa7146_devices);
 	saa7146_num++;
 
 	err = 0;
@@ -545,7 +540,6 @@ static void saa7146_remove_one(struct pci_dev *pdev)
 
 	iounmap(dev->mem);
 	pci_release_region(pdev, 0);
-	list_del(&dev->item);
 	pci_disable_device(pdev);
 	kfree(dev);
 
@@ -592,8 +586,6 @@ EXPORT_SYMBOL_GPL(saa7146_setgpio);
 EXPORT_SYMBOL_GPL(saa7146_i2c_adapter_prepare);
 
 EXPORT_SYMBOL_GPL(saa7146_debug);
-EXPORT_SYMBOL_GPL(saa7146_devices);
-EXPORT_SYMBOL_GPL(saa7146_devices_lock);
 
 MODULE_AUTHOR("Michael Hunold <michael@mihu.de>");
 MODULE_DESCRIPTION("driver for generic saa7146-based hardware");
diff --git a/drivers/media/common/saa7146_fops.c b/drivers/media/common/saa7146_fops.c
index 0cdbd74..b3890bd 100644
--- a/drivers/media/common/saa7146_fops.c
+++ b/drivers/media/common/saa7146_fops.c
@@ -201,7 +201,7 @@ static int fops_open(struct file *file)
 
 	DEB_EE("file:%p, dev:%s\n", file, video_device_node_name(vdev));
 
-	if (mutex_lock_interruptible(&saa7146_devices_lock))
+	if (mutex_lock_interruptible(vdev->lock))
 		return -ERESTARTSYS;
 
 	DEB_D("using: %p\n", dev);
@@ -253,7 +253,7 @@ out:
 		kfree(fh);
 		file->private_data = NULL;
 	}
-	mutex_unlock(&saa7146_devices_lock);
+	mutex_unlock(vdev->lock);
 	return result;
 }
 
@@ -265,7 +265,7 @@ static int fops_release(struct file *file)
 
 	DEB_EE("file:%p\n", file);
 
-	if (mutex_lock_interruptible(&saa7146_devices_lock))
+	if (mutex_lock_interruptible(vdev->lock))
 		return -ERESTARTSYS;
 
 	if (vdev->vfl_type == VFL_TYPE_VBI) {
@@ -283,7 +283,7 @@ static int fops_release(struct file *file)
 	file->private_data = NULL;
 	kfree(fh);
 
-	mutex_unlock(&saa7146_devices_lock);
+	mutex_unlock(vdev->lock);
 
 	return 0;
 }
@@ -293,6 +293,7 @@ static int fops_mmap(struct file *file, struct vm_area_struct * vma)
 	struct video_device *vdev = video_devdata(file);
 	struct saa7146_fh *fh = file->private_data;
 	struct videobuf_queue *q;
+	int res;
 
 	switch (vdev->vfl_type) {
 	case VFL_TYPE_GRABBER: {
@@ -314,10 +315,14 @@ static int fops_mmap(struct file *file, struct vm_area_struct * vma)
 		return 0;
 	}
 
-	return videobuf_mmap_mapper(q,vma);
+	if (mutex_lock_interruptible(vdev->lock))
+		return -ERESTARTSYS;
+	res = videobuf_mmap_mapper(q, vma);
+	mutex_unlock(vdev->lock);
+	return res;
 }
 
-static unsigned int fops_poll(struct file *file, struct poll_table_struct *wait)
+static unsigned int __fops_poll(struct file *file, struct poll_table_struct *wait)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct saa7146_fh *fh = file->private_data;
@@ -356,10 +361,22 @@ static unsigned int fops_poll(struct file *file, struct poll_table_struct *wait)
 	return res;
 }
 
+static unsigned int fops_poll(struct file *file, struct poll_table_struct *wait)
+{
+	struct video_device *vdev = video_devdata(file);
+	unsigned int res;
+
+	mutex_lock(vdev->lock);
+	res = __fops_poll(file, wait);
+	mutex_unlock(vdev->lock);
+	return res;
+}
+
 static ssize_t fops_read(struct file *file, char __user *data, size_t count, loff_t *ppos)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct saa7146_fh *fh = file->private_data;
+	int ret;
 
 	switch (vdev->vfl_type) {
 	case VFL_TYPE_GRABBER:
@@ -373,8 +390,13 @@ static ssize_t fops_read(struct file *file, char __user *data, size_t count, lof
 		DEB_EE("V4L2_BUF_TYPE_VBI_CAPTURE: file:%p, data:%p, count:%lu\n",
 		       file, data, (unsigned long)count);
 */
-		if (fh->dev->ext_vv_data->capabilities & V4L2_CAP_VBI_CAPTURE)
-			return saa7146_vbi_uops.read(file,data,count,ppos);
+		if (fh->dev->ext_vv_data->capabilities & V4L2_CAP_VBI_CAPTURE) {
+			if (mutex_lock_interruptible(vdev->lock))
+				return -ERESTARTSYS;
+			ret = saa7146_vbi_uops.read(file, data, count, ppos);
+			mutex_unlock(vdev->lock);
+			return ret;
+		}
 		return -EINVAL;
 	default:
 		BUG();
@@ -386,15 +408,20 @@ static ssize_t fops_write(struct file *file, const char __user *data, size_t cou
 {
 	struct video_device *vdev = video_devdata(file);
 	struct saa7146_fh *fh = file->private_data;
+	int ret;
 
 	switch (vdev->vfl_type) {
 	case VFL_TYPE_GRABBER:
 		return -EINVAL;
 	case VFL_TYPE_VBI:
-		if (fh->dev->ext_vv_data->vbi_fops.write)
-			return fh->dev->ext_vv_data->vbi_fops.write(file, data, count, ppos);
-		else
-			return -EINVAL;
+		if (fh->dev->ext_vv_data->vbi_fops.write) {
+			if (mutex_lock_interruptible(vdev->lock))
+				return -ERESTARTSYS;
+			ret = fh->dev->ext_vv_data->vbi_fops.write(file, data, count, ppos);
+			mutex_unlock(vdev->lock);
+			return ret;
+		}
+		return -EINVAL;
 	default:
 		BUG();
 		return -EINVAL;
@@ -584,10 +611,6 @@ int saa7146_register_device(struct video_device **vid, struct saa7146_dev* dev,
 	else
 		vfd->ioctl_ops = &dev->ext_vv_data->vbi_ops;
 	vfd->release = video_device_release;
-	/* Locking in file operations other than ioctl should be done by
-	   the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 	vfd->lock = &dev->v4l2_lock;
 	vfd->v4l2_dev = &dev->v4l2_dev;
 	vfd->tvnorms = 0;
diff --git a/include/media/saa7146.h b/include/media/saa7146.h
index 773e527..96058a5 100644
--- a/include/media/saa7146.h
+++ b/include/media/saa7146.h
@@ -117,8 +117,6 @@ struct saa7146_dev
 {
 	struct module			*module;
 
-	struct list_head		item;
-
 	struct v4l2_device 		v4l2_dev;
 	struct v4l2_ctrl_handler	ctrl_handler;
 
@@ -166,8 +164,6 @@ static inline struct saa7146_dev *to_saa7146_dev(struct v4l2_device *v4l2_dev)
 int saa7146_i2c_adapter_prepare(struct saa7146_dev *dev, struct i2c_adapter *i2c_adapter, u32 bitrate);
 
 /* from saa7146_core.c */
-extern struct list_head saa7146_devices;
-extern struct mutex saa7146_devices_lock;
 int saa7146_register_extension(struct saa7146_extension*);
 int saa7146_unregister_extension(struct saa7146_extension*);
 struct saa7146_format* saa7146_format_by_fourcc(struct saa7146_dev *dev, int fourcc);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 03/26] cpia2: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
  2012-06-24 11:25   ` [RFC PATCH 02/26] saa7146: " Hans Verkuil
@ 2012-06-24 11:25   ` Hans Verkuil
  2012-06-24 11:25   ` [RFC PATCH 04/26] usbvision: " Hans Verkuil
                     ` (22 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:25 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/cpia2/cpia2_v4l.c |   39 +++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/media/video/cpia2/cpia2_v4l.c b/drivers/media/video/cpia2/cpia2_v4l.c
index 55e9290..8c18cb1 100644
--- a/drivers/media/video/cpia2/cpia2_v4l.c
+++ b/drivers/media/video/cpia2/cpia2_v4l.c
@@ -84,21 +84,26 @@ MODULE_VERSION(CPIA_VERSION);
 static int cpia2_open(struct file *file)
 {
 	struct camera_data *cam = video_drvdata(file);
-	int retval = v4l2_fh_open(file);
+	int retval;
 
+	if (mutex_lock_interruptible(&cam->v4l2_lock))
+		return -ERESTARTSYS;
+	retval = v4l2_fh_open(file);
 	if (retval)
-		return retval;
+		goto open_unlock;
 
 	if (v4l2_fh_is_singular_file(file)) {
 		if (cpia2_allocate_buffers(cam)) {
 			v4l2_fh_release(file);
-			return -ENOMEM;
+			retval = -ENOMEM;
+			goto open_unlock;
 		}
 
 		/* reset the camera */
 		if (cpia2_reset_camera(cam) < 0) {
 			v4l2_fh_release(file);
-			return -EIO;
+			retval = -EIO;
+			goto open_unlock;
 		}
 
 		cam->APP_len = 0;
@@ -106,7 +111,9 @@ static int cpia2_open(struct file *file)
 	}
 
 	cpia2_dbg_dump_registers(cam);
-	return 0;
+open_unlock:
+	mutex_unlock(&cam->v4l2_lock);
+	return retval;
 }
 
 /******************************************************************************
@@ -119,6 +126,7 @@ static int cpia2_close(struct file *file)
 	struct video_device *dev = video_devdata(file);
 	struct camera_data *cam = video_get_drvdata(dev);
 
+	mutex_lock(&cam->v4l2_lock);
 	if (video_is_registered(&cam->vdev) && v4l2_fh_is_singular_file(file)) {
 		cpia2_usb_stream_stop(cam);
 
@@ -133,6 +141,7 @@ static int cpia2_close(struct file *file)
 		cam->stream_fh = NULL;
 		cam->mmapped = 0;
 	}
+	mutex_unlock(&cam->v4l2_lock);
 	return v4l2_fh_release(file);
 }
 
@@ -146,11 +155,16 @@ static ssize_t cpia2_v4l_read(struct file *file, char __user *buf, size_t count,
 {
 	struct camera_data *cam = video_drvdata(file);
 	int noblock = file->f_flags&O_NONBLOCK;
+	ssize_t ret;
 
 	if(!cam)
 		return -EINVAL;
 
-	return cpia2_read(cam, buf, count, noblock);
+	if (mutex_lock_interruptible(&cam->v4l2_lock))
+		return -ERESTARTSYS;
+	ret = cpia2_read(cam, buf, count, noblock);
+	mutex_unlock(&cam->v4l2_lock);
+	return ret;
 }
 
 
@@ -162,8 +176,12 @@ static ssize_t cpia2_v4l_read(struct file *file, char __user *buf, size_t count,
 static unsigned int cpia2_v4l_poll(struct file *filp, struct poll_table_struct *wait)
 {
 	struct camera_data *cam = video_drvdata(filp);
+	unsigned int res;
 
-	return cpia2_poll(cam, filp, wait);
+	mutex_lock(&cam->v4l2_lock);
+	res = cpia2_poll(cam, filp, wait);
+	mutex_unlock(&cam->v4l2_lock);
+	return res;
 }
 
 
@@ -987,10 +1005,13 @@ static int cpia2_mmap(struct file *file, struct vm_area_struct *area)
 	struct camera_data *cam = video_drvdata(file);
 	int retval;
 
+	if (mutex_lock_interruptible(&cam->v4l2_lock))
+		return -ERESTARTSYS;
 	retval = cpia2_remap_buffer(cam, area);
 
 	if(!retval)
 		cam->stream_fh = file->private_data;
+	mutex_unlock(&cam->v4l2_lock);
 	return retval;
 }
 
@@ -1147,10 +1168,6 @@ int cpia2_register_camera(struct camera_data *cam)
 	cam->vdev.ctrl_handler = hdl;
 	cam->vdev.v4l2_dev = &cam->v4l2_dev;
 	set_bit(V4L2_FL_USE_FH_PRIO, &cam->vdev.flags);
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &cam->vdev.flags);
 
 	reset_camera_struct_v4l(cam);
 
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 04/26] usbvision: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
  2012-06-24 11:25   ` [RFC PATCH 02/26] saa7146: " Hans Verkuil
  2012-06-24 11:25   ` [RFC PATCH 03/26] cpia2: " Hans Verkuil
@ 2012-06-24 11:25   ` Hans Verkuil
  2012-06-24 11:25   ` [RFC PATCH 05/26] em28xx: " Hans Verkuil
                     ` (21 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:25 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/usbvision/usbvision-video.c |   42 +++++++++++++++++++----
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/usbvision/usbvision-video.c b/drivers/media/video/usbvision/usbvision-video.c
index 9bd8f08..8a43179 100644
--- a/drivers/media/video/usbvision/usbvision-video.c
+++ b/drivers/media/video/usbvision/usbvision-video.c
@@ -349,6 +349,8 @@ static int usbvision_v4l2_open(struct file *file)
 
 	PDEBUG(DBG_IO, "open");
 
+	if (mutex_lock_interruptible(&usbvision->v4l2_lock))
+		return -ERESTARTSYS;
 	usbvision_reset_power_off_timer(usbvision);
 
 	if (usbvision->user)
@@ -402,6 +404,7 @@ static int usbvision_v4l2_open(struct file *file)
 
 	/* prepare queues */
 	usbvision_empty_framequeues(usbvision);
+	mutex_unlock(&usbvision->v4l2_lock);
 
 	PDEBUG(DBG_IO, "success");
 	return err_code;
@@ -421,6 +424,7 @@ static int usbvision_v4l2_close(struct file *file)
 
 	PDEBUG(DBG_IO, "close");
 
+	mutex_lock(&usbvision->v4l2_lock);
 	usbvision_audio_off(usbvision);
 	usbvision_restart_isoc(usbvision);
 	usbvision_stop_isoc(usbvision);
@@ -443,6 +447,7 @@ static int usbvision_v4l2_close(struct file *file)
 		printk(KERN_INFO "%s: Final disconnect\n", __func__);
 		usbvision_release(usbvision);
 	}
+	mutex_unlock(&usbvision->v4l2_lock);
 
 	PDEBUG(DBG_IO, "success");
 	return 0;
@@ -956,7 +961,7 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
-static ssize_t usbvision_v4l2_read(struct file *file, char __user *buf,
+static ssize_t usbvision_read(struct file *file, char __user *buf,
 		      size_t count, loff_t *ppos)
 {
 	struct usb_usbvision *usbvision = video_drvdata(file);
@@ -1060,7 +1065,20 @@ static ssize_t usbvision_v4l2_read(struct file *file, char __user *buf,
 	return count;
 }
 
-static int usbvision_v4l2_mmap(struct file *file, struct vm_area_struct *vma)
+static ssize_t usbvision_v4l2_read(struct file *file, char __user *buf,
+		      size_t count, loff_t *ppos)
+{
+	struct usb_usbvision *usbvision = video_drvdata(file);
+	int res;
+
+	if (mutex_lock_interruptible(&usbvision->v4l2_lock))
+		return -ERESTARTSYS;
+	res = usbvision_read(file, buf, count, ppos);
+	mutex_unlock(&usbvision->v4l2_lock);
+	return res;
+}
+
+static int usbvision_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	unsigned long size = vma->vm_end - vma->vm_start,
 		start = vma->vm_start;
@@ -1107,6 +1125,17 @@ static int usbvision_v4l2_mmap(struct file *file, struct vm_area_struct *vma)
 	return 0;
 }
 
+static int usbvision_v4l2_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	struct usb_usbvision *usbvision = video_drvdata(file);
+	int res;
+
+	if (mutex_lock_interruptible(&usbvision->v4l2_lock))
+		return -ERESTARTSYS;
+	res = usbvision_mmap(file, vma);
+	mutex_unlock(&usbvision->v4l2_lock);
+	return res;
+}
 
 /*
  * Here comes the stuff for radio on usbvision based devices
@@ -1119,6 +1148,8 @@ static int usbvision_radio_open(struct file *file)
 
 	PDEBUG(DBG_IO, "%s:", __func__);
 
+	if (mutex_lock_interruptible(&usbvision->v4l2_lock))
+		return -ERESTARTSYS;
 	if (usbvision->user) {
 		dev_err(&usbvision->rdev->dev,
 			"%s: Someone tried to open an already opened USBVision Radio!\n",
@@ -1156,6 +1187,7 @@ static int usbvision_radio_open(struct file *file)
 		}
 	}
 out:
+	mutex_unlock(&usbvision->v4l2_lock);
 	return err_code;
 }
 
@@ -1167,6 +1199,7 @@ static int usbvision_radio_close(struct file *file)
 
 	PDEBUG(DBG_IO, "");
 
+	mutex_lock(&usbvision->v4l2_lock);
 	/* Set packet size to 0 */
 	usbvision->iface_alt = 0;
 	err_code = usb_set_interface(usbvision->dev, usbvision->iface,
@@ -1186,6 +1219,7 @@ static int usbvision_radio_close(struct file *file)
 		usbvision_release(usbvision);
 	}
 
+	mutex_unlock(&usbvision->v4l2_lock);
 	PDEBUG(DBG_IO, "success");
 	return err_code;
 }
@@ -1296,10 +1330,6 @@ static struct video_device *usbvision_vdev_init(struct usb_usbvision *usbvision,
 	if (NULL == vdev)
 		return NULL;
 	*vdev = *vdev_template;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags);
 	vdev->lock = &usbvision->v4l2_lock;
 	vdev->v4l2_dev = &usbvision->v4l2_dev;
 	snprintf(vdev->name, sizeof(vdev->name), "%s", name);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 05/26] em28xx: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (2 preceding siblings ...)
  2012-06-24 11:25   ` [RFC PATCH 04/26] usbvision: " Hans Verkuil
@ 2012-06-24 11:25   ` Hans Verkuil
  2012-06-24 11:25   ` [RFC PATCH 06/26] tm6000: " Hans Verkuil
                     ` (20 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:25 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/em28xx/em28xx-video.c |   52 +++++++++++++++++++----------
 1 file changed, 35 insertions(+), 17 deletions(-)

diff --git a/drivers/media/video/em28xx/em28xx-video.c b/drivers/media/video/em28xx/em28xx-video.c
index 50f5f4f..ecb23df 100644
--- a/drivers/media/video/em28xx/em28xx-video.c
+++ b/drivers/media/video/em28xx/em28xx-video.c
@@ -2146,9 +2146,12 @@ static int em28xx_v4l2_open(struct file *filp)
 			dev->users);
 
 
+	if (mutex_lock_interruptible(&dev->lock))
+		return -ERESTARTSYS;
 	fh = kzalloc(sizeof(struct em28xx_fh), GFP_KERNEL);
 	if (!fh) {
 		em28xx_errdev("em28xx-video.c: Out of memory?!\n");
+		mutex_unlock(&dev->lock);
 		return -ENOMEM;
 	}
 	fh->dev = dev;
@@ -2189,6 +2192,7 @@ static int em28xx_v4l2_open(struct file *filp)
 				    V4L2_BUF_TYPE_VBI_CAPTURE,
 				    V4L2_FIELD_SEQ_TB,
 				    sizeof(struct em28xx_buffer), fh, &dev->lock);
+	mutex_unlock(&dev->lock);
 
 	return errCode;
 }
@@ -2243,6 +2247,7 @@ static int em28xx_v4l2_close(struct file *filp)
 
 	em28xx_videodbg("users=%d\n", dev->users);
 
+	mutex_lock(&dev->lock);
 	if (res_check(fh, EM28XX_RESOURCE_VIDEO)) {
 		videobuf_stop(&fh->vb_vidq);
 		res_free(fh, EM28XX_RESOURCE_VIDEO);
@@ -2261,6 +2266,7 @@ static int em28xx_v4l2_close(struct file *filp)
 			kfree(dev->alt_max_pkt_size);
 			kfree(dev);
 			kfree(fh);
+			mutex_unlock(&dev->lock);
 			return 0;
 		}
 
@@ -2285,6 +2291,7 @@ static int em28xx_v4l2_close(struct file *filp)
 	videobuf_mmap_free(&fh->vb_vbiq);
 	kfree(fh);
 	dev->users--;
+	mutex_unlock(&dev->lock);
 	return 0;
 }
 
@@ -2304,35 +2311,35 @@ em28xx_v4l2_read(struct file *filp, char __user *buf, size_t count,
 	if (rc < 0)
 		return rc;
 
+	if (mutex_lock_interruptible(&dev->lock))
+		return -ERESTARTSYS;
 	/* FIXME: read() is not prepared to allow changing the video
 	   resolution while streaming. Seems a bug at em28xx_set_fmt
 	 */
 
 	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
 		if (res_locked(dev, EM28XX_RESOURCE_VIDEO))
-			return -EBUSY;
-
-		return videobuf_read_stream(&fh->vb_vidq, buf, count, pos, 0,
+			rc = -EBUSY;
+		else
+			rc = videobuf_read_stream(&fh->vb_vidq, buf, count, pos, 0,
 					filp->f_flags & O_NONBLOCK);
-	}
-
-
-	if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
+	} else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
 		if (!res_get(fh, EM28XX_RESOURCE_VBI))
-			return -EBUSY;
-
-		return videobuf_read_stream(&fh->vb_vbiq, buf, count, pos, 0,
+			rc = -EBUSY;
+		else
+			rc = videobuf_read_stream(&fh->vb_vbiq, buf, count, pos, 0,
 					filp->f_flags & O_NONBLOCK);
 	}
+	mutex_unlock(&dev->lock);
 
-	return 0;
+	return rc;
 }
 
 /*
- * em28xx_v4l2_poll()
+ * em28xx_poll()
  * will allocate buffers when called for the first time
  */
-static unsigned int em28xx_v4l2_poll(struct file *filp, poll_table *wait)
+static unsigned int em28xx_poll(struct file *filp, poll_table *wait)
 {
 	struct em28xx_fh *fh = filp->private_data;
 	struct em28xx *dev = fh->dev;
@@ -2355,6 +2362,18 @@ static unsigned int em28xx_v4l2_poll(struct file *filp, poll_table *wait)
 	}
 }
 
+static unsigned int em28xx_v4l2_poll(struct file *filp, poll_table *wait)
+{
+	struct em28xx_fh *fh = filp->private_data;
+	struct em28xx *dev = fh->dev;
+	unsigned int res;
+
+	mutex_lock(&dev->lock);
+	res = em28xx_poll(filp, wait);
+	mutex_unlock(&dev->lock);
+	return res;
+}
+
 /*
  * em28xx_v4l2_mmap()
  */
@@ -2368,10 +2387,13 @@ static int em28xx_v4l2_mmap(struct file *filp, struct vm_area_struct *vma)
 	if (rc < 0)
 		return rc;
 
+	if (mutex_lock_interruptible(&dev->lock))
+		return -ERESTARTSYS;
 	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		rc = videobuf_mmap_mapper(&fh->vb_vidq, vma);
 	else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
 		rc = videobuf_mmap_mapper(&fh->vb_vbiq, vma);
+	mutex_unlock(&dev->lock);
 
 	em28xx_videodbg("vma start=0x%08lx, size=%ld, ret=%d\n",
 		(unsigned long)vma->vm_start,
@@ -2495,10 +2517,6 @@ static struct video_device *em28xx_vdev_init(struct em28xx *dev,
 	vfd->release	= video_device_release;
 	vfd->debug	= video_debug;
 	vfd->lock	= &dev->lock;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 
 	snprintf(vfd->name, sizeof(vfd->name), "%s %s",
 		 dev->name, type_name);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 06/26] tm6000: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (3 preceding siblings ...)
  2012-06-24 11:25   ` [RFC PATCH 05/26] em28xx: " Hans Verkuil
@ 2012-06-24 11:25   ` Hans Verkuil
  2012-06-24 11:25   ` [RFC PATCH 07/26] mem2mem_testdev: " Hans Verkuil
                     ` (19 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:25 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/tm6000/tm6000-video.c |   52 ++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 9 deletions(-)

diff --git a/drivers/media/video/tm6000/tm6000-video.c b/drivers/media/video/tm6000/tm6000-video.c
index f7034df..45ed59c 100644
--- a/drivers/media/video/tm6000/tm6000-video.c
+++ b/drivers/media/video/tm6000/tm6000-video.c
@@ -1448,7 +1448,7 @@ static int radio_queryctrl(struct file *file, void *priv,
 	File operations for the device
    ------------------------------------------------------------------*/
 
-static int tm6000_open(struct file *file)
+static int __tm6000_open(struct file *file)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct tm6000_core *dev = video_drvdata(file);
@@ -1540,23 +1540,41 @@ static int tm6000_open(struct file *file)
 	return 0;
 }
 
+static int tm6000_open(struct file *file)
+{
+	struct video_device *vdev = video_devdata(file);
+	int res;
+
+	mutex_lock(vdev->lock);
+	res = __tm6000_open(file);
+	mutex_unlock(vdev->lock);
+	return res;
+}
+
 static ssize_t
 tm6000_read(struct file *file, char __user *data, size_t count, loff_t *pos)
 {
-	struct tm6000_fh        *fh = file->private_data;
+	struct tm6000_fh *fh = file->private_data;
+	struct tm6000_core *dev = fh->dev;
 
 	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+		int res;
+
 		if (!res_get(fh->dev, fh, true))
 			return -EBUSY;
 
-		return videobuf_read_stream(&fh->vb_vidq, data, count, pos, 0,
+		if (mutex_lock_interruptible(&dev->lock))
+			return -ERESTARTSYS;
+		res = videobuf_read_stream(&fh->vb_vidq, data, count, pos, 0,
 					file->f_flags & O_NONBLOCK);
+		mutex_unlock(&dev->lock);
+		return res;
 	}
 	return 0;
 }
 
 static unsigned int
-tm6000_poll(struct file *file, struct poll_table_struct *wait)
+__tm6000_poll(struct file *file, struct poll_table_struct *wait)
 {
 	struct tm6000_fh        *fh = file->private_data;
 	struct tm6000_buffer    *buf;
@@ -1583,6 +1601,18 @@ tm6000_poll(struct file *file, struct poll_table_struct *wait)
 	return 0;
 }
 
+static unsigned int tm6000_poll(struct file *file, struct poll_table_struct *wait)
+{
+	struct tm6000_fh *fh = file->private_data;
+	struct tm6000_core *dev = fh->dev;
+	unsigned int res;
+
+	mutex_lock(&dev->lock);
+	res = __tm6000_poll(file, wait);
+	mutex_unlock(&dev->lock);
+	return res;
+}
+
 static int tm6000_release(struct file *file)
 {
 	struct tm6000_fh         *fh = file->private_data;
@@ -1592,6 +1622,7 @@ static int tm6000_release(struct file *file)
 	dprintk(dev, V4L2_DEBUG_OPEN, "tm6000: close called (dev=%s, users=%d)\n",
 		video_device_node_name(vdev), dev->users);
 
+	mutex_lock(&dev->lock);
 	dev->users--;
 
 	res_free(dev, fh);
@@ -1619,6 +1650,7 @@ static int tm6000_release(struct file *file)
 	}
 
 	kfree(fh);
+	mutex_unlock(&dev->lock);
 
 	return 0;
 }
@@ -1626,8 +1658,14 @@ static int tm6000_release(struct file *file)
 static int tm6000_mmap(struct file *file, struct vm_area_struct * vma)
 {
 	struct tm6000_fh *fh = file->private_data;
+	struct tm6000_core *dev = fh->dev;
+	int res;
 
-	return videobuf_mmap_mapper(&fh->vb_vidq, vma);
+	if (mutex_lock_interruptible(&dev->lock))
+		return -ERESTARTSYS;
+	res = videobuf_mmap_mapper(&fh->vb_vidq, vma);
+	mutex_unlock(&dev->lock);
+	return res;
 }
 
 static struct v4l2_file_operations tm6000_fops = {
@@ -1724,10 +1762,6 @@ static struct video_device *vdev_init(struct tm6000_core *dev,
 	vfd->release = video_device_release;
 	vfd->debug = tm6000_debug;
 	vfd->lock = &dev->lock;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 
 	snprintf(vfd->name, sizeof(vfd->name), "%s %s", dev->name, type_name);
 
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 07/26] mem2mem_testdev: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (4 preceding siblings ...)
  2012-06-24 11:25   ` [RFC PATCH 06/26] tm6000: " Hans Verkuil
@ 2012-06-24 11:25   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 08/26] dt3155v4l: " Hans Verkuil
                     ` (18 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:25 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/mem2mem_testdev.c |   29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/media/video/mem2mem_testdev.c b/drivers/media/video/mem2mem_testdev.c
index d2dec58..595e268 100644
--- a/drivers/media/video/mem2mem_testdev.c
+++ b/drivers/media/video/mem2mem_testdev.c
@@ -849,10 +849,15 @@ static int m2mtest_open(struct file *file)
 {
 	struct m2mtest_dev *dev = video_drvdata(file);
 	struct m2mtest_ctx *ctx = NULL;
+	int rc = 0;
 
+	if (mutex_lock_interruptible(&dev->dev_mutex))
+		return -ERESTARTSYS;
 	ctx = kzalloc(sizeof *ctx, GFP_KERNEL);
-	if (!ctx)
-		return -ENOMEM;
+	if (!ctx) {
+		rc = -ENOMEM;
+		goto open_unlock;
+	}
 
 	file->private_data = ctx;
 	ctx->dev = dev;
@@ -863,16 +868,18 @@ static int m2mtest_open(struct file *file)
 	ctx->m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, ctx, &queue_init);
 
 	if (IS_ERR(ctx->m2m_ctx)) {
-		int ret = PTR_ERR(ctx->m2m_ctx);
+		rc = PTR_ERR(ctx->m2m_ctx);
 
 		kfree(ctx);
-		return ret;
+		goto open_unlock;
 	}
 
 	atomic_inc(&dev->num_inst);
 
 	dprintk(dev, "Created instance %p, m2m_ctx: %p\n", ctx, ctx->m2m_ctx);
 
+open_unlock:
+	mutex_unlock(&dev->dev_mutex);
 	return 0;
 }
 
@@ -883,7 +890,9 @@ static int m2mtest_release(struct file *file)
 
 	dprintk(dev, "Releasing instance %p\n", ctx);
 
+	mutex_lock(&dev->dev_mutex);
 	v4l2_m2m_ctx_release(ctx->m2m_ctx);
+	mutex_unlock(&dev->dev_mutex);
 	kfree(ctx);
 
 	atomic_dec(&dev->num_inst);
@@ -901,9 +910,15 @@ static unsigned int m2mtest_poll(struct file *file,
 
 static int m2mtest_mmap(struct file *file, struct vm_area_struct *vma)
 {
+	struct m2mtest_dev *dev = video_drvdata(file);
 	struct m2mtest_ctx *ctx = file->private_data;
+	int res;
 
-	return v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
+	if (mutex_lock_interruptible(&dev->dev_mutex))
+		return -ERESTARTSYS;
+	res = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
+	mutex_unlock(&dev->dev_mutex);
+	return res;
 }
 
 static const struct v4l2_file_operations m2mtest_fops = {
@@ -958,10 +973,6 @@ static int m2mtest_probe(struct platform_device *pdev)
 	}
 
 	*vfd = m2mtest_videodev;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 	vfd->lock = &dev->dev_mutex;
 
 	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 08/26] dt3155v4l: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (5 preceding siblings ...)
  2012-06-24 11:25   ` [RFC PATCH 07/26] mem2mem_testdev: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 09/26] wl128x: " Hans Verkuil
                     ` (17 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/staging/media/dt3155v4l/dt3155v4l.c |   29 ++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/dt3155v4l/dt3155v4l.c b/drivers/staging/media/dt3155v4l/dt3155v4l.c
index c365cdf..f10ac45 100644
--- a/drivers/staging/media/dt3155v4l/dt3155v4l.c
+++ b/drivers/staging/media/dt3155v4l/dt3155v4l.c
@@ -381,6 +381,8 @@ dt3155_open(struct file *filp)
 	int ret = 0;
 	struct dt3155_priv *pd = video_drvdata(filp);
 
+	if (mutex_lock_interruptible(&pd->mux))
+		return -ERESTARTSYS;
 	if (!pd->users) {
 		pd->q = kzalloc(sizeof(*pd->q), GFP_KERNEL);
 		if (!pd->q) {
@@ -411,6 +413,7 @@ err_request_irq:
 	kfree(pd->q);
 	pd->q = NULL;
 err_alloc_queue:
+	mutex_unlock(&pd->mux);
 	return ret;
 }
 
@@ -419,6 +422,7 @@ dt3155_release(struct file *filp)
 {
 	struct dt3155_priv *pd = video_drvdata(filp);
 
+	mutex_lock(&pd->mux);
 	pd->users--;
 	BUG_ON(pd->users < 0);
 	if (!pd->users) {
@@ -429,6 +433,7 @@ dt3155_release(struct file *filp)
 		kfree(pd->q);
 		pd->q = NULL;
 	}
+	mutex_unlock(&pd->mux);
 	return 0;
 }
 
@@ -436,24 +441,38 @@ static ssize_t
 dt3155_read(struct file *filp, char __user *user, size_t size, loff_t *loff)
 {
 	struct dt3155_priv *pd = video_drvdata(filp);
+	ssize_t res;
 
-	return vb2_read(pd->q, user, size, loff, filp->f_flags & O_NONBLOCK);
+	if (mutex_lock_interruptible(&pd->mux))
+		return -ERESTARTSYS;
+	res = vb2_read(pd->q, user, size, loff, filp->f_flags & O_NONBLOCK);
+	mutex_unlock(&pd->mux);
+	return res;
 }
 
 static unsigned int
 dt3155_poll(struct file *filp, struct poll_table_struct *polltbl)
 {
 	struct dt3155_priv *pd = video_drvdata(filp);
+	unsigned int res;
 
-	return vb2_poll(pd->q, filp, polltbl);
+	mutex_lock(&pd->mux);
+	res = vb2_poll(pd->q, filp, polltbl);
+	mutex_unlock(&pd->mux);
+	return res;
 }
 
 static int
 dt3155_mmap(struct file *filp, struct vm_area_struct *vma)
 {
 	struct dt3155_priv *pd = video_drvdata(filp);
+	int res;
 
-	return vb2_mmap(pd->q, vma);
+	if (mutex_lock_interruptible(&pd->mux))
+		return -ERESTARTSYS;
+	res = vb2_mmap(pd->q, vma);
+	mutex_unlock(&pd->mux);
+	return res;
 }
 
 static const struct v4l2_file_operations dt3155_fops = {
@@ -898,10 +917,6 @@ dt3155_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	INIT_LIST_HEAD(&pd->dmaq);
 	mutex_init(&pd->mux);
 	pd->vdev->lock = &pd->mux; /* for locking v4l2_file_operations */
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &pd->vdev->flags);
 	spin_lock_init(&pd->lock);
 	pd->csr2 = csr2_init;
 	pd->config = config_init;
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 09/26] wl128x: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (6 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 08/26] dt3155v4l: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 10/26] fsl-viu: " Hans Verkuil
                     ` (16 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/radio/wl128x/fmdrv_v4l2.c |   38 +++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/media/radio/wl128x/fmdrv_v4l2.c b/drivers/media/radio/wl128x/fmdrv_v4l2.c
index 49a11ec..db2248e 100644
--- a/drivers/media/radio/wl128x/fmdrv_v4l2.c
+++ b/drivers/media/radio/wl128x/fmdrv_v4l2.c
@@ -56,23 +56,29 @@ static ssize_t fm_v4l2_fops_read(struct file *file, char __user * buf,
 		return -EIO;
 	}
 
-	/* Turn on RDS mode , if it is disabled */
+	if (mutex_lock_interruptible(&fmdev->mutex))
+		return -ERESTARTSYS;
+
+	/* Turn on RDS mode if it is disabled */
 	ret = fm_rx_get_rds_mode(fmdev, &rds_mode);
 	if (ret < 0) {
 		fmerr("Unable to read current rds mode\n");
-		return ret;
+		goto read_unlock;
 	}
 
 	if (rds_mode == FM_RDS_DISABLE) {
 		ret = fmc_set_rds_mode(fmdev, FM_RDS_ENABLE);
 		if (ret < 0) {
 			fmerr("Failed to enable rds mode\n");
-			return ret;
+			goto read_unlock;
 		}
 	}
 
 	/* Copy RDS data from internal buffer to user buffer */
-	return fmc_transfer_rds_from_internal_buff(fmdev, file, buf, count);
+	ret = fmc_transfer_rds_from_internal_buff(fmdev, file, buf, count);
+read_unlock:
+	mutex_unlock(&fmdev->mutex);
+	return ret;
 }
 
 /* Write TX RDS data */
@@ -91,8 +97,11 @@ static ssize_t fm_v4l2_fops_write(struct file *file, const char __user * buf,
 		return -EFAULT;
 
 	fmdev = video_drvdata(file);
+	if (mutex_lock_interruptible(&fmdev->mutex))
+		return -ERESTARTSYS;
 	fm_tx_set_radio_text(fmdev, rds.text, rds.text_type);
 	fm_tx_set_af(fmdev, rds.af_freq);
+	mutex_unlock(&fmdev->mutex);
 
 	return sizeof(rds);
 }
@@ -103,7 +112,9 @@ static u32 fm_v4l2_fops_poll(struct file *file, struct poll_table_struct *pts)
 	struct fmdev *fmdev;
 
 	fmdev = video_drvdata(file);
+	mutex_lock(&fmdev->mutex);
 	ret = fmc_is_rds_data_available(fmdev, file, pts);
+	mutex_unlock(&fmdev->mutex);
 	if (ret < 0)
 		return POLLIN | POLLRDNORM;
 
@@ -127,10 +138,12 @@ static int fm_v4l2_fops_open(struct file *file)
 
 	fmdev = video_drvdata(file);
 
+	if (mutex_lock_interruptible(&fmdev->mutex))
+		return -ERESTARTSYS;
 	ret = fmc_prepare(fmdev);
 	if (ret < 0) {
 		fmerr("Unable to prepare FM CORE\n");
-		return ret;
+		goto open_unlock;
 	}
 
 	fmdbg("Load FM RX firmware..\n");
@@ -138,10 +151,12 @@ static int fm_v4l2_fops_open(struct file *file)
 	ret = fmc_set_mode(fmdev, FM_MODE_RX);
 	if (ret < 0) {
 		fmerr("Unable to load FM RX firmware\n");
-		return ret;
+		goto open_unlock;
 	}
 	radio_disconnected = 1;
 
+open_unlock:
+	mutex_unlock(&fmdev->mutex);
 	return ret;
 }
 
@@ -156,19 +171,22 @@ static int fm_v4l2_fops_release(struct file *file)
 		return 0;
 	}
 
+	mutex_lock(&fmdev->mutex);
 	ret = fmc_set_mode(fmdev, FM_MODE_OFF);
 	if (ret < 0) {
 		fmerr("Unable to turn off the chip\n");
-		return ret;
+		goto release_unlock;
 	}
 
 	ret = fmc_release(fmdev);
 	if (ret < 0) {
 		fmerr("FM CORE release failed\n");
-		return ret;
+		goto release_unlock;
 	}
 	radio_disconnected = 0;
 
+release_unlock:
+	mutex_unlock(&fmdev->mutex);
 	return ret;
 }
 
@@ -520,10 +538,6 @@ int fm_v4l2_init_video_device(struct fmdev *fmdev, int radio_nr)
 	video_set_drvdata(gradio_dev, fmdev);
 
 	gradio_dev->lock = &fmdev->mutex;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &gradio_dev->flags);
 
 	/* Register with V4L2 subsystem as RADIO device */
 	if (video_register_device(gradio_dev, VFL_TYPE_RADIO, radio_nr)) {
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 10/26] fsl-viu: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (7 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 09/26] wl128x: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 11/26] s2255drv: " Hans Verkuil
                     ` (15 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/fsl-viu.c |   27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/media/video/fsl-viu.c b/drivers/media/video/fsl-viu.c
index 777486f..20f9810 100644
--- a/drivers/media/video/fsl-viu.c
+++ b/drivers/media/video/fsl-viu.c
@@ -1279,10 +1279,16 @@ static int viu_open(struct file *file)
 	dprintk(1, "open minor=%d type=%s users=%d\n", minor,
 		v4l2_type_names[V4L2_BUF_TYPE_VIDEO_CAPTURE], dev->users);
 
+	if (mutex_lock_interruptible(&dev->lock)) {
+		dev->users--;
+		return -ERESTARTSYS;
+	}
+
 	/* allocate and initialize per filehandle data */
 	fh = kzalloc(sizeof(*fh), GFP_KERNEL);
 	if (!fh) {
 		dev->users--;
+		mutex_unlock(&dev->lock);
 		return -ENOMEM;
 	}
 
@@ -1325,6 +1331,7 @@ static int viu_open(struct file *file)
 				       fh->type, V4L2_FIELD_INTERLACED,
 				       sizeof(struct viu_buf), fh,
 				       &fh->dev->lock);
+	mutex_unlock(&dev->lock);
 	return 0;
 }
 
@@ -1340,9 +1347,12 @@ static ssize_t viu_read(struct file *file, char __user *data, size_t count,
 		dev->ovenable = 0;
 
 	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+		if (mutex_lock_interruptible(&dev->lock))
+			return -ERESTARTSYS;
 		viu_start_dma(dev);
 		ret = videobuf_read_stream(&fh->vb_vidq, data, count,
 				ppos, 0, file->f_flags & O_NONBLOCK);
+		mutex_unlock(&dev->lock);
 		return ret;
 	}
 	return 0;
@@ -1352,11 +1362,16 @@ static unsigned int viu_poll(struct file *file, struct poll_table_struct *wait)
 {
 	struct viu_fh *fh = file->private_data;
 	struct videobuf_queue *q = &fh->vb_vidq;
+	struct viu_dev *dev = fh->dev;
+	unsigned int res;
 
 	if (V4L2_BUF_TYPE_VIDEO_CAPTURE != fh->type)
 		return POLLERR;
 
-	return videobuf_poll_stream(file, q, wait);
+	mutex_lock(&dev->lock);
+	res = videobuf_poll_stream(file, q, wait);
+	mutex_unlock(&dev->lock);
+	return res;
 }
 
 static int viu_release(struct file *file)
@@ -1365,9 +1380,11 @@ static int viu_release(struct file *file)
 	struct viu_dev *dev = fh->dev;
 	int minor = video_devdata(file)->minor;
 
+	mutex_lock(&dev->lock);
 	viu_stop_dma(dev);
 	videobuf_stop(&fh->vb_vidq);
 	videobuf_mmap_free(&fh->vb_vidq);
+	mutex_unlock(&dev->lock);
 
 	kfree(fh);
 
@@ -1394,11 +1411,15 @@ void viu_reset(struct viu_reg *reg)
 static int viu_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct viu_fh *fh = file->private_data;
+	struct viu_dev *dev = fh->dev;
 	int ret;
 
 	dprintk(1, "mmap called, vma=0x%08lx\n", (unsigned long)vma);
 
+	if (mutex_lock_interruptible(&dev->lock))
+		return -ERESTARTSYS;
 	ret = videobuf_mmap_mapper(&fh->vb_vidq, vma);
+	mutex_unlock(&dev->lock);
 
 	dprintk(1, "vma start=0x%08lx, size=%ld, ret=%d\n",
 		(unsigned long)vma->vm_start,
@@ -1544,10 +1565,6 @@ static int __devinit viu_of_probe(struct platform_device *op)
 
 	/* initialize locks */
 	mutex_init(&viu_dev->lock);
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &viu_dev->vdev->flags);
 	viu_dev->vdev->lock = &viu_dev->lock;
 	spin_lock_init(&viu_dev->slock);
 
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 11/26] s2255drv: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (8 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 10/26] fsl-viu: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 12/26] vpbe_display: " Hans Verkuil
                     ` (14 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/s2255drv.c |   42 ++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/media/video/s2255drv.c b/drivers/media/video/s2255drv.c
index 01c2179..0001f1d 100644
--- a/drivers/media/video/s2255drv.c
+++ b/drivers/media/video/s2255drv.c
@@ -258,7 +258,6 @@ struct s2255_dev {
 	atomic_t                num_channels;
 	int			frames;
 	struct mutex		lock;	/* channels[].vdev.lock */
-	struct mutex		open_lock;
 	struct usb_device	*udev;
 	struct usb_interface	*interface;
 	u8			read_endpoint;
@@ -1684,7 +1683,7 @@ static int vidioc_enum_frameintervals(struct file *file, void *priv,
 	return 0;
 }
 
-static int s2255_open(struct file *file)
+static int __s2255_open(struct file *file)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct s2255_channel *channel = video_drvdata(file);
@@ -1694,16 +1693,9 @@ static int s2255_open(struct file *file)
 	int state;
 	dprintk(1, "s2255: open called (dev=%s)\n",
 		video_device_node_name(vdev));
-	/*
-	 * open lock necessary to prevent multiple instances
-	 * of v4l-conf (or other programs) from simultaneously
-	 * reloading firmware.
-	 */
-	mutex_lock(&dev->open_lock);
 	state = atomic_read(&dev->fw_data->fw_state);
 	switch (state) {
 	case S2255_FW_DISCONNECTING:
-		mutex_unlock(&dev->open_lock);
 		return -ENODEV;
 	case S2255_FW_FAILED:
 		s2255_dev_err(&dev->udev->dev,
@@ -1742,11 +1734,9 @@ static int s2255_open(struct file *file)
 		break;
 	case S2255_FW_FAILED:
 		printk(KERN_INFO "2255 firmware load failed.\n");
-		mutex_unlock(&dev->open_lock);
 		return -ENODEV;
 	case S2255_FW_DISCONNECTING:
 		printk(KERN_INFO "%s: disconnecting\n", __func__);
-		mutex_unlock(&dev->open_lock);
 		return -ENODEV;
 	case S2255_FW_LOADED_DSPWAIT:
 	case S2255_FW_NOTLOADED:
@@ -1760,14 +1750,11 @@ static int s2255_open(struct file *file)
 		 */
 		atomic_set(&dev->fw_data->fw_state,
 			   S2255_FW_FAILED);
-		mutex_unlock(&dev->open_lock);
 		return -EAGAIN;
 	default:
 		printk(KERN_INFO "%s: unknown state\n", __func__);
-		mutex_unlock(&dev->open_lock);
 		return -EFAULT;
 	}
-	mutex_unlock(&dev->open_lock);
 	/* allocate + initialize per filehandle data */
 	fh = kzalloc(sizeof(*fh), GFP_KERNEL);
 	if (NULL == fh)
@@ -1798,16 +1785,30 @@ static int s2255_open(struct file *file)
 	return 0;
 }
 
+static int s2255_open(struct file *file)
+{
+	struct video_device *vdev = video_devdata(file);
+	int ret;
+
+	if (mutex_lock_interruptible(vdev->lock))
+		return -ERESTARTSYS;
+	ret = __s2255_open(file);
+	mutex_unlock(vdev->lock);
+	return ret;
+}
 
 static unsigned int s2255_poll(struct file *file,
 			       struct poll_table_struct *wait)
 {
 	struct s2255_fh *fh = file->private_data;
+	struct s2255_dev *dev = fh->dev;
 	int rc;
 	dprintk(100, "%s\n", __func__);
 	if (V4L2_BUF_TYPE_VIDEO_CAPTURE != fh->type)
 		return POLLERR;
+	mutex_lock(&dev->lock);
 	rc = videobuf_poll_stream(file, &fh->vb_vidq, wait);
+	mutex_unlock(&dev->lock);
 	return rc;
 }
 
@@ -1827,7 +1828,6 @@ static void s2255_destroy(struct s2255_dev *dev)
 	kfree(dev->fw_data);
 	/* reset the DSP so firmware can be reloaded next time */
 	s2255_reset_dsppower(dev);
-	mutex_destroy(&dev->open_lock);
 	mutex_destroy(&dev->lock);
 	usb_put_dev(dev->udev);
 	v4l2_device_unregister(&dev->v4l2_dev);
@@ -1843,6 +1843,7 @@ static int s2255_release(struct file *file)
 	struct s2255_channel *channel = fh->channel;
 	if (!dev)
 		return -ENODEV;
+	mutex_lock(&dev->lock);
 	/* turn off stream */
 	if (res_check(fh)) {
 		if (channel->b_acquire)
@@ -1851,6 +1852,7 @@ static int s2255_release(struct file *file)
 		res_free(fh);
 	}
 	videobuf_mmap_free(&fh->vb_vidq);
+	mutex_unlock(&dev->lock);
 	dprintk(1, "%s (dev=%s)\n", __func__, video_device_node_name(vdev));
 	kfree(fh);
 	return 0;
@@ -1859,12 +1861,16 @@ static int s2255_release(struct file *file)
 static int s2255_mmap_v4l(struct file *file, struct vm_area_struct *vma)
 {
 	struct s2255_fh *fh = file->private_data;
+	struct s2255_dev *dev = fh->dev;
 	int ret;
 
 	if (!fh)
 		return -ENODEV;
 	dprintk(4, "%s, vma=0x%08lx\n", __func__, (unsigned long)vma);
+	if (mutex_lock_interruptible(&dev->lock))
+		return -ERESTARTSYS;
 	ret = videobuf_mmap_mapper(&fh->vb_vidq, vma);
+	mutex_unlock(&dev->lock);
 	dprintk(4, "%s vma start=0x%08lx, size=%ld, ret=%d\n", __func__,
 		(unsigned long)vma->vm_start,
 		(unsigned long)vma->vm_end - (unsigned long)vma->vm_start, ret);
@@ -1944,10 +1950,6 @@ static int s2255_probe_v4l(struct s2255_dev *dev)
 		/* register 4 video devices */
 		channel->vdev = template;
 		channel->vdev.lock = &dev->lock;
-		/* Locking in file operations other than ioctl should be done
-		   by the driver, not the V4L2 core.
-		   This driver needs auditing so that this flag can be removed. */
-		set_bit(V4L2_FL_LOCK_ALL_FOPS, &channel->vdev.flags);
 		channel->vdev.v4l2_dev = &dev->v4l2_dev;
 		video_set_drvdata(&channel->vdev, channel);
 		if (video_nr == -1)
@@ -2535,7 +2537,6 @@ static int s2255_probe(struct usb_interface *interface,
 	if (!dev->fw_data)
 		goto errorFWDATA1;
 	mutex_init(&dev->lock);
-	mutex_init(&dev->open_lock);
 	/* grab usb_device and save it */
 	dev->udev = usb_get_dev(interface_to_usbdev(interface));
 	if (dev->udev == NULL) {
@@ -2637,7 +2638,6 @@ errorEP:
 	usb_put_dev(dev->udev);
 errorUDEV:
 	kfree(dev->fw_data);
-	mutex_destroy(&dev->open_lock);
 	mutex_destroy(&dev->lock);
 errorFWDATA1:
 	kfree(dev);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 12/26] vpbe_display: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (9 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 11/26] s2255drv: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 13/26] vpif_capture: " Hans Verkuil
                     ` (13 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/davinci/vpbe_display.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/media/video/davinci/vpbe_display.c b/drivers/media/video/davinci/vpbe_display.c
index e106b72..0f473f6 100644
--- a/drivers/media/video/davinci/vpbe_display.c
+++ b/drivers/media/video/davinci/vpbe_display.c
@@ -1376,10 +1376,15 @@ static int vpbe_display_mmap(struct file *filep, struct vm_area_struct *vma)
 	struct vpbe_fh *fh = filep->private_data;
 	struct vpbe_layer *layer = fh->layer;
 	struct vpbe_device *vpbe_dev = fh->disp_dev->vpbe_dev;
+	int ret;
 
 	v4l2_dbg(1, debug, &vpbe_dev->v4l2_dev, "vpbe_display_mmap\n");
 
-	return videobuf_mmap_mapper(&layer->buffer_queue, vma);
+	if (mutex_lock_interruptible(&layer->opslock))
+		return -ERESTARTSYS;
+	ret = videobuf_mmap_mapper(&layer->buffer_queue, vma);
+	mutex_unlock(&layer->opslock);
+	return ret;
 }
 
 /* vpbe_display_poll(): It is used for select/poll system call
@@ -1392,8 +1397,11 @@ static unsigned int vpbe_display_poll(struct file *filep, poll_table *wait)
 	unsigned int err = 0;
 
 	v4l2_dbg(1, debug, &vpbe_dev->v4l2_dev, "vpbe_display_poll\n");
-	if (layer->started)
+	if (layer->started) {
+		mutex_lock(&layer->opslock);
 		err = videobuf_poll_stream(filep, &layer->buffer_queue, wait);
+		mutex_unlock(&layer->opslock);
+	}
 	return err;
 }
 
@@ -1428,10 +1436,12 @@ static int vpbe_display_open(struct file *file)
 	fh->disp_dev = disp_dev;
 
 	if (!layer->usrs) {
-
+		if (mutex_lock_interruptible(&layer->opslock))
+			return -ERESTARTSYS;
 		/* First claim the layer for this device */
 		err = osd_device->ops.request_layer(osd_device,
 						layer->layer_info.id);
+		mutex_unlock(&layer->opslock);
 		if (err < 0) {
 			/* Couldn't get layer */
 			v4l2_err(&vpbe_dev->v4l2_dev,
@@ -1469,6 +1479,7 @@ static int vpbe_display_release(struct file *file)
 
 	v4l2_dbg(1, debug, &vpbe_dev->v4l2_dev, "vpbe_display_release\n");
 
+	mutex_lock(&layer->opslock);
 	/* if this instance is doing IO */
 	if (fh->io_allowed) {
 		/* Reset io_usrs member of layer object */
@@ -1503,6 +1514,7 @@ static int vpbe_display_release(struct file *file)
 	/* Close the priority */
 	v4l2_prio_close(&layer->prio, fh->prio);
 	file->private_data = NULL;
+	mutex_unlock(&layer->opslock);
 
 	/* Free memory allocated to file handle object */
 	kfree(fh);
@@ -1618,10 +1630,6 @@ static __devinit int init_vpbe_layer(int i, struct vpbe_display *disp_dev,
 	vbd->ioctl_ops	= &vpbe_ioctl_ops;
 	vbd->minor	= -1;
 	vbd->v4l2_dev   = &disp_dev->vpbe_dev->v4l2_dev;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vbd->flags);
 	vbd->lock	= &vpbe_display_layer->opslock;
 
 	if (disp_dev->vpbe_dev->current_timings.timings_type &
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 13/26] vpif_capture: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (10 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 12/26] vpbe_display: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 14/26] vpif_display: " Hans Verkuil
                     ` (12 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/davinci/vpif_capture.c |   28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/davinci/vpif_capture.c b/drivers/media/video/davinci/vpif_capture.c
index 9604695..425f6d8 100644
--- a/drivers/media/video/davinci/vpif_capture.c
+++ b/drivers/media/video/davinci/vpif_capture.c
@@ -713,10 +713,15 @@ static int vpif_mmap(struct file *filep, struct vm_area_struct *vma)
 	struct vpif_fh *fh = filep->private_data;
 	struct channel_obj *ch = fh->channel;
 	struct common_obj *common = &(ch->common[VPIF_VIDEO_INDEX]);
+	int ret;
 
 	vpif_dbg(2, debug, "vpif_mmap\n");
 
-	return videobuf_mmap_mapper(&common->buffer_queue, vma);
+	if (mutex_lock_interruptible(&common->lock))
+		return -ERESTARTSYS;
+	ret = videobuf_mmap_mapper(&common->buffer_queue, vma);
+	mutex_unlock(&common->lock);
+	return ret;
 }
 
 /**
@@ -729,12 +734,16 @@ static unsigned int vpif_poll(struct file *filep, poll_table * wait)
 	struct vpif_fh *fh = filep->private_data;
 	struct channel_obj *channel = fh->channel;
 	struct common_obj *common = &(channel->common[VPIF_VIDEO_INDEX]);
+	unsigned int res = 0;
 
 	vpif_dbg(2, debug, "vpif_poll\n");
 
-	if (common->started)
-		return videobuf_poll_stream(filep, &common->buffer_queue, wait);
-	return 0;
+	if (common->started) {
+		mutex_lock(&common->lock);
+		res = videobuf_poll_stream(filep, &common->buffer_queue, wait);
+		mutex_unlock(&common->lock);
+	}
+	return res;
 }
 
 /**
@@ -788,6 +797,10 @@ static int vpif_open(struct file *filep)
 		return -ENOMEM;
 	}
 
+	if (mutex_lock_interruptible(&common->lock)) {
+		kfree(fh);
+		return -ERESTARTSYS;
+	}
 	/* store pointer to fh in private_data member of filep */
 	filep->private_data = fh;
 	fh->channel = ch;
@@ -805,6 +818,7 @@ static int vpif_open(struct file *filep)
 	/* Initialize priority of this instance to default priority */
 	fh->prio = V4L2_PRIORITY_UNSET;
 	v4l2_prio_open(&ch->prio, &fh->prio);
+	mutex_unlock(&common->lock);
 	return 0;
 }
 
@@ -825,6 +839,7 @@ static int vpif_release(struct file *filep)
 
 	common = &ch->common[VPIF_VIDEO_INDEX];
 
+	mutex_lock(&common->lock);
 	/* if this instance is doing IO */
 	if (fh->io_allowed[VPIF_VIDEO_INDEX]) {
 		/* Reset io_usrs member of channel object */
@@ -854,6 +869,7 @@ static int vpif_release(struct file *filep)
 	if (fh->initialized)
 		ch->initialized = 0;
 
+	mutex_unlock(&common->lock);
 	filep->private_data = NULL;
 	kfree(fh);
 	return 0;
@@ -2228,10 +2244,6 @@ static __init int vpif_probe(struct platform_device *pdev)
 		common = &(ch->common[VPIF_VIDEO_INDEX]);
 		spin_lock_init(&common->irqlock);
 		mutex_init(&common->lock);
-		/* Locking in file operations other than ioctl should be done
-		   by the driver, not the V4L2 core.
-		   This driver needs auditing so that this flag can be removed. */
-		set_bit(V4L2_FL_LOCK_ALL_FOPS, &ch->video_dev->flags);
 		ch->video_dev->lock = &common->lock;
 		/* Initialize prio member of channel object */
 		v4l2_prio_init(&ch->prio);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 14/26] vpif_display: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (11 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 13/26] vpif_capture: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 15/26] mx2_emmaprp: " Hans Verkuil
                     ` (11 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/davinci/vpif_display.c |   34 +++++++++++++++++++---------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/media/video/davinci/vpif_display.c b/drivers/media/video/davinci/vpif_display.c
index e6488ee..43e818a 100644
--- a/drivers/media/video/davinci/vpif_display.c
+++ b/drivers/media/video/davinci/vpif_display.c
@@ -580,10 +580,15 @@ static int vpif_mmap(struct file *filep, struct vm_area_struct *vma)
 	struct vpif_fh *fh = filep->private_data;
 	struct channel_obj *ch = fh->channel;
 	struct common_obj *common = &(ch->common[VPIF_VIDEO_INDEX]);
+	int ret;
 
 	vpif_dbg(2, debug, "vpif_mmap\n");
 
-	return videobuf_mmap_mapper(&common->buffer_queue, vma);
+	if (mutex_lock_interruptible(&common->lock))
+		return -ERESTARTSYS;
+	ret = videobuf_mmap_mapper(&common->buffer_queue, vma);
+	mutex_unlock(&common->lock);
+	return ret;
 }
 
 /*
@@ -594,11 +599,15 @@ static unsigned int vpif_poll(struct file *filep, poll_table *wait)
 	struct vpif_fh *fh = filep->private_data;
 	struct channel_obj *ch = fh->channel;
 	struct common_obj *common = &ch->common[VPIF_VIDEO_INDEX];
+	unsigned int res = 0;
 
-	if (common->started)
-		return videobuf_poll_stream(filep, &common->buffer_queue, wait);
+	if (common->started) {
+		mutex_lock(&common->lock);
+		res = videobuf_poll_stream(filep, &common->buffer_queue, wait);
+		mutex_unlock(&common->lock);
+	}
 
-	return 0;
+	return res;
 }
 
 /*
@@ -608,10 +617,10 @@ static unsigned int vpif_poll(struct file *filep, poll_table *wait)
 static int vpif_open(struct file *filep)
 {
 	struct video_device *vdev = video_devdata(filep);
-	struct channel_obj *ch = NULL;
-	struct vpif_fh *fh = NULL;
+	struct channel_obj *ch = video_get_drvdata(vdev);
+	struct common_obj *common = &ch->common[VPIF_VIDEO_INDEX];
+	struct vpif_fh *fh;
 
-	ch = video_get_drvdata(vdev);
 	/* Allocate memory for the file handle object */
 	fh = kzalloc(sizeof(struct vpif_fh), GFP_KERNEL);
 	if (fh == NULL) {
@@ -619,6 +628,10 @@ static int vpif_open(struct file *filep)
 		return -ENOMEM;
 	}
 
+	if (mutex_lock_interruptible(&common->lock)) {
+		kfree(fh);
+		return -ERESTARTSYS;
+	}
 	/* store pointer to fh in private_data member of filep */
 	filep->private_data = fh;
 	fh->channel = ch;
@@ -636,6 +649,7 @@ static int vpif_open(struct file *filep)
 	/* Initialize priority of this instance to default priority */
 	fh->prio = V4L2_PRIORITY_UNSET;
 	v4l2_prio_open(&ch->prio, &fh->prio);
+	mutex_unlock(&common->lock);
 
 	return 0;
 }
@@ -650,6 +664,7 @@ static int vpif_release(struct file *filep)
 	struct channel_obj *ch = fh->channel;
 	struct common_obj *common = &ch->common[VPIF_VIDEO_INDEX];
 
+	mutex_lock(&common->lock);
 	/* if this instance is doing IO */
 	if (fh->io_allowed[VPIF_VIDEO_INDEX]) {
 		/* Reset io_usrs member of channel object */
@@ -682,6 +697,7 @@ static int vpif_release(struct file *filep)
 	v4l2_prio_close(&ch->prio, fh->prio);
 	filep->private_data = NULL;
 	fh->initialized = 0;
+	mutex_unlock(&common->lock);
 	kfree(fh);
 
 	return 0;
@@ -1778,10 +1794,6 @@ static __init int vpif_probe(struct platform_device *pdev)
 		v4l2_prio_init(&ch->prio);
 		ch->common[VPIF_VIDEO_INDEX].fmt.type =
 						V4L2_BUF_TYPE_VIDEO_OUTPUT;
-		/* Locking in file operations other than ioctl should be done
-		   by the driver, not the V4L2 core.
-		   This driver needs auditing so that this flag can be removed. */
-		set_bit(V4L2_FL_LOCK_ALL_FOPS, &ch->video_dev->flags);
 		ch->video_dev->lock = &common->lock;
 
 		/* register video device */
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 15/26] mx2_emmaprp: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (12 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 14/26] vpif_display: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 16/26] sh_vou: " Hans Verkuil
                     ` (10 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/mx2_emmaprp.c |   28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/mx2_emmaprp.c b/drivers/media/video/mx2_emmaprp.c
index 0bd5815..c5f6c5c 100644
--- a/drivers/media/video/mx2_emmaprp.c
+++ b/drivers/media/video/mx2_emmaprp.c
@@ -791,11 +791,17 @@ static int emmaprp_open(struct file *file)
 	file->private_data = ctx;
 	ctx->dev = pcdev;
 
+	if (mutex_lock_interruptible(&pcdev->dev_mutex)) {
+		kfree(ctx);
+		return -ERESTARTSYS;
+	}
+
 	ctx->m2m_ctx = v4l2_m2m_ctx_init(pcdev->m2m_dev, ctx, &queue_init);
 
 	if (IS_ERR(ctx->m2m_ctx)) {
 		int ret = PTR_ERR(ctx->m2m_ctx);
 
+		mutex_unlock(&pcdev->dev_mutex);
 		kfree(ctx);
 		return ret;
 	}
@@ -803,6 +809,7 @@ static int emmaprp_open(struct file *file)
 	clk_enable(pcdev->clk_emma);
 	ctx->q_data[V4L2_M2M_SRC].fmt = &formats[1];
 	ctx->q_data[V4L2_M2M_DST].fmt = &formats[0];
+	mutex_unlock(&pcdev->dev_mutex);
 
 	dprintk(pcdev, "Created instance %p, m2m_ctx: %p\n", ctx, ctx->m2m_ctx);
 
@@ -816,8 +823,10 @@ static int emmaprp_release(struct file *file)
 
 	dprintk(pcdev, "Releasing instance %p\n", ctx);
 
+	mutex_lock(&pcdev->dev_mutex);
 	clk_disable(pcdev->clk_emma);
 	v4l2_m2m_ctx_release(ctx->m2m_ctx);
+	mutex_unlock(&pcdev->dev_mutex);
 	kfree(ctx);
 
 	return 0;
@@ -826,16 +835,27 @@ static int emmaprp_release(struct file *file)
 static unsigned int emmaprp_poll(struct file *file,
 				 struct poll_table_struct *wait)
 {
+	struct emmaprp_dev *pcdev = video_drvdata(file);
 	struct emmaprp_ctx *ctx = file->private_data;
+	unsigned int res;
 
-	return v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
+	mutex_lock(&pcdev->dev_mutex);
+	res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
+	mutex_unlock(&pcdev->dev_mutex);
+	return res;
 }
 
 static int emmaprp_mmap(struct file *file, struct vm_area_struct *vma)
 {
+	struct emmaprp_dev *pcdev = video_drvdata(file);
 	struct emmaprp_ctx *ctx = file->private_data;
+	int ret;
 
-	return v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
+	if (mutex_lock_interruptible(&pcdev->dev_mutex))
+		return -ERESTARTSYS;
+	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
+	mutex_unlock(&pcdev->dev_mutex);
+	return ret;
 }
 
 static const struct v4l2_file_operations emmaprp_fops = {
@@ -904,10 +924,6 @@ static int emmaprp_probe(struct platform_device *pdev)
 	}
 
 	*vfd = emmaprp_videodev;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 	vfd->lock = &pcdev->dev_mutex;
 
 	video_set_drvdata(vfd, pcdev);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 16/26] sh_vou: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (13 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 15/26] mx2_emmaprp: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 17/26] bfin_capture: " Hans Verkuil
                     ` (9 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/sh_vou.c |   25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/sh_vou.c b/drivers/media/video/sh_vou.c
index 8fd1874..9f62fd8 100644
--- a/drivers/media/video/sh_vou.c
+++ b/drivers/media/video/sh_vou.c
@@ -1171,6 +1171,8 @@ static int sh_vou_open(struct file *file)
 
 	file->private_data = vou_file;
 
+	if (mutex_lock_interruptible(&vou_dev->fop_lock))
+		return -ERESTARTSYS;
 	if (atomic_inc_return(&vou_dev->use_count) == 1) {
 		int ret;
 		/* First open */
@@ -1181,6 +1183,7 @@ static int sh_vou_open(struct file *file)
 			atomic_dec(&vou_dev->use_count);
 			pm_runtime_put(vdev->v4l2_dev->dev);
 			vou_dev->status = SH_VOU_IDLE;
+			mutex_unlock(&vou_dev->fop_lock);
 			return ret;
 		}
 	}
@@ -1191,6 +1194,7 @@ static int sh_vou_open(struct file *file)
 				       V4L2_FIELD_NONE,
 				       sizeof(struct videobuf_buffer), vdev,
 				       &vou_dev->fop_lock);
+	mutex_unlock(&vou_dev->fop_lock);
 
 	return 0;
 }
@@ -1204,10 +1208,12 @@ static int sh_vou_release(struct file *file)
 	dev_dbg(vou_file->vbq.dev, "%s()\n", __func__);
 
 	if (!atomic_dec_return(&vou_dev->use_count)) {
+		mutex_lock(&vou_dev->fop_lock);
 		/* Last close */
 		vou_dev->status = SH_VOU_IDLE;
 		sh_vou_reg_a_set(vou_dev, VOUER, 0, 0x101);
 		pm_runtime_put(vdev->v4l2_dev->dev);
+		mutex_unlock(&vou_dev->fop_lock);
 	}
 
 	file->private_data = NULL;
@@ -1218,20 +1224,31 @@ static int sh_vou_release(struct file *file)
 
 static int sh_vou_mmap(struct file *file, struct vm_area_struct *vma)
 {
+	struct sh_vou_device *vou_dev = video_get_drvdata(vdev);
 	struct sh_vou_file *vou_file = file->private_data;
+	int ret;
 
 	dev_dbg(vou_file->vbq.dev, "%s()\n", __func__);
 
-	return videobuf_mmap_mapper(&vou_file->vbq, vma);
+	if (mutex_lock_interruptible(&vou_dev->fop_lock))
+		return -ERESTARTSYS;
+	ret = videobuf_mmap_mapper(&vou_file->vbq, vma);
+	mutex_unlock(&vou_dev->fop_lock);
+	return ret;
 }
 
 static unsigned int sh_vou_poll(struct file *file, poll_table *wait)
 {
+	struct sh_vou_device *vou_dev = video_get_drvdata(vdev);
 	struct sh_vou_file *vou_file = file->private_data;
+	unsigned int res;
 
 	dev_dbg(vou_file->vbq.dev, "%s()\n", __func__);
 
-	return videobuf_poll_stream(file, &vou_file->vbq, wait);
+	mutex_lock(&vou_dev->fop_lock);
+	res = videobuf_poll_stream(file, &vou_file->vbq, wait);
+	mutex_unlock(&vou_dev->fop_lock);
+	return res;
 }
 
 static int sh_vou_g_chip_ident(struct file *file, void *fh,
@@ -1390,10 +1407,6 @@ static int __devinit sh_vou_probe(struct platform_device *pdev)
 	vdev->v4l2_dev = &vou_dev->v4l2_dev;
 	vdev->release = video_device_release;
 	vdev->lock = &vou_dev->fop_lock;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags);
 
 	vou_dev->vdev = vdev;
 	video_set_drvdata(vdev, vou_dev);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 17/26] bfin_capture: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (14 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 16/26] sh_vou: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 18/26] cx231xx: " Hans Verkuil
                     ` (8 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/blackfin/bfin_capture.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/blackfin/bfin_capture.c b/drivers/media/video/blackfin/bfin_capture.c
index 0aba45e..1677623 100644
--- a/drivers/media/video/blackfin/bfin_capture.c
+++ b/drivers/media/video/blackfin/bfin_capture.c
@@ -235,8 +235,13 @@ static int bcap_release(struct file *file)
 static int bcap_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct bcap_device *bcap_dev = video_drvdata(file);
+	int ret;
 
-	return vb2_mmap(&bcap_dev->buffer_queue, vma);
+	if (mutex_lock_interruptible(&bcap_dev->mutex))
+		return -ERESTARTSYS;
+	ret = vb2_mmap(&bcap_dev->buffer_queue, vma);
+	mutex_unlock(&bcap_dev->mutex);
+	return ret;
 }
 
 #ifndef CONFIG_MMU
@@ -259,8 +264,12 @@ static unsigned long bcap_get_unmapped_area(struct file *file,
 static unsigned int bcap_poll(struct file *file, poll_table *wait)
 {
 	struct bcap_device *bcap_dev = video_drvdata(file);
+	unsigned int res;
 
-	return vb2_poll(&bcap_dev->buffer_queue, file, wait);
+	mutex_lock(&bcap_dev->mutex);
+	res = vb2_poll(&bcap_dev->buffer_queue, file, wait);
+	mutex_unlock(&bcap_dev->mutex);
+	return res;
 }
 
 static int bcap_queue_setup(struct vb2_queue *vq,
@@ -942,10 +951,6 @@ static int __devinit bcap_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&bcap_dev->dma_queue);
 
 	vfd->lock = &bcap_dev->mutex;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 
 	/* register video device */
 	ret = video_register_device(bcap_dev->video_dev, VFL_TYPE_GRABBER, -1);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 18/26] cx231xx: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (15 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 17/26] bfin_capture: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 19/26] soc_camera: " Hans Verkuil
                     ` (7 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/cx231xx/cx231xx-video.c |   47 ++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/drivers/media/video/cx231xx/cx231xx-video.c b/drivers/media/video/cx231xx/cx231xx-video.c
index 523aa49..790b28d 100644
--- a/drivers/media/video/cx231xx/cx231xx-video.c
+++ b/drivers/media/video/cx231xx/cx231xx-video.c
@@ -2168,6 +2168,10 @@ static int cx231xx_v4l2_open(struct file *filp)
 		cx231xx_errdev("cx231xx-video.c: Out of memory?!\n");
 		return -ENOMEM;
 	}
+	if (mutex_lock_interruptible(&dev->lock)) {
+		kfree(fh);
+		return -ERESTARTSYS;
+	}
 	fh->dev = dev;
 	fh->radio = radio;
 	fh->type = fh_type;
@@ -2226,6 +2230,7 @@ static int cx231xx_v4l2_open(struct file *filp)
 					    sizeof(struct cx231xx_buffer),
 					    fh, &dev->lock);
 	}
+	mutex_unlock(&dev->lock);
 
 	return errCode;
 }
@@ -2272,11 +2277,11 @@ void cx231xx_release_analog_resources(struct cx231xx *dev)
 }
 
 /*
- * cx231xx_v4l2_close()
+ * cx231xx_close()
  * stops streaming and deallocates all resources allocated by the v4l2
  * calls and ioctls
  */
-static int cx231xx_v4l2_close(struct file *filp)
+static int cx231xx_close(struct file *filp)
 {
 	struct cx231xx_fh *fh = filp->private_data;
 	struct cx231xx *dev = fh->dev;
@@ -2355,6 +2360,18 @@ static int cx231xx_v4l2_close(struct file *filp)
 	return 0;
 }
 
+static int cx231xx_v4l2_close(struct file *filp)
+{
+	struct cx231xx_fh *fh = filp->private_data;
+	struct cx231xx *dev = fh->dev;
+	int rc;
+
+	mutex_lock(&dev->lock);
+	rc = cx231xx_close(filp);
+	mutex_unlock(&dev->lock);
+	return rc;
+}
+
 /*
  * cx231xx_v4l2_read()
  * will allocate buffers when called for the first time
@@ -2378,8 +2395,12 @@ cx231xx_v4l2_read(struct file *filp, char __user *buf, size_t count,
 		if (unlikely(rc < 0))
 			return rc;
 
-		return videobuf_read_stream(&fh->vb_vidq, buf, count, pos, 0,
+		if (mutex_lock_interruptible(&dev->lock))
+			return -ERESTARTSYS;
+		rc = videobuf_read_stream(&fh->vb_vidq, buf, count, pos, 0,
 					    filp->f_flags & O_NONBLOCK);
+		mutex_unlock(&dev->lock);
+		return rc;
 	}
 	return 0;
 }
@@ -2404,10 +2425,15 @@ static unsigned int cx231xx_v4l2_poll(struct file *filp, poll_table *wait)
 		return POLLERR;
 
 	if ((V4L2_BUF_TYPE_VIDEO_CAPTURE == fh->type) ||
-	    (V4L2_BUF_TYPE_VBI_CAPTURE == fh->type))
-		return videobuf_poll_stream(filp, &fh->vb_vidq, wait);
-	else
-		return POLLERR;
+	    (V4L2_BUF_TYPE_VBI_CAPTURE == fh->type)) {
+		unsigned int res;
+
+		mutex_lock(&dev->lock);
+		res = videobuf_poll_stream(filp, &fh->vb_vidq, wait);
+		mutex_unlock(&dev->lock);
+		return res;
+	}
+	return POLLERR;
 }
 
 /*
@@ -2428,7 +2454,10 @@ static int cx231xx_v4l2_mmap(struct file *filp, struct vm_area_struct *vma)
 	if (unlikely(rc < 0))
 		return rc;
 
+	if (mutex_lock_interruptible(&dev->lock))
+		return -ERESTARTSYS;
 	rc = videobuf_mmap_mapper(&fh->vb_vidq, vma);
+	mutex_unlock(&dev->lock);
 
 	cx231xx_videodbg("vma start=0x%08lx, size=%ld, ret=%d\n",
 			 (unsigned long)vma->vm_start,
@@ -2545,10 +2574,6 @@ static struct video_device *cx231xx_vdev_init(struct cx231xx *dev,
 	vfd->release = video_device_release;
 	vfd->debug = video_debug;
 	vfd->lock = &dev->lock;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 
 	snprintf(vfd->name, sizeof(vfd->name), "%s %s", dev->name, type_name);
 
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 19/26] soc_camera: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (16 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 18/26] cx231xx: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-07-20  9:21     ` Guennadi Liakhovetski
  2012-06-24 11:26   ` [RFC PATCH 20/26] s5p-jpeg: " Hans Verkuil
                     ` (6 subsequent siblings)
  24 siblings, 1 reply; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/soc_camera.c |   31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 0421bf9..368b6fc 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -507,9 +507,12 @@ static int soc_camera_open(struct file *file)
 
 	ici = to_soc_camera_host(icd->parent);
 
+	if (mutex_lock_interruptible(&icd->video_lock))
+		return -ERESTARTSYS;
 	if (!try_module_get(ici->ops->owner)) {
 		dev_err(icd->pdev, "Couldn't lock capture bus driver.\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto emodule;
 	}
 
 	icd->use_count++;
@@ -570,6 +573,7 @@ static int soc_camera_open(struct file *file)
 		}
 		v4l2_ctrl_handler_setup(&icd->ctrl_handler);
 	}
+	mutex_unlock(&icd->video_lock);
 
 	file->private_data = icd;
 	dev_dbg(icd->pdev, "camera device open\n");
@@ -590,6 +594,8 @@ epower:
 eiciadd:
 	icd->use_count--;
 	module_put(ici->ops->owner);
+emodule:
+	mutex_unlock(&icd->video_lock);
 
 	return ret;
 }
@@ -599,6 +605,7 @@ static int soc_camera_close(struct file *file)
 	struct soc_camera_device *icd = file->private_data;
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 
+	mutex_lock(&icd->video_lock);
 	icd->use_count--;
 	if (!icd->use_count) {
 		struct soc_camera_link *icl = to_soc_camera_link(icd);
@@ -615,6 +622,7 @@ static int soc_camera_close(struct file *file)
 
 	if (icd->streamer == file)
 		icd->streamer = NULL;
+	mutex_unlock(&icd->video_lock);
 
 	module_put(ici->ops->owner);
 
@@ -645,10 +653,13 @@ static int soc_camera_mmap(struct file *file, struct vm_area_struct *vma)
 	if (icd->streamer != file)
 		return -EBUSY;
 
+	if (mutex_lock_interruptible(&icd->video_lock))
+		return -ERESTARTSYS;
 	if (ici->ops->init_videobuf)
 		err = videobuf_mmap_mapper(&icd->vb_vidq, vma);
 	else
 		err = vb2_mmap(&icd->vb2_vidq, vma);
+	mutex_unlock(&icd->video_lock);
 
 	dev_dbg(icd->pdev, "vma start=0x%08lx, size=%ld, ret=%d\n",
 		(unsigned long)vma->vm_start,
@@ -662,16 +673,18 @@ static unsigned int soc_camera_poll(struct file *file, poll_table *pt)
 {
 	struct soc_camera_device *icd = file->private_data;
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+	unsigned res = POLLERR;
 
 	if (icd->streamer != file)
-		return -EBUSY;
-
-	if (ici->ops->init_videobuf && list_empty(&icd->vb_vidq.stream)) {
-		dev_err(icd->pdev, "Trying to poll with no queued buffers!\n");
 		return POLLERR;
-	}
 
-	return ici->ops->poll(file, pt);
+	mutex_lock(&icd->video_lock);
+	if (ici->ops->init_videobuf && list_empty(&icd->vb_vidq.stream))
+		dev_err(icd->pdev, "Trying to poll with no queued buffers!\n");
+	else
+		res = ici->ops->poll(file, pt);
+	mutex_unlock(&icd->video_lock);
+	return res;
 }
 
 void soc_camera_lock(struct vb2_queue *vq)
@@ -1432,10 +1445,6 @@ static int video_dev_create(struct soc_camera_device *icd)
 	vdev->tvnorms		= V4L2_STD_UNKNOWN;
 	vdev->ctrl_handler	= &icd->ctrl_handler;
 	vdev->lock		= &icd->video_lock;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags);
 
 	icd->vdev = vdev;
 
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 20/26] s5p-jpeg: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (17 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 19/26] soc_camera: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 21/26] s5p-g2d: " Hans Verkuil
                     ` (5 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/s5p-jpeg/jpeg-core.c |   34 +++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/media/video/s5p-jpeg/jpeg-core.c b/drivers/media/video/s5p-jpeg/jpeg-core.c
index 28b5225d..ff0cc37 100644
--- a/drivers/media/video/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/video/s5p-jpeg/jpeg-core.c
@@ -292,6 +292,11 @@ static int s5p_jpeg_open(struct file *file)
 	if (!ctx)
 		return -ENOMEM;
 
+	if (mutex_lock_interruptible(&jpeg->lock)) {
+		ret = -ERESTARTSYS;
+		goto free;
+	}
+
 	v4l2_fh_init(&ctx->fh, vfd);
 	/* Use separate control handler per file handle */
 	ctx->fh.ctrl_handler = &ctx->ctrl_handler;
@@ -319,20 +324,26 @@ static int s5p_jpeg_open(struct file *file)
 
 	ctx->out_q.fmt = out_fmt;
 	ctx->cap_q.fmt = s5p_jpeg_find_format(ctx->mode, V4L2_PIX_FMT_YUYV);
+	mutex_unlock(&jpeg->lock);
 	return 0;
 
 error:
 	v4l2_fh_del(&ctx->fh);
 	v4l2_fh_exit(&ctx->fh);
+	mutex_unlock(&jpeg->lock);
+free:
 	kfree(ctx);
 	return ret;
 }
 
 static int s5p_jpeg_release(struct file *file)
 {
+	struct s5p_jpeg *jpeg = video_drvdata(file);
 	struct s5p_jpeg_ctx *ctx = fh_to_ctx(file->private_data);
 
+	mutex_lock(&jpeg->lock);
 	v4l2_m2m_ctx_release(ctx->m2m_ctx);
+	mutex_unlock(&jpeg->lock);
 	v4l2_ctrl_handler_free(&ctx->ctrl_handler);
 	v4l2_fh_del(&ctx->fh);
 	v4l2_fh_exit(&ctx->fh);
@@ -344,16 +355,27 @@ static int s5p_jpeg_release(struct file *file)
 static unsigned int s5p_jpeg_poll(struct file *file,
 				 struct poll_table_struct *wait)
 {
+	struct s5p_jpeg *jpeg = video_drvdata(file);
 	struct s5p_jpeg_ctx *ctx = fh_to_ctx(file->private_data);
+	unsigned int res;
 
-	return v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
+	mutex_lock(&jpeg->lock);
+	res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
+	mutex_unlock(&jpeg->lock);
+	return res;
 }
 
 static int s5p_jpeg_mmap(struct file *file, struct vm_area_struct *vma)
 {
+	struct s5p_jpeg *jpeg = video_drvdata(file);
 	struct s5p_jpeg_ctx *ctx = fh_to_ctx(file->private_data);
+	int ret;
 
-	return v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
+	if (mutex_lock_interruptible(&jpeg->lock))
+		return -ERESTARTSYS;
+	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
+	mutex_unlock(&jpeg->lock);
+	return ret;
 }
 
 static const struct v4l2_file_operations s5p_jpeg_fops = {
@@ -1368,10 +1390,6 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
 	jpeg->vfd_encoder->release	= video_device_release;
 	jpeg->vfd_encoder->lock		= &jpeg->lock;
 	jpeg->vfd_encoder->v4l2_dev	= &jpeg->v4l2_dev;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &jpeg->vfd_encoder->flags);
 
 	ret = video_register_device(jpeg->vfd_encoder, VFL_TYPE_GRABBER, -1);
 	if (ret) {
@@ -1399,10 +1417,6 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
 	jpeg->vfd_decoder->release	= video_device_release;
 	jpeg->vfd_decoder->lock		= &jpeg->lock;
 	jpeg->vfd_decoder->v4l2_dev	= &jpeg->v4l2_dev;
-	/* Locking in file operations other than ioctl should be done by the driver,
-	   not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &jpeg->vfd_decoder->flags);
 
 	ret = video_register_device(jpeg->vfd_decoder, VFL_TYPE_GRABBER, -1);
 	if (ret) {
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 21/26] s5p-g2d: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (18 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 20/26] s5p-jpeg: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-26  9:35     ` Kamil Debski
  2012-06-24 11:26   ` [RFC PATCH 22/26] s5p-tv: " Hans Verkuil
                     ` (4 subsequent siblings)
  24 siblings, 1 reply; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/s5p-g2d/g2d.c |   27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/s5p-g2d/g2d.c b/drivers/media/video/s5p-g2d/g2d.c
index 7c98ee7..d8cf1db 100644
--- a/drivers/media/video/s5p-g2d/g2d.c
+++ b/drivers/media/video/s5p-g2d/g2d.c
@@ -248,9 +248,14 @@ static int g2d_open(struct file *file)
 	ctx->in		= def_frame;
 	ctx->out	= def_frame;
 
+	if (mutex_lock_interruptible(&dev->mutex)) {
+		kfree(ctx);
+		return -ERESTARTSYS;
+	}
 	ctx->m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, ctx, &queue_init);
 	if (IS_ERR(ctx->m2m_ctx)) {
 		ret = PTR_ERR(ctx->m2m_ctx);
+		mutex_unlock(&dev->mutex);
 		kfree(ctx);
 		return ret;
 	}
@@ -264,6 +269,7 @@ static int g2d_open(struct file *file)
 	v4l2_ctrl_handler_setup(&ctx->ctrl_handler);
 
 	ctx->fh.ctrl_handler = &ctx->ctrl_handler;
+	mutex_unlock(&dev->mutex);
 
 	v4l2_info(&dev->v4l2_dev, "instance opened\n");
 	return 0;
@@ -401,13 +407,26 @@ static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 static unsigned int g2d_poll(struct file *file, struct poll_table_struct *wait)
 {
 	struct g2d_ctx *ctx = fh2ctx(file->private_data);
-	return v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
+	struct g2d_dev *dev = ctx->dev;
+	unsigned int res;
+
+	mutex_lock(&dev->mutex);
+	res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
+	mutex_unlock(&dev->mutex);
+	return res;
 }
 
 static int g2d_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct g2d_ctx *ctx = fh2ctx(file->private_data);
-	return v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
+	struct g2d_dev *dev = ctx->dev;
+	int ret;
+
+	if (mutex_lock_interruptible(&dev->mutex))
+		return -ERESTARTSYS;
+	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
+	mutex_unlock(&dev->mutex);
+	return ret;
 }
 
 static int vidioc_reqbufs(struct file *file, void *priv,
@@ -748,10 +767,6 @@ static int g2d_probe(struct platform_device *pdev)
 		goto unreg_v4l2_dev;
 	}
 	*vfd = g2d_videodev;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 	vfd->lock = &dev->mutex;
 	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
 	if (ret) {
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 22/26] s5p-tv: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (19 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 21/26] s5p-g2d: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 23/26] fimc-capture.c: " Hans Verkuil
                     ` (3 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/s5p-tv/mixer_video.c |   29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/s5p-tv/mixer_video.c b/drivers/media/video/s5p-tv/mixer_video.c
index 33fde2a..7a8880f 100644
--- a/drivers/media/video/s5p-tv/mixer_video.c
+++ b/drivers/media/video/s5p-tv/mixer_video.c
@@ -750,18 +750,20 @@ static int mxr_video_open(struct file *file)
 	int ret = 0;
 
 	mxr_dbg(mdev, "%s:%d\n", __func__, __LINE__);
+	if (mutex_lock_interruptible(&layer->mutex))
+		return -ERESTARTSYS;
 	/* assure device probe is finished */
 	wait_for_device_probe();
 	/* creating context for file descriptor */
 	ret = v4l2_fh_open(file);
 	if (ret) {
 		mxr_err(mdev, "v4l2_fh_open failed\n");
-		return ret;
+		goto unlock;
 	}
 
 	/* leaving if layer is already initialized */
 	if (!v4l2_fh_is_singular_file(file))
-		return 0;
+		goto unlock;
 
 	/* FIXME: should power be enabled on open? */
 	ret = mxr_power_get(mdev);
@@ -779,6 +781,7 @@ static int mxr_video_open(struct file *file)
 	layer->fmt = layer->fmt_array[0];
 	/* setup default geometry */
 	mxr_layer_default_geo(layer);
+	mutex_unlock(&layer->mutex);
 
 	return 0;
 
@@ -788,6 +791,9 @@ fail_power:
 fail_fh_open:
 	v4l2_fh_release(file);
 
+unlock:
+	mutex_unlock(&layer->mutex);
+
 	return ret;
 }
 
@@ -795,19 +801,28 @@ static unsigned int
 mxr_video_poll(struct file *file, struct poll_table_struct *wait)
 {
 	struct mxr_layer *layer = video_drvdata(file);
+	unsigned int res;
 
 	mxr_dbg(layer->mdev, "%s:%d\n", __func__, __LINE__);
 
-	return vb2_poll(&layer->vb_queue, file, wait);
+	mutex_lock(&layer->mutex);
+	res = vb2_poll(&layer->vb_queue, file, wait);
+	mutex_unlock(&layer->mutex);
+	return res;
 }
 
 static int mxr_video_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct mxr_layer *layer = video_drvdata(file);
+	int ret;
 
 	mxr_dbg(layer->mdev, "%s:%d\n", __func__, __LINE__);
 
-	return vb2_mmap(&layer->vb_queue, vma);
+	if (mutex_lock_interruptible(&layer->mutex))
+		return -ERESTARTSYS;
+	ret = vb2_mmap(&layer->vb_queue, vma);
+	mutex_unlock(&layer->mutex);
+	return ret;
 }
 
 static int mxr_video_release(struct file *file)
@@ -815,11 +830,13 @@ static int mxr_video_release(struct file *file)
 	struct mxr_layer *layer = video_drvdata(file);
 
 	mxr_dbg(layer->mdev, "%s:%d\n", __func__, __LINE__);
+	mutex_lock(&layer->mutex);
 	if (v4l2_fh_is_singular_file(file)) {
 		vb2_queue_release(&layer->vb_queue);
 		mxr_power_put(layer->mdev);
 	}
 	v4l2_fh_release(file);
+	mutex_unlock(&layer->mutex);
 	return 0;
 }
 
@@ -1069,10 +1086,6 @@ struct mxr_layer *mxr_base_layer_create(struct mxr_device *mdev,
 	set_bit(V4L2_FL_USE_FH_PRIO, &layer->vfd.flags);
 
 	video_set_drvdata(&layer->vfd, layer);
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &layer->vfd.flags);
 	layer->vfd.lock = &layer->mutex;
 	layer->vfd.v4l2_dev = &mdev->v4l2_dev;
 
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 23/26] fimc-capture.c: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (20 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 22/26] s5p-tv: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 24/26] fimc-m2m.c: " Hans Verkuil
                     ` (2 subsequent siblings)
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/s5p-fimc/fimc-capture.c |   36 +++++++++++++++++++--------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c b/drivers/media/video/s5p-fimc/fimc-capture.c
index 62ce539..62045dc 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -479,17 +479,22 @@ static int fimc_capture_set_default_format(struct fimc_dev *fimc);
 static int fimc_capture_open(struct file *file)
 {
 	struct fimc_dev *fimc = video_drvdata(file);
-	int ret = v4l2_fh_open(file);
+	int ret;
+
+	/* Return if the corresponding video mem2mem node is already opened. */
+	if (fimc_m2m_active(fimc))
+		return -EBUSY;
 
+	ret = v4l2_fh_open(file);
 	if (ret)
 		return ret;
 
 	dbg("pid: %d, state: 0x%lx", task_pid_nr(current), fimc->state);
 
-	/* Return if the corresponding video mem2mem node is already opened. */
-	if (fimc_m2m_active(fimc))
-		return -EBUSY;
-
+	if (mutex_lock_interruptible(&fimc->lock)) {
+		v4l2_fh_release(file);
+		return -ERESTARTSYS;
+	}
 	set_bit(ST_CAPT_BUSY, &fimc->state);
 	pm_runtime_get_sync(&fimc->pdev->dev);
 
@@ -503,6 +508,7 @@ static int fimc_capture_open(struct file *file)
 			fimc->vid_cap.refcnt--;
 			v4l2_fh_release(file);
 			clear_bit(ST_CAPT_BUSY, &fimc->state);
+			mutex_unlock(&fimc->lock);
 			return ret;
 		}
 		ret = fimc_capture_ctrls_create(fimc);
@@ -510,6 +516,7 @@ static int fimc_capture_open(struct file *file)
 		if (!ret && !fimc->vid_cap.user_subdev_api)
 			ret = fimc_capture_set_default_format(fimc);
 	}
+	mutex_unlock(&fimc->lock);
 	return ret;
 }
 
@@ -519,6 +526,7 @@ static int fimc_capture_close(struct file *file)
 
 	dbg("pid: %d, state: 0x%lx", task_pid_nr(current), fimc->state);
 
+	mutex_lock(&fimc->lock);
 	if (--fimc->vid_cap.refcnt == 0) {
 		clear_bit(ST_CAPT_BUSY, &fimc->state);
 		fimc_stop_capture(fimc, false);
@@ -532,6 +540,7 @@ static int fimc_capture_close(struct file *file)
 		vb2_queue_release(&fimc->vid_cap.vbq);
 		fimc_ctrls_delete(fimc->vid_cap.ctx);
 	}
+	mutex_unlock(&fimc->lock);
 	return v4l2_fh_release(file);
 }
 
@@ -539,15 +548,24 @@ static unsigned int fimc_capture_poll(struct file *file,
 				      struct poll_table_struct *wait)
 {
 	struct fimc_dev *fimc = video_drvdata(file);
+	unsigned res;
 
-	return vb2_poll(&fimc->vid_cap.vbq, file, wait);
+	mutex_lock(&fimc->lock);
+	res = vb2_poll(&fimc->vid_cap.vbq, file, wait);
+	mutex_unlock(&fimc->lock);
+	return res;
 }
 
 static int fimc_capture_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct fimc_dev *fimc = video_drvdata(file);
+	int ret;
 
-	return vb2_mmap(&fimc->vid_cap.vbq, vma);
+	if (mutex_lock_interruptible(&fimc->lock))
+		return -ERESTARTSYS;
+	ret = vb2_mmap(&fimc->vid_cap.vbq, vma);
+	mutex_unlock(&fimc->lock);
+	return ret;
 }
 
 static const struct v4l2_file_operations fimc_capture_fops = {
@@ -1590,10 +1608,6 @@ static int fimc_register_capture_device(struct fimc_dev *fimc,
 	vfd->minor	= -1;
 	vfd->release	= video_device_release;
 	vfd->lock	= &fimc->lock;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 	video_set_drvdata(vfd, fimc);
 
 	vid_cap = &fimc->vid_cap;
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 24/26] fimc-m2m.c: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (21 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 23/26] fimc-capture.c: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 25/26] s5p-mfc: " Hans Verkuil
  2012-06-24 11:26   ` [RFC PATCH 26/26] v4l2-dev: " Hans Verkuil
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/s5p-fimc/fimc-m2m.c |   28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/s5p-fimc/fimc-m2m.c b/drivers/media/video/s5p-fimc/fimc-m2m.c
index 4c58e05..1074397 100644
--- a/drivers/media/video/s5p-fimc/fimc-m2m.c
+++ b/drivers/media/video/s5p-fimc/fimc-m2m.c
@@ -660,6 +660,11 @@ static int fimc_m2m_open(struct file *file)
 	v4l2_fh_init(&ctx->fh, fimc->m2m.vfd);
 	ctx->fimc_dev = fimc;
 
+	if (mutex_lock_interruptible(&fimc->lock)) {
+		kfree(ctx);
+		return -ERESTARTSYS;
+	}
+
 	/* Default color format */
 	ctx->s_frame.fmt = fimc_get_format(0);
 	ctx->d_frame.fmt = fimc_get_format(0);
@@ -687,6 +692,7 @@ static int fimc_m2m_open(struct file *file)
 
 	if (fimc->m2m.refcnt++ == 0)
 		set_bit(ST_M2M_RUN, &fimc->state);
+	mutex_unlock(&fimc->lock);
 	return 0;
 
 error_c:
@@ -695,6 +701,7 @@ error_fh:
 	v4l2_fh_del(&ctx->fh);
 	v4l2_fh_exit(&ctx->fh);
 	kfree(ctx);
+	mutex_unlock(&fimc->lock);
 	return ret;
 }
 
@@ -706,6 +713,7 @@ static int fimc_m2m_release(struct file *file)
 	dbg("pid: %d, state: 0x%lx, refcnt= %d",
 		task_pid_nr(current), fimc->state, fimc->m2m.refcnt);
 
+	mutex_lock(&fimc->lock);
 	v4l2_m2m_ctx_release(ctx->m2m_ctx);
 	fimc_ctrls_delete(ctx);
 	v4l2_fh_del(&ctx->fh);
@@ -713,6 +721,7 @@ static int fimc_m2m_release(struct file *file)
 
 	if (--fimc->m2m.refcnt <= 0)
 		clear_bit(ST_M2M_RUN, &fimc->state);
+	mutex_unlock(&fimc->lock);
 	kfree(ctx);
 	return 0;
 }
@@ -721,16 +730,27 @@ static unsigned int fimc_m2m_poll(struct file *file,
 				  struct poll_table_struct *wait)
 {
 	struct fimc_ctx *ctx = fh_to_ctx(file->private_data);
+	struct fimc_dev *fimc = ctx->fimc_dev;
+	unsigned int res;
 
-	return v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
+	mutex_lock(&fimc->lock);
+	res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
+	mutex_unlock(&fimc->lock);
+	return res;
 }
 
 
 static int fimc_m2m_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct fimc_ctx *ctx = fh_to_ctx(file->private_data);
+	struct fimc_dev *fimc = ctx->fimc_dev;
+	int ret;
 
-	return v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
+	if (mutex_lock_interruptible(&fimc->lock))
+		return -ERESTARTSYS;
+	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
+	mutex_unlock(&fimc->lock);
+	return ret;
 }
 
 static const struct v4l2_file_operations fimc_m2m_fops = {
@@ -772,10 +792,6 @@ int fimc_register_m2m_device(struct fimc_dev *fimc,
 	vfd->minor = -1;
 	vfd->release = video_device_release;
 	vfd->lock = &fimc->lock;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 
 	snprintf(vfd->name, sizeof(vfd->name), "fimc.%d.m2m", fimc->id);
 	video_set_drvdata(vfd, fimc);
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 25/26] s5p-mfc: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (22 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 24/26] fimc-m2m.c: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  2012-06-26  9:35     ` Kamil Debski
  2012-06-24 11:26   ` [RFC PATCH 26/26] v4l2-dev: " Hans Verkuil
  24 siblings, 1 reply; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add proper locking to the file operations, allowing for the removal
of the V4L2_FL_LOCK_ALL_FOPS flag.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/s5p-mfc/s5p_mfc.c |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/s5p-mfc/s5p_mfc.c b/drivers/media/video/s5p-mfc/s5p_mfc.c
index 9bb68e7..e3e616d 100644
--- a/drivers/media/video/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/video/s5p-mfc/s5p_mfc.c
@@ -645,6 +645,8 @@ static int s5p_mfc_open(struct file *file)
 	int ret = 0;
 
 	mfc_debug_enter();
+	if (mutex_lock_interruptible(&dev->mfc_mutex))
+		return -ERESTARTSYS;
 	dev->num_inst++;	/* It is guarded by mfc_mutex in vfd */
 	/* Allocate memory for context */
 	ctx = kzalloc(sizeof *ctx, GFP_KERNEL);
@@ -765,6 +767,7 @@ static int s5p_mfc_open(struct file *file)
 		goto err_queue_init;
 	}
 	init_waitqueue_head(&ctx->queue);
+	mutex_unlock(&dev->mfc_mutex);
 	mfc_debug_leave();
 	return ret;
 	/* Deinit when failure occured */
@@ -790,6 +793,7 @@ err_no_ctx:
 	kfree(ctx);
 err_alloc:
 	dev->num_inst--;
+	mutex_unlock(&dev->mfc_mutex);
 	mfc_debug_leave();
 	return ret;
 }
@@ -802,6 +806,7 @@ static int s5p_mfc_release(struct file *file)
 	unsigned long flags;
 
 	mfc_debug_enter();
+	mutex_lock(&dev->mfc_mutex);
 	s5p_mfc_clock_on();
 	vb2_queue_release(&ctx->vq_src);
 	vb2_queue_release(&ctx->vq_dst);
@@ -855,6 +860,7 @@ static int s5p_mfc_release(struct file *file)
 	v4l2_fh_exit(&ctx->fh);
 	kfree(ctx);
 	mfc_debug_leave();
+	mutex_unlock(&dev->mfc_mutex);
 	return 0;
 }
 
@@ -869,6 +875,7 @@ static unsigned int s5p_mfc_poll(struct file *file,
 	unsigned int rc = 0;
 	unsigned long flags;
 
+	mutex_lock(&dev->mfc_mutex);
 	src_q = &ctx->vq_src;
 	dst_q = &ctx->vq_dst;
 	/*
@@ -902,6 +909,7 @@ static unsigned int s5p_mfc_poll(struct file *file,
 		rc |= POLLIN | POLLRDNORM;
 	spin_unlock_irqrestore(&dst_q->done_lock, flags);
 end:
+	mutex_unlock(&dev->mfc_mutex);
 	return rc;
 }
 
@@ -909,8 +917,12 @@ end:
 static int s5p_mfc_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct s5p_mfc_ctx *ctx = fh_to_ctx(file->private_data);
+	struct s5p_mfc_dev *dev = ctx->dev;
 	unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
 	int ret;
+
+	if (mutex_lock_interruptible(&dev->mfc_mutex))
+		return -ERESTARTSYS;
 	if (offset < DST_QUEUE_OFF_BASE) {
 		mfc_debug(2, "mmaping source\n");
 		ret = vb2_mmap(&ctx->vq_src, vma);
@@ -919,6 +931,7 @@ static int s5p_mfc_mmap(struct file *file, struct vm_area_struct *vma)
 		vma->vm_pgoff -= (DST_QUEUE_OFF_BASE >> PAGE_SHIFT);
 		ret = vb2_mmap(&ctx->vq_dst, vma);
 	}
+	mutex_unlock(&dev->mfc_mutex);
 	return ret;
 }
 
@@ -1034,10 +1047,6 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 	vfd->ioctl_ops	= get_dec_v4l2_ioctl_ops();
 	vfd->release	= video_device_release,
 	vfd->lock	= &dev->mfc_mutex;
-	/* Locking in file operations other than ioctl should be done
-	   by the driver, not the V4L2 core.
-	   This driver needs auditing so that this flag can be removed. */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 	vfd->v4l2_dev	= &dev->v4l2_dev;
 	snprintf(vfd->name, sizeof(vfd->name), "%s", S5P_MFC_DEC_NAME);
 	dev->vfd_dec	= vfd;
@@ -1062,8 +1071,6 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 	vfd->ioctl_ops	= get_enc_v4l2_ioctl_ops();
 	vfd->release	= video_device_release,
 	vfd->lock	= &dev->mfc_mutex;
-	/* This should not be necessary */
-	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
 	vfd->v4l2_dev	= &dev->v4l2_dev;
 	snprintf(vfd->name, sizeof(vfd->name), "%s", S5P_MFC_ENC_NAME);
 	dev->vfd_enc	= vfd;
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [RFC PATCH 26/26] v4l2-dev: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
                     ` (23 preceding siblings ...)
  2012-06-24 11:26   ` [RFC PATCH 25/26] s5p-mfc: " Hans Verkuil
@ 2012-06-24 11:26   ` Hans Verkuil
  24 siblings, 0 replies; 30+ messages in thread
From: Hans Verkuil @ 2012-06-24 11:26 UTC (permalink / raw)
  To: linux-media
  Cc: Andy Walls, Guennadi Liakhovetski, Mauro Carvalho Chehab,
	Scott Jiang, Manjunatha Halli, Manjunath Hadli,
	Anatolij Gustschin, Javier Martin, Sensoray Linux Development,
	Sylwester Nawrocki, Kamil Debski, Andrzej Pietrasiewicz,
	Sachin Kamat, Tomasz Stanislawski, mitov, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

All drivers that needed V4L2_FL_LOCK_ALL_FOPS have been converted,
so remove this flag altogether.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/v4l2-dev.c |   64 ++++++++--------------------------------
 include/media/v4l2-dev.h       |    3 --
 2 files changed, 12 insertions(+), 55 deletions(-)

diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 5ccbd46..3f55170 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -270,52 +270,35 @@ 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 = -ENODEV;
 
 	if (!vdev->fops->read)
 		return -EINVAL;
-	if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags) &&
-	    mutex_lock_interruptible(vdev->lock))
-		return -ERESTARTSYS;
 	if (video_is_registered(vdev))
-		ret = vdev->fops->read(filp, buf, sz, off);
-	if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
-		mutex_unlock(vdev->lock);
-	return ret;
+		return vdev->fops->read(filp, buf, sz, off);
+	return -ENODEV;
 }
 
 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 = -ENODEV;
 
 	if (!vdev->fops->write)
 		return -EINVAL;
-	if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags) &&
-	    mutex_lock_interruptible(vdev->lock))
-		return -ERESTARTSYS;
 	if (video_is_registered(vdev))
-		ret = vdev->fops->write(filp, buf, sz, off);
-	if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
-		mutex_unlock(vdev->lock);
-	return ret;
+		return vdev->fops->write(filp, buf, sz, off);
+	return -ENODEV;
 }
 
 static unsigned int v4l2_poll(struct file *filp, struct poll_table_struct *poll)
 {
 	struct video_device *vdev = video_devdata(filp);
-	int ret = POLLERR | POLLHUP;
 
 	if (!vdev->fops->poll)
 		return DEFAULT_POLLMASK;
-	if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
-		mutex_lock(vdev->lock);
 	if (video_is_registered(vdev))
-		ret = vdev->fops->poll(filp, poll);
-	if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
-		mutex_unlock(vdev->lock);
-	return ret;
+		return vdev->fops->poll(filp, poll);
+	return POLLERR | POLLHUP;
 }
 
 static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
@@ -397,18 +380,12 @@ static unsigned long v4l2_get_unmapped_area(struct file *filp,
 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)
-		return ret;
-	if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags) &&
-	    mutex_lock_interruptible(vdev->lock))
-		return -ERESTARTSYS;
+		return -ENODEV;
 	if (video_is_registered(vdev))
-		ret = vdev->fops->mmap(filp, vm);
-	if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
-		mutex_unlock(vdev->lock);
-	return ret;
+		return vdev->fops->mmap(filp, vm);
+	return -ENODEV;
 }
 
 /* Override for the open function */
@@ -429,20 +406,12 @@ static int v4l2_open(struct inode *inode, struct file *filp)
 	video_get(vdev);
 	mutex_unlock(&videodev_lock);
 	if (vdev->fops->open) {
-		if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags) &&
-		    mutex_lock_interruptible(vdev->lock)) {
-			ret = -ERESTARTSYS;
-			goto err;
-		}
 		if (video_is_registered(vdev))
 			ret = vdev->fops->open(filp);
 		else
 			ret = -ENODEV;
-		if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
-			mutex_unlock(vdev->lock);
 	}
 
-err:
 	/* decrease the refcount in case of an error */
 	if (ret)
 		video_put(vdev);
@@ -453,19 +422,14 @@ err:
 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 (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
-			mutex_lock(vdev->lock);
+	if (vdev->fops->release)
 		vdev->fops->release(filp);
-		if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
-			mutex_unlock(vdev->lock);
-	}
+
 	/* decrease the refcount unconditionally since the release()
 	   return value is ignored. */
 	video_put(vdev);
-	return ret;
+	return 0;
 }
 
 static const struct file_operations v4l2_fops = {
@@ -835,10 +799,6 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
 	WARN_ON(video_device[vdev->minor] != NULL);
 	vdev->index = get_index(vdev);
 	mutex_unlock(&videodev_lock);
-	/* if no lock was passed, then make sure the LOCK_ALL_FOPS bit is
-	   clear and warn if it wasn't. */
-	if (vdev->lock == NULL)
-		WARN_ON(test_and_clear_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags));
 
 	if (vdev->ioctl_ops)
 		determine_valid_ioctls(vdev);
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index a056e6e..08aca24 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -39,9 +39,6 @@ struct v4l2_ctrl_handler;
 #define V4L2_FL_USES_V4L2_FH	(1)
 /* Use the prio field of v4l2_fh for core priority checking */
 #define V4L2_FL_USE_FH_PRIO	(2)
-/* If ioctl core locking is in use, then apply that also to all
-   file operations. Don't use this flag in new drivers! */
-#define V4L2_FL_LOCK_ALL_FOPS	(3)
 
 /* Priority helper functions */
 
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* RE: [RFC PATCH 25/26] s5p-mfc: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:26   ` [RFC PATCH 25/26] s5p-mfc: " Hans Verkuil
@ 2012-06-26  9:35     ` Kamil Debski
  0 siblings, 0 replies; 30+ messages in thread
From: Kamil Debski @ 2012-06-26  9:35 UTC (permalink / raw)
  To: 'Hans Verkuil', linux-media
  Cc: 'Andy Walls', 'Guennadi Liakhovetski',
	'Mauro Carvalho Chehab', 'Scott Jiang',
	'Manjunatha Halli', 'Manjunath Hadli',
	'Anatolij Gustschin', 'Javier Martin',
	'Sensoray Linux Development',
	Sylwester Nawrocki, Andrzej Pietrasiewicz, 'Sachin Kamat',
	Tomasz Stanislawski, mitov, 'Hans Verkuil'

Hi Hans,

Thank you for your patch. I have tested it on our hardware and MFC works.

Best wishes,
--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center

> -----Original Message-----
> From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
> Sent: 24 June 2012 13:26
> To: linux-media@vger.kernel.org
> Cc: Andy Walls; Guennadi Liakhovetski; Mauro Carvalho Chehab; Scott Jiang;
> Manjunatha Halli; Manjunath Hadli; Anatolij Gustschin; Javier Martin;
> Sensoray Linux Development; Sylwester Nawrocki; Kamil Debski; Andrzej
> Pietrasiewicz; Sachin Kamat; Tomasz Stanislawski; mitov@issp.bas.bg; Hans
> Verkuil
> Subject: [RFC PATCH 25/26] s5p-mfc: remove V4L2_FL_LOCK_ALL_FOPS
> 
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Add proper locking to the file operations, allowing for the removal
> of the V4L2_FL_LOCK_ALL_FOPS flag.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Kamil Debski <k.debski@samsung.com>

> ---
>  drivers/media/video/s5p-mfc/s5p_mfc.c |   19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/video/s5p-mfc/s5p_mfc.c
> b/drivers/media/video/s5p-mfc/s5p_mfc.c
> index 9bb68e7..e3e616d 100644
> --- a/drivers/media/video/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/video/s5p-mfc/s5p_mfc.c
> @@ -645,6 +645,8 @@ static int s5p_mfc_open(struct file *file)
>  	int ret = 0;
> 
>  	mfc_debug_enter();
> +	if (mutex_lock_interruptible(&dev->mfc_mutex))
> +		return -ERESTARTSYS;
>  	dev->num_inst++;	/* It is guarded by mfc_mutex in vfd */
>  	/* Allocate memory for context */
>  	ctx = kzalloc(sizeof *ctx, GFP_KERNEL);
> @@ -765,6 +767,7 @@ static int s5p_mfc_open(struct file *file)
>  		goto err_queue_init;
>  	}
>  	init_waitqueue_head(&ctx->queue);
> +	mutex_unlock(&dev->mfc_mutex);
>  	mfc_debug_leave();
>  	return ret;
>  	/* Deinit when failure occured */
> @@ -790,6 +793,7 @@ err_no_ctx:
>  	kfree(ctx);
>  err_alloc:
>  	dev->num_inst--;
> +	mutex_unlock(&dev->mfc_mutex);
>  	mfc_debug_leave();
>  	return ret;
>  }
> @@ -802,6 +806,7 @@ static int s5p_mfc_release(struct file *file)
>  	unsigned long flags;
> 
>  	mfc_debug_enter();
> +	mutex_lock(&dev->mfc_mutex);
>  	s5p_mfc_clock_on();
>  	vb2_queue_release(&ctx->vq_src);
>  	vb2_queue_release(&ctx->vq_dst);
> @@ -855,6 +860,7 @@ static int s5p_mfc_release(struct file *file)
>  	v4l2_fh_exit(&ctx->fh);
>  	kfree(ctx);
>  	mfc_debug_leave();
> +	mutex_unlock(&dev->mfc_mutex);
>  	return 0;
>  }
> 
> @@ -869,6 +875,7 @@ static unsigned int s5p_mfc_poll(struct file *file,
>  	unsigned int rc = 0;
>  	unsigned long flags;
> 
> +	mutex_lock(&dev->mfc_mutex);
>  	src_q = &ctx->vq_src;
>  	dst_q = &ctx->vq_dst;
>  	/*
> @@ -902,6 +909,7 @@ static unsigned int s5p_mfc_poll(struct file *file,
>  		rc |= POLLIN | POLLRDNORM;
>  	spin_unlock_irqrestore(&dst_q->done_lock, flags);
>  end:
> +	mutex_unlock(&dev->mfc_mutex);
>  	return rc;
>  }
> 
> @@ -909,8 +917,12 @@ end:
>  static int s5p_mfc_mmap(struct file *file, struct vm_area_struct *vma)
>  {
>  	struct s5p_mfc_ctx *ctx = fh_to_ctx(file->private_data);
> +	struct s5p_mfc_dev *dev = ctx->dev;
>  	unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
>  	int ret;
> +
> +	if (mutex_lock_interruptible(&dev->mfc_mutex))
> +		return -ERESTARTSYS;
>  	if (offset < DST_QUEUE_OFF_BASE) {
>  		mfc_debug(2, "mmaping source\n");
>  		ret = vb2_mmap(&ctx->vq_src, vma);
> @@ -919,6 +931,7 @@ static int s5p_mfc_mmap(struct file *file, struct
> vm_area_struct *vma)
>  		vma->vm_pgoff -= (DST_QUEUE_OFF_BASE >> PAGE_SHIFT);
>  		ret = vb2_mmap(&ctx->vq_dst, vma);
>  	}
> +	mutex_unlock(&dev->mfc_mutex);
>  	return ret;
>  }
> 
> @@ -1034,10 +1047,6 @@ static int s5p_mfc_probe(struct platform_device
> *pdev)
>  	vfd->ioctl_ops	= get_dec_v4l2_ioctl_ops();
>  	vfd->release	= video_device_release,
>  	vfd->lock	= &dev->mfc_mutex;
> -	/* Locking in file operations other than ioctl should be done
> -	   by the driver, not the V4L2 core.
> -	   This driver needs auditing so that this flag can be removed. */
> -	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
>  	vfd->v4l2_dev	= &dev->v4l2_dev;
>  	snprintf(vfd->name, sizeof(vfd->name), "%s", S5P_MFC_DEC_NAME);
>  	dev->vfd_dec	= vfd;
> @@ -1062,8 +1071,6 @@ static int s5p_mfc_probe(struct platform_device
> *pdev)
>  	vfd->ioctl_ops	= get_enc_v4l2_ioctl_ops();
>  	vfd->release	= video_device_release,
>  	vfd->lock	= &dev->mfc_mutex;
> -	/* This should not be necessary */
> -	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
>  	vfd->v4l2_dev	= &dev->v4l2_dev;
>  	snprintf(vfd->name, sizeof(vfd->name), "%s", S5P_MFC_ENC_NAME);
>  	dev->vfd_enc	= vfd;
> --
> 1.7.10


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [RFC PATCH 21/26] s5p-g2d: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:26   ` [RFC PATCH 21/26] s5p-g2d: " Hans Verkuil
@ 2012-06-26  9:35     ` Kamil Debski
  0 siblings, 0 replies; 30+ messages in thread
From: Kamil Debski @ 2012-06-26  9:35 UTC (permalink / raw)
  To: 'Hans Verkuil', linux-media
  Cc: 'Andy Walls', 'Guennadi Liakhovetski',
	'Mauro Carvalho Chehab', 'Scott Jiang',
	'Manjunatha Halli', 'Manjunath Hadli',
	'Anatolij Gustschin', 'Javier Martin',
	'Sensoray Linux Development',
	Sylwester Nawrocki, Andrzej Pietrasiewicz, 'Sachin Kamat',
	Tomasz Stanislawski, mitov, 'Hans Verkuil'

Hi Hans,

Thank you for your patch. I have tested it on our hardware and G2D works.

Best wishes,
--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center


> -----Original Message-----
> From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
> Sent: 24 June 2012 13:26
> To: linux-media@vger.kernel.org
> Cc: Andy Walls; Guennadi Liakhovetski; Mauro Carvalho Chehab; Scott Jiang;
> Manjunatha Halli; Manjunath Hadli; Anatolij Gustschin; Javier Martin;
> Sensoray Linux Development; Sylwester Nawrocki; Kamil Debski; Andrzej
> Pietrasiewicz; Sachin Kamat; Tomasz Stanislawski; mitov@issp.bas.bg; Hans
> Verkuil
> Subject: [RFC PATCH 21/26] s5p-g2d: remove V4L2_FL_LOCK_ALL_FOPS
> 
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Add proper locking to the file operations, allowing for the removal
> of the V4L2_FL_LOCK_ALL_FOPS flag.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Kamil Debski <k.debski@samsung.com>

> ---
>  drivers/media/video/s5p-g2d/g2d.c |   27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/video/s5p-g2d/g2d.c b/drivers/media/video/s5p-
> g2d/g2d.c
> index 7c98ee7..d8cf1db 100644
> --- a/drivers/media/video/s5p-g2d/g2d.c
> +++ b/drivers/media/video/s5p-g2d/g2d.c
> @@ -248,9 +248,14 @@ static int g2d_open(struct file *file)
>  	ctx->in		= def_frame;
>  	ctx->out	= def_frame;
> 
> +	if (mutex_lock_interruptible(&dev->mutex)) {
> +		kfree(ctx);
> +		return -ERESTARTSYS;
> +	}
>  	ctx->m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, ctx, &queue_init);
>  	if (IS_ERR(ctx->m2m_ctx)) {
>  		ret = PTR_ERR(ctx->m2m_ctx);
> +		mutex_unlock(&dev->mutex);
>  		kfree(ctx);
>  		return ret;
>  	}
> @@ -264,6 +269,7 @@ static int g2d_open(struct file *file)
>  	v4l2_ctrl_handler_setup(&ctx->ctrl_handler);
> 
>  	ctx->fh.ctrl_handler = &ctx->ctrl_handler;
> +	mutex_unlock(&dev->mutex);
> 
>  	v4l2_info(&dev->v4l2_dev, "instance opened\n");
>  	return 0;
> @@ -401,13 +407,26 @@ static int vidioc_s_fmt(struct file *file, void
> *prv, struct v4l2_format *f)
>  static unsigned int g2d_poll(struct file *file, struct poll_table_struct
> *wait)
>  {
>  	struct g2d_ctx *ctx = fh2ctx(file->private_data);
> -	return v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
> +	struct g2d_dev *dev = ctx->dev;
> +	unsigned int res;
> +
> +	mutex_lock(&dev->mutex);
> +	res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
> +	mutex_unlock(&dev->mutex);
> +	return res;
>  }
> 
>  static int g2d_mmap(struct file *file, struct vm_area_struct *vma)
>  {
>  	struct g2d_ctx *ctx = fh2ctx(file->private_data);
> -	return v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
> +	struct g2d_dev *dev = ctx->dev;
> +	int ret;
> +
> +	if (mutex_lock_interruptible(&dev->mutex))
> +		return -ERESTARTSYS;
> +	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
> +	mutex_unlock(&dev->mutex);
> +	return ret;
>  }
> 
>  static int vidioc_reqbufs(struct file *file, void *priv,
> @@ -748,10 +767,6 @@ static int g2d_probe(struct platform_device *pdev)
>  		goto unreg_v4l2_dev;
>  	}
>  	*vfd = g2d_videodev;
> -	/* Locking in file operations other than ioctl should be done
> -	   by the driver, not the V4L2 core.
> -	   This driver needs auditing so that this flag can be removed. */
> -	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
>  	vfd->lock = &dev->mutex;
>  	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
>  	if (ret) {
> --
> 1.7.10


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC PATCH 19/26] soc_camera: remove V4L2_FL_LOCK_ALL_FOPS
  2012-06-24 11:26   ` [RFC PATCH 19/26] soc_camera: " Hans Verkuil
@ 2012-07-20  9:21     ` Guennadi Liakhovetski
  0 siblings, 0 replies; 30+ messages in thread
From: Guennadi Liakhovetski @ 2012-07-20  9:21 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, Andy Walls, Mauro Carvalho Chehab, Scott Jiang,
	Manjunatha Halli, Manjunath Hadli, Anatolij Gustschin,
	Javier Martin, Sensoray Linux Development, Sylwester Nawrocki,
	Kamil Debski, Andrzej Pietrasiewicz, Sachin Kamat,
	Tomasz Stanislawski, mitov, Hans Verkuil

Hi Hans

Thanks for the patch. I guess, you'll want to pull it via your queue. Let 
me know if for some reason you'd prefer me to take it.

On Sun, 24 Jun 2012, Hans Verkuil wrote:

> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Add proper locking to the file operations, allowing for the removal
> of the V4L2_FL_LOCK_ALL_FOPS flag.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Thanks
Guennadi

> ---
>  drivers/media/video/soc_camera.c |   31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
> index 0421bf9..368b6fc 100644
> --- a/drivers/media/video/soc_camera.c
> +++ b/drivers/media/video/soc_camera.c
> @@ -507,9 +507,12 @@ static int soc_camera_open(struct file *file)
>  
>  	ici = to_soc_camera_host(icd->parent);
>  
> +	if (mutex_lock_interruptible(&icd->video_lock))
> +		return -ERESTARTSYS;
>  	if (!try_module_get(ici->ops->owner)) {
>  		dev_err(icd->pdev, "Couldn't lock capture bus driver.\n");
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto emodule;
>  	}
>  
>  	icd->use_count++;
> @@ -570,6 +573,7 @@ static int soc_camera_open(struct file *file)
>  		}
>  		v4l2_ctrl_handler_setup(&icd->ctrl_handler);
>  	}
> +	mutex_unlock(&icd->video_lock);
>  
>  	file->private_data = icd;
>  	dev_dbg(icd->pdev, "camera device open\n");
> @@ -590,6 +594,8 @@ epower:
>  eiciadd:
>  	icd->use_count--;
>  	module_put(ici->ops->owner);
> +emodule:
> +	mutex_unlock(&icd->video_lock);
>  
>  	return ret;
>  }
> @@ -599,6 +605,7 @@ static int soc_camera_close(struct file *file)
>  	struct soc_camera_device *icd = file->private_data;
>  	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
>  
> +	mutex_lock(&icd->video_lock);
>  	icd->use_count--;
>  	if (!icd->use_count) {
>  		struct soc_camera_link *icl = to_soc_camera_link(icd);
> @@ -615,6 +622,7 @@ static int soc_camera_close(struct file *file)
>  
>  	if (icd->streamer == file)
>  		icd->streamer = NULL;
> +	mutex_unlock(&icd->video_lock);
>  
>  	module_put(ici->ops->owner);
>  
> @@ -645,10 +653,13 @@ static int soc_camera_mmap(struct file *file, struct vm_area_struct *vma)
>  	if (icd->streamer != file)
>  		return -EBUSY;
>  
> +	if (mutex_lock_interruptible(&icd->video_lock))
> +		return -ERESTARTSYS;
>  	if (ici->ops->init_videobuf)
>  		err = videobuf_mmap_mapper(&icd->vb_vidq, vma);
>  	else
>  		err = vb2_mmap(&icd->vb2_vidq, vma);
> +	mutex_unlock(&icd->video_lock);
>  
>  	dev_dbg(icd->pdev, "vma start=0x%08lx, size=%ld, ret=%d\n",
>  		(unsigned long)vma->vm_start,
> @@ -662,16 +673,18 @@ static unsigned int soc_camera_poll(struct file *file, poll_table *pt)
>  {
>  	struct soc_camera_device *icd = file->private_data;
>  	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
> +	unsigned res = POLLERR;
>  
>  	if (icd->streamer != file)
> -		return -EBUSY;
> -
> -	if (ici->ops->init_videobuf && list_empty(&icd->vb_vidq.stream)) {
> -		dev_err(icd->pdev, "Trying to poll with no queued buffers!\n");
>  		return POLLERR;
> -	}
>  
> -	return ici->ops->poll(file, pt);
> +	mutex_lock(&icd->video_lock);
> +	if (ici->ops->init_videobuf && list_empty(&icd->vb_vidq.stream))
> +		dev_err(icd->pdev, "Trying to poll with no queued buffers!\n");
> +	else
> +		res = ici->ops->poll(file, pt);
> +	mutex_unlock(&icd->video_lock);
> +	return res;
>  }
>  
>  void soc_camera_lock(struct vb2_queue *vq)
> @@ -1432,10 +1445,6 @@ static int video_dev_create(struct soc_camera_device *icd)
>  	vdev->tvnorms		= V4L2_STD_UNKNOWN;
>  	vdev->ctrl_handler	= &icd->ctrl_handler;
>  	vdev->lock		= &icd->video_lock;
> -	/* Locking in file operations other than ioctl should be done
> -	   by the driver, not the V4L2 core.
> -	   This driver needs auditing so that this flag can be removed. */
> -	set_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags);
>  
>  	icd->vdev = vdev;
>  
> -- 
> 1.7.10
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2012-07-20  9:22 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-24 11:25 [RFC PATCH 00/26] Remove the V4L2_FL_LOCK_ALL_FOPS flag Hans Verkuil
2012-06-24 11:25 ` [RFC PATCH 01/26] ivtv: remove V4L2_FL_LOCK_ALL_FOPS Hans Verkuil
2012-06-24 11:25   ` [RFC PATCH 02/26] saa7146: " Hans Verkuil
2012-06-24 11:25   ` [RFC PATCH 03/26] cpia2: " Hans Verkuil
2012-06-24 11:25   ` [RFC PATCH 04/26] usbvision: " Hans Verkuil
2012-06-24 11:25   ` [RFC PATCH 05/26] em28xx: " Hans Verkuil
2012-06-24 11:25   ` [RFC PATCH 06/26] tm6000: " Hans Verkuil
2012-06-24 11:25   ` [RFC PATCH 07/26] mem2mem_testdev: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 08/26] dt3155v4l: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 09/26] wl128x: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 10/26] fsl-viu: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 11/26] s2255drv: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 12/26] vpbe_display: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 13/26] vpif_capture: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 14/26] vpif_display: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 15/26] mx2_emmaprp: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 16/26] sh_vou: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 17/26] bfin_capture: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 18/26] cx231xx: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 19/26] soc_camera: " Hans Verkuil
2012-07-20  9:21     ` Guennadi Liakhovetski
2012-06-24 11:26   ` [RFC PATCH 20/26] s5p-jpeg: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 21/26] s5p-g2d: " Hans Verkuil
2012-06-26  9:35     ` Kamil Debski
2012-06-24 11:26   ` [RFC PATCH 22/26] s5p-tv: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 23/26] fimc-capture.c: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 24/26] fimc-m2m.c: " Hans Verkuil
2012-06-24 11:26   ` [RFC PATCH 25/26] s5p-mfc: " Hans Verkuil
2012-06-26  9:35     ` Kamil Debski
2012-06-24 11:26   ` [RFC PATCH 26/26] v4l2-dev: " Hans Verkuil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.