Re: [RFC] Serialization flag example

[Mauro Carvalho Chehab <mchehab@redhat.com>]

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