This used to be the default if the lock pointer was set, but now that lock is by
default only used for ioctl serialization. Those drivers that already used
core locking have this flag set explicitly, except for some drivers where
it was obvious that there was no need to serialize any file operations other
than ioctl.
The drivers that didn't need this flag were:
drivers/media/radio/dsbr100.c
drivers/media/radio/radio-isa.c
drivers/media/radio/radio-keene.c
drivers/media/radio/radio-miropcm20.c
drivers/media/radio/radio-mr800.c
drivers/media/radio/radio-tea5764.c
drivers/media/radio/radio-timb.c
drivers/media/video/vivi.c
sound/i2c/other/tea575x-tuner.c
The other drivers that use core locking and where it was not immediately
obvious that this flag wasn't needed were changed so that the flag is set
together with a comment that that driver needs work to avoid having to
set that flag. This will often involve taking the core lock in the fops
themselves.
Eventually this flag should go and it should not be used in new drivers.
There are a few reasons why we want to avoid core locking of non-ioctl
fops: in the case of mmap this can lead to a deadlock in rare situations
since when mmap is called the mmap_sem is held and it is possible for
other parts of the code to take that lock as well (copy_from_user()/copy_to_user()
perform a down_read(&mm->mmap_sem) when a page fault occurs).
It is very unlikely that that happens since the core lock serializes all
fops, but the kernel warns about it if lock validation is turned on.
For poll it is also undesirable to take the core lock as that can introduce
increased latency. The same is true for read/write.
While it was possible to make flags or something to turn on/off taking the
core lock for each file operation, in practice it is much simpler to just
not take it at all except for ioctl and leave it to the driver to take the
lock. There are only a handful fops compared to the zillion ioctls we have.
I also wanted to make it obvious which drivers still take the lock for all
fops, so that's why I chose to have drivers set it explicitly.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
vfd->fops = &video_fops;
vfd->ioctl_ops = &dev->ext_vv_data->ops;
vfd->release = video_device_release;
+ /* Locking in file operations other than ioctl should be done by
+ the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
vfd->lock = &dev->v4l2_lock;
vfd->tvnorms = 0;
for (i = 0; i < dev->ext_vv_data->num_stds; i++)
video_set_drvdata(gradio_dev, fmdev);
gradio_dev->lock = &fmdev->mutex;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &gradio_dev->flags);
/* Register with V4L2 subsystem as RADIO device */
if (video_register_device(gradio_dev, VFL_TYPE_RADIO, radio_nr)) {
INIT_LIST_HEAD(&bcap_dev->dma_queue);
vfd->lock = &bcap_dev->mutex;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
/* register video device */
ret = video_register_device(bcap_dev->video_dev, VFL_TYPE_GRABBER, -1);
cam->vdev.ctrl_handler = hdl;
cam->vdev.v4l2_dev = &cam->v4l2_dev;
set_bit(V4L2_FL_USE_FH_PRIO, &cam->vdev.flags);
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &cam->vdev.flags);
reset_camera_struct_v4l(cam);
vfd->release = video_device_release;
vfd->debug = video_debug;
vfd->lock = &dev->lock;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
snprintf(vfd->name, sizeof(vfd->name), "%s %s", dev->name, type_name);
vbd->ioctl_ops = &vpbe_ioctl_ops;
vbd->minor = -1;
vbd->v4l2_dev = &disp_dev->vpbe_dev->v4l2_dev;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vbd->flags);
vbd->lock = &vpbe_display_layer->opslock;
if (disp_dev->vpbe_dev->current_timings.timings_type &
common = &(ch->common[VPIF_VIDEO_INDEX]);
spin_lock_init(&common->irqlock);
mutex_init(&common->lock);
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &ch->video_dev->flags);
ch->video_dev->lock = &common->lock;
/* Initialize prio member of channel object */
v4l2_prio_init(&ch->prio);
v4l2_prio_init(&ch->prio);
ch->common[VPIF_VIDEO_INDEX].fmt.type =
V4L2_BUF_TYPE_VIDEO_OUTPUT;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &ch->video_dev->flags);
ch->video_dev->lock = &common->lock;
/* register video device */
vfd->release = video_device_release;
vfd->debug = video_debug;
vfd->lock = &dev->lock;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
snprintf(vfd->name, sizeof(vfd->name), "%s %s",
dev->name, type_name);
/* initialize locks */
mutex_init(&viu_dev->lock);
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &viu_dev->vdev->flags);
viu_dev->vdev->lock = &viu_dev->lock;
spin_lock_init(&viu_dev->slock);
s->vdev->release = video_device_release;
s->vdev->tvnorms = V4L2_STD_ALL;
s->vdev->lock = &itv->serialize_lock;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &s->vdev->flags);
set_bit(V4L2_FL_USE_FH_PRIO, &s->vdev->flags);
ivtv_set_funcs(s->vdev);
return 0;
}
*vfd = m2mtest_videodev;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
vfd->lock = &dev->dev_mutex;
ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
}
*vfd = emmaprp_videodev;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
vfd->lock = &pcdev->dev_mutex;
video_set_drvdata(vfd, pcdev);
/* register 4 video devices */
channel->vdev = template;
channel->vdev.lock = &dev->lock;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &channel->vdev.flags);
channel->vdev.v4l2_dev = &dev->v4l2_dev;
video_set_drvdata(&channel->vdev, channel);
if (video_nr == -1)
vfd->minor = -1;
vfd->release = video_device_release;
vfd->lock = &fimc->lock;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
video_set_drvdata(vfd, fimc);
vid_cap = &fimc->vid_cap;
vfd->minor = -1;
vfd->release = video_device_release;
vfd->lock = &fimc->lock;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
snprintf(vfd->name, sizeof(vfd->name), "%s.m2m", dev_name(&pdev->dev));
video_set_drvdata(vfd, fimc);
goto unreg_v4l2_dev;
}
*vfd = g2d_videodev;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
vfd->lock = &dev->mutex;
ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
if (ret) {
jpeg->vfd_encoder->release = video_device_release;
jpeg->vfd_encoder->lock = &jpeg->lock;
jpeg->vfd_encoder->v4l2_dev = &jpeg->v4l2_dev;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &jpeg->vfd_encoder->flags);
ret = video_register_device(jpeg->vfd_encoder, VFL_TYPE_GRABBER, -1);
if (ret) {
jpeg->vfd_decoder->release = video_device_release;
jpeg->vfd_decoder->lock = &jpeg->lock;
jpeg->vfd_decoder->v4l2_dev = &jpeg->v4l2_dev;
+ /* Locking in file operations other than ioctl should be done by the driver,
+ not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &jpeg->vfd_decoder->flags);
ret = video_register_device(jpeg->vfd_decoder, VFL_TYPE_GRABBER, -1);
if (ret) {
vfd->ioctl_ops = get_dec_v4l2_ioctl_ops();
vfd->release = video_device_release,
vfd->lock = &dev->mfc_mutex;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
vfd->v4l2_dev = &dev->v4l2_dev;
snprintf(vfd->name, sizeof(vfd->name), "%s", S5P_MFC_DEC_NAME);
dev->vfd_dec = vfd;
vfd->ioctl_ops = get_enc_v4l2_ioctl_ops();
vfd->release = video_device_release,
vfd->lock = &dev->mfc_mutex;
+ /* This should not be necessary */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
vfd->v4l2_dev = &dev->v4l2_dev;
snprintf(vfd->name, sizeof(vfd->name), "%s", S5P_MFC_ENC_NAME);
dev->vfd_enc = vfd;
set_bit(V4L2_FL_USE_FH_PRIO, &layer->vfd.flags);
video_set_drvdata(&layer->vfd, layer);
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &layer->vfd.flags);
layer->vfd.lock = &layer->mutex;
layer->vfd.v4l2_dev = &mdev->v4l2_dev;
vdev->v4l2_dev = &vou_dev->v4l2_dev;
vdev->release = video_device_release;
vdev->lock = &vou_dev->fop_lock;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags);
vou_dev->vdev = vdev;
video_set_drvdata(vdev, vou_dev);
vdev->tvnorms = V4L2_STD_UNKNOWN;
vdev->ctrl_handler = &icd->ctrl_handler;
vdev->lock = &icd->video_lock;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags);
icd->vdev = vdev;
vfd->release = video_device_release;
vfd->debug = tm6000_debug;
vfd->lock = &dev->lock;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vfd->flags);
snprintf(vfd->name, sizeof(vfd->name), "%s %s", dev->name, type_name);
if (NULL == vdev)
return NULL;
*vdev = *vdev_template;
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags);
vdev->lock = &usbvision->v4l2_lock;
vdev->v4l2_dev = &usbvision->v4l2_dev;
snprintf(vdev->name, sizeof(vdev->name), "%s", name);
if (!vdev->fops->read)
return -EINVAL;
- if (vdev->lock && mutex_lock_interruptible(vdev->lock))
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags) &&
+ mutex_lock_interruptible(vdev->lock))
return -ERESTARTSYS;
if (video_is_registered(vdev))
ret = vdev->fops->read(filp, buf, sz, off);
- if (vdev->lock)
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
mutex_unlock(vdev->lock);
return ret;
}
if (!vdev->fops->write)
return -EINVAL;
- if (vdev->lock && mutex_lock_interruptible(vdev->lock))
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags) &&
+ mutex_lock_interruptible(vdev->lock))
return -ERESTARTSYS;
if (video_is_registered(vdev))
ret = vdev->fops->write(filp, buf, sz, off);
- if (vdev->lock)
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
mutex_unlock(vdev->lock);
return ret;
}
if (!vdev->fops->poll)
return DEFAULT_POLLMASK;
- if (vdev->lock)
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
mutex_lock(vdev->lock);
if (video_is_registered(vdev))
ret = vdev->fops->poll(filp, poll);
- if (vdev->lock)
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
mutex_unlock(vdev->lock);
return ret;
}
if (!vdev->fops->mmap)
return ret;
- if (vdev->lock && mutex_lock_interruptible(vdev->lock))
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags) &&
+ mutex_lock_interruptible(vdev->lock))
return -ERESTARTSYS;
if (video_is_registered(vdev))
ret = vdev->fops->mmap(filp, vm);
- if (vdev->lock)
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
mutex_unlock(vdev->lock);
return ret;
}
video_get(vdev);
mutex_unlock(&videodev_lock);
if (vdev->fops->open) {
- if (vdev->lock && mutex_lock_interruptible(vdev->lock)) {
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags) &&
+ mutex_lock_interruptible(vdev->lock)) {
ret = -ERESTARTSYS;
goto err;
}
ret = vdev->fops->open(filp);
else
ret = -ENODEV;
- if (vdev->lock)
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
mutex_unlock(vdev->lock);
}
int ret = 0;
if (vdev->fops->release) {
- if (vdev->lock)
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
mutex_lock(vdev->lock);
vdev->fops->release(filp);
- if (vdev->lock)
+ if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
mutex_unlock(vdev->lock);
}
/* decrease the refcount unconditionally since the release()
WARN_ON(video_device[vdev->minor] != NULL);
vdev->index = get_index(vdev);
mutex_unlock(&videodev_lock);
+ /* if no lock was passed, then make sure the LOCK_ALL_FOPS bit is
+ clear and warn if it wasn't. */
+ if (vdev->lock == NULL)
+ WARN_ON(test_and_clear_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags));
if (vdev->ioctl_ops)
determine_valid_ioctls(vdev);
INIT_LIST_HEAD(&pd->dmaq);
mutex_init(&pd->mux);
pd->vdev->lock = &pd->mux; /* for locking v4l2_file_operations */
+ /* Locking in file operations other than ioctl should be done
+ by the driver, not the V4L2 core.
+ This driver needs auditing so that this flag can be removed. */
+ set_bit(V4L2_FL_LOCK_ALL_FOPS, &pd->vdev->flags);
spin_lock_init(&pd->lock);
pd->csr2 = csr2_init;
pd->config = config_init;
#define V4L2_FL_USES_V4L2_FH (1)
/* Use the prio field of v4l2_fh for core priority checking */
#define V4L2_FL_USE_FH_PRIO (2)
+/* If ioctl core locking is in use, then apply that also to all
+ file operations. */
+#define V4L2_FL_LOCK_ALL_FOPS (3)
/* Priority helper functions */
projects / ~andy / linux / commitdiff