Re: [RFC PATCH 2/6] v4l2-core: push taking ioctl mutex down to ioctl handler.

[Ezequiel Garcia <ezequiel@collabora.com>]

On Mon, 2018-05-14 at 13:55 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hansverk@cisco.com>
> 
> The ioctl serialization mutex (vdev->lock or q->lock for vb2 queues)
> was taken at the highest level in v4l2-dev.c. This prevents more
> fine-grained locking since at that level we cannot examine the ioctl
> arguments, we can only do that after video_usercopy is called.
> 
> So push the locking down to __video_do_ioctl() and subdev_do_ioctl_lock().
> 
> This also allows us to make a few functions in v4l2-ioctl.c static and
> video_usercopy() is no longer exported.
> 
> The locking scheme is not changed by this patch, just pushed down.
> 
> Signed-off-by: Hans Verkuil <hansverk@cisco.com>
> ---
>  drivers/media/v4l2-core/v4l2-dev.c    |  6 ------
>  drivers/media/v4l2-core/v4l2-ioctl.c  | 17 ++++++++++++++---
>  drivers/media/v4l2-core/v4l2-subdev.c | 17 ++++++++++++++++-
>  include/media/v4l2-dev.h              |  9 ---------
>  include/media/v4l2-ioctl.h            | 12 ------------
>  5 files changed, 30 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index c4f4357e9ca4..4ffd7d60a901 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -360,14 +360,8 @@ static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  	int ret = -ENODEV;
>  
>  	if (vdev->fops->unlocked_ioctl) {
> -		struct mutex *lock = v4l2_ioctl_get_lock(vdev, cmd);
> -
> -		if (lock && mutex_lock_interruptible(lock))
> -			return -ERESTARTSYS;
>  		if (video_is_registered(vdev))

This is_registered check looks spurious.

Other than that, it looks good.