[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 bfd392e..6b64fd6 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -245,13 +245,38 @@
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 0b08f96..7fe6f92 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -35,6 +35,7 @@
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 6648036..b16f307 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -51,6 +51,8 @@
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.