From: Hans Verkuil <hverkuil@xs4all.nl>
Date: Fri, 26 Nov 2010 09:47:28 +0000 (-0300)
Subject: [media] V4L: improve the BKL replacement heuristic
X-Git-Tag: firefly_0821_release~7613^2~3227^2~6
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=879aa24d6394aa04b690a600a41ff500441ad384;p=firefly-linux-kernel-4.4.55.git

[media] V4L: improve the BKL replacement heuristic

The BKL replacement mutex had some serious performance side-effects on
V4L drivers. It is replaced by a better heuristic that works around the
worst of the side-effects.

Read the v4l2-dev.c comments for the whole sorry story. This is a
temporary measure only until we can convert all v4l drivers to use
unlocked_ioctl.

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

diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index bfd392e2436d..6b64fd607b20 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -245,13 +245,38 @@ static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		if (vdev->lock)
 			mutex_unlock(vdev->lock);
 	} else if (vdev->fops->ioctl) {
-		/* TODO: convert all drivers to unlocked_ioctl */
+		/* This code path is a replacement for the BKL. It is a major
+		 * hack but it will have to do for those drivers that are not
+		 * yet converted to use unlocked_ioctl.
+		 *
+		 * There are two options: if the driver implements struct
+		 * v4l2_device, then the lock defined there is used to
+		 * serialize the ioctls. Otherwise the v4l2 core lock defined
+		 * below is used. This lock is really bad since it serializes
+		 * completely independent devices.
+		 *
+		 * Both variants suffer from the same problem: if the driver
+		 * sleeps, then it blocks all ioctls since the lock is still
+		 * held. This is very common for VIDIOC_DQBUF since that
+		 * normally waits for a frame to arrive. As a result any other
+		 * ioctl calls will proceed very, very slowly since each call
+		 * will have to wait for the VIDIOC_QBUF to finish. Things that
+		 * should take 0.01s may now take 10-20 seconds.
+		 *
+		 * The workaround is to *not* take the lock for VIDIOC_DQBUF.
+		 * This actually works OK for videobuf-based drivers, since
+		 * videobuf will take its own internal lock.
+		 */
 		static DEFINE_MUTEX(v4l2_ioctl_mutex);
+		struct mutex *m = vdev->v4l2_dev ?
+			&vdev->v4l2_dev->ioctl_lock : &v4l2_ioctl_mutex;
 
-		mutex_lock(&v4l2_ioctl_mutex);
+		if (cmd != VIDIOC_DQBUF && mutex_lock_interruptible(m))
+			return -ERESTARTSYS;
 		if (video_is_registered(vdev))
 			ret = vdev->fops->ioctl(filp, cmd, arg);
-		mutex_unlock(&v4l2_ioctl_mutex);
+		if (cmd != VIDIOC_DQBUF)
+			mutex_unlock(m);
 	} else
 		ret = -ENOTTY;
 
diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c
index 0b08f96b74a5..7fe6f92af480 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -35,6 +35,7 @@ int v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev)
 
 	INIT_LIST_HEAD(&v4l2_dev->subdevs);
 	spin_lock_init(&v4l2_dev->lock);
+	mutex_init(&v4l2_dev->ioctl_lock);
 	v4l2_dev->dev = dev;
 	if (dev == NULL) {
 		/* If dev == NULL, then name must be filled in by the caller */
diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
index 6648036b728d..b16f307d471a 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -51,6 +51,8 @@ struct v4l2_device {
 			unsigned int notification, void *arg);
 	/* The control handler. May be NULL. */
 	struct v4l2_ctrl_handler *ctrl_handler;
+	/* BKL replacement mutex. Temporary solution only. */
+	struct mutex ioctl_lock;
 };
 
 /* Initialize v4l2_dev and make dev->driver_data point to v4l2_dev.