Hans Verkuil wrote:
>> Maybe a better alternative would be to pass to the V4L2 core, optionally,
>> some lock,
>> used internally on the driver. This approach will still be pedantic, as all
>> ioctls will
>> keep being serialized, but at least the driver will need to explicitly
>> handle the lock,
>> and the same lock can be used on other parts of the driver.
>
> Well, I guess you could add a 'struct mutex *serialize;' field to v4l2_device
> that drivers can set. But I don't see much of a difference in practice.
It makes easier to implement more complex approaches, where you may need to use
more
locks. Also, It makes no sense to make a DVB code or an alsa code dependent on
a V4L
header, just because it needs to see a mutex.
Also, a mutex at the driver need to be initialized inside the driver. It is not
just one
flag that someone writing a new driver will clone without really understanding
what
it is doing.
> Regarding the 'pedantic approach': we can easily move the locking to
> video_ioctl2:
>
> struct video_device *vdev = video_devdata(file);
> int serialize = must_serialize_ioctl(vdev, cmd);
>
> if (serialize)
> mutex_lock(&vdev->v4l2_dev->serialize_lock);
> /* Handles IOCTL */
> err = __video_do_ioctl(file, cmd, parg);
> if (serialize)
> mutex_unlock(&vdev->v4l2_dev->serialize_lock);
>
>
> And must_serialize_ioctl() looks like this:
>
> static int must_serialize_ioctl(struct video_device *vdev, int cmd)
> {
> if (!vdev->v4l2_dev || !vdev->v4l2_dev->fl_serialize)
> return 0;
> switch (cmd) {
> case VIDIOC_QUERYCAP:
> case VIDIOC_ENUM_FMT:
> ...
> return 0;
> }
> return 1;
> }
>
> Basically the CAP (capability) ioctls and all ENUM ioctls do not have to be
> serialized. I am not sure whether the streaming ioctls should also be included
> here. I can't really grasp the consequences of whatever choice we make. If we
> want to be compatible with what happens today with ioctl and the BKL, then we
> need to lock and have videobuf unlock whenever it has to wait for an event.
I suspect that the list of "must have" is driver-dependent.
--
Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
