Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 879aa24d authored by Hans Verkuil's avatar Hans Verkuil Committed by Mauro Carvalho Chehab
Browse files

[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: default avatarHans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent 2877842d

drivers/media/video/v4l2-dev.c

+28 −3
Original line number Diff line number Diff line
@@ -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;

drivers/media/video/v4l2-device.c

+1 −0
Original line number Diff line number Diff line
@@ -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 */

include/media/v4l2-device.h

+2 −0
Original line number Diff line number Diff line
@@ -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.