From patchwork Tue Nov 16 21:56:45 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans Verkuil X-Patchwork-Id: 329871 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id oAGLv3lI005871 for ; Tue, 16 Nov 2010 21:57:09 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932812Ab0KPV4u (ORCPT ); Tue, 16 Nov 2010 16:56:50 -0500 Received: from smtp-vbr4.xs4all.nl ([194.109.24.24]:1291 "EHLO smtp-vbr4.xs4all.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932723Ab0KPV4u (ORCPT ); Tue, 16 Nov 2010 16:56:50 -0500 Received: from localhost (209.80-203-30.nextgentel.com [80.203.30.209]) by smtp-vbr4.xs4all.nl (8.13.8/8.13.8) with ESMTP id oAGLuldu015949 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 16 Nov 2010 22:56:48 +0100 (CET) (envelope-from hverkuil@xs4all.nl) Message-Id: In-Reply-To: References: From: Hans Verkuil Date: Tue, 16 Nov 2010 22:56:45 +0100 Subject: [RFCv2 PATCH 14/15] V4L: improve the BKL replacement heuristic To: linux-media@vger.kernel.org Cc: Arnd Bergmann X-Virus-Scanned: by XS4ALL Virus Scanner Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.3 (demeter1.kernel.org [140.211.167.41]); Tue, 16 Nov 2010 21:57:09 +0000 (UTC) diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c index 8eb0756..59ef642 100644 --- a/drivers/media/video/v4l2-dev.c +++ b/drivers/media/video/v4l2-dev.c @@ -258,11 +258,42 @@ 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 */ - lock_kernel(); + /* 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; + + if (cmd != VIDIOC_DQBUF) { + int res = mutex_lock_interruptible(m); + + if (res) + return res; + } if (video_is_registered(vdev)) ret = vdev->fops->ioctl(filp, cmd, arg); - unlock_kernel(); + 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 @@ 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-dev.h b/include/media/v4l2-dev.h index 15802a0..59dec5a 100644 --- a/include/media/v4l2-dev.h +++ b/include/media/v4l2-dev.h @@ -39,7 +39,7 @@ struct v4l2_file_operations { ssize_t (*read) (struct file *, char __user *, size_t, loff_t *); ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *); unsigned int (*poll) (struct file *, struct poll_table_struct *); - long (*ioctl) (struct file *, unsigned int, unsigned long); + long (*ioctl __deprecated) (struct file *, unsigned int, unsigned long); long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long); int (*mmap) (struct file *, struct vm_area_struct *); int (*open) (struct file *); 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 @@ 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.