Commit 0edf2e5e authored by Arnd Bergmann's avatar Arnd Bergmann Committed by Mauro Carvalho Chehab

[media] v4l: kill the BKL

All of the hard problems for BKL removal appear to be solved in the
v4l-dvb/master tree. This removes the BKL from the various open
functions that do not need it, or only use it to protect an
open count.

The zoran driver is nontrivial in this regard, so I introduce
a new mutex that locks both the open/release and the ioctl
functions. Someone with access to the hardware can probably
improve that by using the existing lock in all cases.

Finally, all drivers that still use the locked version of the
ioctl function now get called under a new mutex instead of
the BKL.
Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent 2c2742da
...@@ -19,7 +19,6 @@ comment "Multimedia core support" ...@@ -19,7 +19,6 @@ comment "Multimedia core support"
config VIDEO_DEV config VIDEO_DEV
tristate "Video For Linux" tristate "Video For Linux"
depends on BKL # used in many drivers for ioctl handling, need to kill
---help--- ---help---
V4L core support for video capture and overlay devices, webcams and V4L core support for video capture and overlay devices, webcams and
AM/FM radio cards. AM/FM radio cards.
......
...@@ -31,7 +31,6 @@ ...@@ -31,7 +31,6 @@
#include <linux/delay.h> #include <linux/delay.h>
#include <linux/device.h> #include <linux/device.h>
#include <linux/firmware.h> #include <linux/firmware.h>
#include <linux/smp_lock.h>
#include <linux/vmalloc.h> #include <linux/vmalloc.h>
#include <media/v4l2-common.h> #include <media/v4l2-common.h>
#include <media/v4l2-ioctl.h> #include <media/v4l2-ioctl.h>
...@@ -1927,10 +1926,9 @@ static int mpeg_open(struct file *file) ...@@ -1927,10 +1926,9 @@ static int mpeg_open(struct file *file)
dev = h; dev = h;
} }
if (dev == NULL) { if (dev == NULL)
unlock_kernel();
return -ENODEV; return -ENODEV;
}
mutex_lock(&dev->lock); mutex_lock(&dev->lock);
/* allocate + initialize per filehandle data */ /* allocate + initialize per filehandle data */
......
...@@ -31,7 +31,6 @@ ...@@ -31,7 +31,6 @@
#include <linux/delay.h> #include <linux/delay.h>
#include <linux/device.h> #include <linux/device.h>
#include <linux/firmware.h> #include <linux/firmware.h>
#include <linux/smp_lock.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <media/v4l2-common.h> #include <media/v4l2-common.h>
#include <media/v4l2-ioctl.h> #include <media/v4l2-ioctl.h>
...@@ -1576,12 +1575,8 @@ static int mpeg_open(struct file *file) ...@@ -1576,12 +1575,8 @@ static int mpeg_open(struct file *file)
/* allocate + initialize per filehandle data */ /* allocate + initialize per filehandle data */
fh = kzalloc(sizeof(*fh), GFP_KERNEL); fh = kzalloc(sizeof(*fh), GFP_KERNEL);
if (NULL == fh) { if (!fh)
unlock_kernel();
return -ENOMEM; return -ENOMEM;
}
lock_kernel();
file->private_data = fh; file->private_data = fh;
fh->dev = dev; fh->dev = dev;
...@@ -1592,8 +1587,6 @@ static int mpeg_open(struct file *file) ...@@ -1592,8 +1587,6 @@ static int mpeg_open(struct file *file)
V4L2_FIELD_INTERLACED, V4L2_FIELD_INTERLACED,
sizeof(struct cx23885_buffer), sizeof(struct cx23885_buffer),
fh, NULL); fh, NULL);
unlock_kernel();
return 0; return 0;
} }
......
...@@ -26,7 +26,6 @@ ...@@ -26,7 +26,6 @@
#include <linux/kmod.h> #include <linux/kmod.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/smp_lock.h>
#include <linux/interrupt.h> #include <linux/interrupt.h>
#include <linux/delay.h> #include <linux/delay.h>
#include <linux/kthread.h> #include <linux/kthread.h>
...@@ -743,8 +742,6 @@ static int video_open(struct file *file) ...@@ -743,8 +742,6 @@ static int video_open(struct file *file)
if (NULL == fh) if (NULL == fh)
return -ENOMEM; return -ENOMEM;
lock_kernel();
file->private_data = fh; file->private_data = fh;
fh->dev = dev; fh->dev = dev;
fh->radio = radio; fh->radio = radio;
...@@ -762,8 +759,6 @@ static int video_open(struct file *file) ...@@ -762,8 +759,6 @@ static int video_open(struct file *file)
dprintk(1, "post videobuf_queue_init()\n"); dprintk(1, "post videobuf_queue_init()\n");
unlock_kernel();
return 0; return 0;
} }
......
...@@ -31,7 +31,6 @@ static const char version[] = "0.24"; ...@@ -31,7 +31,6 @@ static const char version[] = "0.24";
#include <linux/init.h> #include <linux/init.h>
#include <linux/vmalloc.h> #include <linux/vmalloc.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/smp_lock.h>
#include <linux/pagemap.h> #include <linux/pagemap.h>
#include <linux/usb.h> #include <linux/usb.h>
#include "se401.h" #include "se401.h"
...@@ -951,9 +950,9 @@ static int se401_open(struct file *file) ...@@ -951,9 +950,9 @@ static int se401_open(struct file *file)
struct usb_se401 *se401 = (struct usb_se401 *)dev; struct usb_se401 *se401 = (struct usb_se401 *)dev;
int err = 0; int err = 0;
lock_kernel(); mutex_lock(&se401->lock);
if (se401->user) { if (se401->user) {
unlock_kernel(); mutex_unlock(&se401->lock);
return -EBUSY; return -EBUSY;
} }
se401->fbuf = rvmalloc(se401->maxframesize * SE401_NUMFRAMES); se401->fbuf = rvmalloc(se401->maxframesize * SE401_NUMFRAMES);
...@@ -962,7 +961,7 @@ static int se401_open(struct file *file) ...@@ -962,7 +961,7 @@ static int se401_open(struct file *file)
else else
err = -ENOMEM; err = -ENOMEM;
se401->user = !err; se401->user = !err;
unlock_kernel(); mutex_unlock(&se401->lock);
return err; return err;
} }
......
...@@ -27,7 +27,6 @@ ...@@ -27,7 +27,6 @@
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/errno.h> #include <linux/errno.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/smp_lock.h>
#include <linux/usb.h> #include <linux/usb.h>
#include <linux/mm.h> #include <linux/mm.h>
...@@ -673,14 +672,11 @@ static int v4l_stk_open(struct file *fp) ...@@ -673,14 +672,11 @@ static int v4l_stk_open(struct file *fp)
vdev = video_devdata(fp); vdev = video_devdata(fp);
dev = vdev_to_camera(vdev); dev = vdev_to_camera(vdev);
lock_kernel();
if (dev == NULL || !is_present(dev)) { if (dev == NULL || !is_present(dev)) {
unlock_kernel();
return -ENXIO; return -ENXIO;
} }
fp->private_data = dev; fp->private_data = dev;
usb_autopm_get_interface(dev->interface); usb_autopm_get_interface(dev->interface);
unlock_kernel();
return 0; return 0;
} }
......
...@@ -36,7 +36,6 @@ ...@@ -36,7 +36,6 @@
#include <linux/string.h> #include <linux/string.h>
#include <linux/types.h> #include <linux/types.h>
#include <linux/firmware.h> #include <linux/firmware.h>
#include <linux/smp_lock.h>
#include "vendorcmds.h" #include "vendorcmds.h"
#include "pd-common.h" #include "pd-common.h"
...@@ -485,15 +484,11 @@ static void poseidon_disconnect(struct usb_interface *interface) ...@@ -485,15 +484,11 @@ static void poseidon_disconnect(struct usb_interface *interface)
/*unregister v4l2 device */ /*unregister v4l2 device */
v4l2_device_unregister(&pd->v4l2_dev); v4l2_device_unregister(&pd->v4l2_dev);
lock_kernel(); pd_dvb_usb_device_exit(pd);
{ poseidon_fm_exit(pd);
pd_dvb_usb_device_exit(pd);
poseidon_fm_exit(pd);
poseidon_audio_free(pd); poseidon_audio_free(pd);
pd_video_exit(pd); pd_video_exit(pd);
}
unlock_kernel();
usb_set_intfdata(interface, NULL); usb_set_intfdata(interface, NULL);
kref_put(&pd->kref, poseidon_delete); kref_put(&pd->kref, poseidon_delete);
......
...@@ -43,7 +43,6 @@ ...@@ -43,7 +43,6 @@
#include <linux/vmalloc.h> #include <linux/vmalloc.h>
#include <linux/mm.h> #include <linux/mm.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/smp_lock.h>
#include <linux/mutex.h> #include <linux/mutex.h>
#include <linux/firmware.h> #include <linux/firmware.h>
#include <linux/ihex.h> #include <linux/ihex.h>
...@@ -483,29 +482,28 @@ vicam_open(struct file *file) ...@@ -483,29 +482,28 @@ vicam_open(struct file *file)
return -EINVAL; return -EINVAL;
} }
/* the videodev_lock held above us protects us from /* cam_lock/open_count protects us from simultaneous opens
* simultaneous opens...for now. we probably shouldn't * ... for now. we probably shouldn't rely on this fact forever.
* rely on this fact forever.
*/ */
lock_kernel(); mutex_lock(&cam->cam_lock);
if (cam->open_count > 0) { if (cam->open_count > 0) {
printk(KERN_INFO printk(KERN_INFO
"vicam_open called on already opened camera"); "vicam_open called on already opened camera");
unlock_kernel(); mutex_unlock(&cam->cam_lock);
return -EBUSY; return -EBUSY;
} }
cam->raw_image = kmalloc(VICAM_MAX_READ_SIZE, GFP_KERNEL); cam->raw_image = kmalloc(VICAM_MAX_READ_SIZE, GFP_KERNEL);
if (!cam->raw_image) { if (!cam->raw_image) {
unlock_kernel(); mutex_unlock(&cam->cam_lock);
return -ENOMEM; return -ENOMEM;
} }
cam->framebuf = rvmalloc(VICAM_MAX_FRAME_SIZE * VICAM_FRAMES); cam->framebuf = rvmalloc(VICAM_MAX_FRAME_SIZE * VICAM_FRAMES);
if (!cam->framebuf) { if (!cam->framebuf) {
kfree(cam->raw_image); kfree(cam->raw_image);
unlock_kernel(); mutex_unlock(&cam->cam_lock);
return -ENOMEM; return -ENOMEM;
} }
...@@ -513,10 +511,17 @@ vicam_open(struct file *file) ...@@ -513,10 +511,17 @@ vicam_open(struct file *file)
if (!cam->cntrlbuf) { if (!cam->cntrlbuf) {
kfree(cam->raw_image); kfree(cam->raw_image);
rvfree(cam->framebuf, VICAM_MAX_FRAME_SIZE * VICAM_FRAMES); rvfree(cam->framebuf, VICAM_MAX_FRAME_SIZE * VICAM_FRAMES);
unlock_kernel(); mutex_unlock(&cam->cam_lock);
return -ENOMEM; return -ENOMEM;
} }
cam->needsDummyRead = 1;
cam->open_count++;
file->private_data = cam;
mutex_unlock(&cam->cam_lock);
// First upload firmware, then turn the camera on // First upload firmware, then turn the camera on
if (!cam->is_initialized) { if (!cam->is_initialized) {
...@@ -527,12 +532,6 @@ vicam_open(struct file *file) ...@@ -527,12 +532,6 @@ vicam_open(struct file *file)
set_camera_power(cam, 1); set_camera_power(cam, 1);
cam->needsDummyRead = 1;
cam->open_count++;
file->private_data = cam;
unlock_kernel();
return 0; return 0;
} }
......
...@@ -25,7 +25,6 @@ ...@@ -25,7 +25,6 @@
#include <linux/init.h> #include <linux/init.h>
#include <linux/kmod.h> #include <linux/kmod.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/smp_lock.h>
#include <asm/uaccess.h> #include <asm/uaccess.h>
#include <asm/system.h> #include <asm/system.h>
...@@ -247,10 +246,12 @@ static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) ...@@ -247,10 +246,12 @@ static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
mutex_unlock(vdev->lock); mutex_unlock(vdev->lock);
} else if (vdev->fops->ioctl) { } else if (vdev->fops->ioctl) {
/* TODO: convert all drivers to unlocked_ioctl */ /* TODO: convert all drivers to unlocked_ioctl */
lock_kernel(); static DEFINE_MUTEX(v4l2_ioctl_mutex);
mutex_lock(&v4l2_ioctl_mutex);
if (video_is_registered(vdev)) if (video_is_registered(vdev))
ret = vdev->fops->ioctl(filp, cmd, arg); ret = vdev->fops->ioctl(filp, cmd, arg);
unlock_kernel(); mutex_unlock(&v4l2_ioctl_mutex);
} else } else
ret = -ENOTTY; ret = -ENOTTY;
......
...@@ -388,6 +388,7 @@ struct zoran { ...@@ -388,6 +388,7 @@ struct zoran {
struct videocodec *vfe; /* video front end */ struct videocodec *vfe; /* video front end */
struct mutex resource_lock; /* prevent evil stuff */ struct mutex resource_lock; /* prevent evil stuff */
struct mutex other_lock; /* please merge with above */
u8 initialized; /* flag if zoran has been correctly initialized */ u8 initialized; /* flag if zoran has been correctly initialized */
int user; /* number of current users */ int user; /* number of current users */
......
...@@ -1227,6 +1227,7 @@ static int __devinit zoran_probe(struct pci_dev *pdev, ...@@ -1227,6 +1227,7 @@ static int __devinit zoran_probe(struct pci_dev *pdev,
snprintf(ZR_DEVNAME(zr), sizeof(ZR_DEVNAME(zr)), "MJPEG[%u]", zr->id); snprintf(ZR_DEVNAME(zr), sizeof(ZR_DEVNAME(zr)), "MJPEG[%u]", zr->id);
spin_lock_init(&zr->spinlock); spin_lock_init(&zr->spinlock);
mutex_init(&zr->resource_lock); mutex_init(&zr->resource_lock);
mutex_init(&zr->other_lock);
if (pci_enable_device(pdev)) if (pci_enable_device(pdev))
goto zr_unreg; goto zr_unreg;
pci_read_config_byte(zr->pci_dev, PCI_CLASS_REVISION, &zr->revision); pci_read_config_byte(zr->pci_dev, PCI_CLASS_REVISION, &zr->revision);
......
...@@ -49,7 +49,6 @@ ...@@ -49,7 +49,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/delay.h> #include <linux/delay.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/smp_lock.h>
#include <linux/pci.h> #include <linux/pci.h>
#include <linux/vmalloc.h> #include <linux/vmalloc.h>
#include <linux/wait.h> #include <linux/wait.h>
...@@ -913,7 +912,7 @@ static int zoran_open(struct file *file) ...@@ -913,7 +912,7 @@ static int zoran_open(struct file *file)
dprintk(2, KERN_INFO "%s: %s(%s, pid=[%d]), users(-)=%d\n", dprintk(2, KERN_INFO "%s: %s(%s, pid=[%d]), users(-)=%d\n",
ZR_DEVNAME(zr), __func__, current->comm, task_pid_nr(current), zr->user + 1); ZR_DEVNAME(zr), __func__, current->comm, task_pid_nr(current), zr->user + 1);
lock_kernel(); mutex_lock(&zr->other_lock);
if (zr->user >= 2048) { if (zr->user >= 2048) {
dprintk(1, KERN_ERR "%s: too many users (%d) on device\n", dprintk(1, KERN_ERR "%s: too many users (%d) on device\n",
...@@ -963,14 +962,14 @@ static int zoran_open(struct file *file) ...@@ -963,14 +962,14 @@ static int zoran_open(struct file *file)
file->private_data = fh; file->private_data = fh;
fh->zr = zr; fh->zr = zr;
zoran_open_init_session(fh); zoran_open_init_session(fh);
unlock_kernel(); mutex_unlock(&zr->other_lock);
return 0; return 0;
fail_fh: fail_fh:
kfree(fh); kfree(fh);
fail_unlock: fail_unlock:
unlock_kernel(); mutex_unlock(&zr->other_lock);
dprintk(2, KERN_INFO "%s: open failed (%d), users(-)=%d\n", dprintk(2, KERN_INFO "%s: open failed (%d), users(-)=%d\n",
ZR_DEVNAME(zr), res, zr->user); ZR_DEVNAME(zr), res, zr->user);
...@@ -989,7 +988,7 @@ zoran_close(struct file *file) ...@@ -989,7 +988,7 @@ zoran_close(struct file *file)
/* kernel locks (fs/device.c), so don't do that ourselves /* kernel locks (fs/device.c), so don't do that ourselves
* (prevents deadlocks) */ * (prevents deadlocks) */
/*mutex_lock(&zr->resource_lock);*/ mutex_lock(&zr->other_lock);
zoran_close_end_session(fh); zoran_close_end_session(fh);
...@@ -1023,6 +1022,7 @@ zoran_close(struct file *file) ...@@ -1023,6 +1022,7 @@ zoran_close(struct file *file)
encoder_call(zr, video, s_routing, 2, 0, 0); encoder_call(zr, video, s_routing, 2, 0, 0);
} }
} }
mutex_unlock(&zr->other_lock);
file->private_data = NULL; file->private_data = NULL;
kfree(fh->overlay_mask); kfree(fh->overlay_mask);
...@@ -3370,11 +3370,26 @@ static const struct v4l2_ioctl_ops zoran_ioctl_ops = { ...@@ -3370,11 +3370,26 @@ static const struct v4l2_ioctl_ops zoran_ioctl_ops = {
#endif #endif
}; };
/* please use zr->resource_lock consistently and kill this wrapper */
static long zoran_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
struct zoran_fh *fh = file->private_data;
struct zoran *zr = fh->zr;
int ret;
mutex_lock(&zr->other_lock);
ret = video_ioctl2(file, cmd, arg);
mutex_unlock(&zr->other_lock);
return ret;
}
static const struct v4l2_file_operations zoran_fops = { static const struct v4l2_file_operations zoran_fops = {
.owner = THIS_MODULE, .owner = THIS_MODULE,
.open = zoran_open, .open = zoran_open,
.release = zoran_close, .release = zoran_close,
.ioctl = video_ioctl2, .unlocked_ioctl = zoran_ioctl,
.read = zoran_read, .read = zoran_read,
.write = zoran_write, .write = zoran_write,
.mmap = zoran_mmap, .mmap = zoran_mmap,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment