[media] pwc: Rework locking

While testing gtk-v4l's new ctrl event code, I hit the following deadlock
in the pwc driver:

Thread 1:
-Does a VIDIOC_G_CTRL
-video2_ioctl takes the modlock
-video2_ioctl calls v4l2_g_ctrl
-v4l2_g_ctrl takes the ctrl_handler lock
-v4l2_g_ctrl calls pwc_g_volatile_ctrl
-pwc_g_volatile_ctrl releases the modlock as the usb transfer can take a
 significant amount of time and we don't want to block DQBUF / QBUF too long
Thread 2:
-Does a VIDIOC_FOO_CTRL
-video2_ioctl takes the modlock
-video2_ioctl calls v4l2_foo_ctrl
-v4l2_foo_ctrl blocks while trying to take the ctrl_handler lock
Thread 1:
-Blocks while trying to re-take the modlock, as its caller will eventually
 unlock that

Now we have thread 1 waiting for the modlock while holding the ctrl_handler
lock and thread 2 waiting for the ctrl_handler lock while holding the
modlock -> deadlock.

Conclusion:
1) We cannot unlock modlock from pwc_s_ctrl / pwc_g_volatile_ctrl,
   but this can cause QBUF / DQBUF to block for up to a full second
2) After evaluating various option I came to the conclusion that pwc should
   stop using the v4l2 core locking, and instead do its own locking

Thus this patch stops pwc using the v4l2 core locking, and replaces that with
it doing its own locking where necessary.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

1 files changed, 22 insertions, 38 deletions

diff --git a/drivers/media/video/pwc/pwc-v4l.c b/drivers/media/video/pwc/pwc-v4l.c
index 1303641c70c1..97e8d629582c 100644
--- a/drivers/media/video/pwc/pwc-v4l.c
+++ b/drivers/media/video/pwc/pwc-v4l.c
@@ -475,17 +475,11 @@ static int pwc_s_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *f)
         struct pwc_device *pdev = video_drvdata(file);
         int ret, fps, snapshot, compression, pixelformat;
 
-        if (!pdev->udev)
-                return -ENODEV;
-
-        if (pdev->capt_file != NULL &&
-            pdev->capt_file != file)
+        if (pwc_test_n_set_capt_file(pdev, file))
                 return -EBUSY;
 
-        pdev->capt_file = file;
-
         ret = pwc_vidioc_try_fmt(pdev, f);
-        if (ret<0)
+        if (ret < 0)
                 return ret;
 
         pixelformat = f->fmt.pix.pixelformat;
@@ -505,8 +499,16 @@ static int pwc_s_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *f)
             pixelformat != V4L2_PIX_FMT_PWC2)
                 return -EINVAL;
 
-        if (vb2_is_streaming(&pdev->vb_queue))
-                return -EBUSY;
+        mutex_lock(&pdev->udevlock);
+        if (!pdev->udev) {
+                ret = -ENODEV;
+                goto leave;
+        }
+
+        if (pdev->iso_init) {
+                ret = -EBUSY;
+                goto leave;
+        }
 
         PWC_DEBUG_IOCTL("Trying to set format to: width=%d height=%d fps=%d "
                         "compression=%d snapshot=%d format=%c%c%c%c\n",
@@ -526,15 +528,14 @@ static int pwc_s_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *f)
 
         PWC_DEBUG_IOCTL("pwc_set_video_mode(), return=%d\n", ret);
 
-        if (ret)
-                return ret;
-
-        pdev->pixfmt = pixelformat;
-
-        pwc_vidioc_fill_fmt(pdev, f);
-
-        return 0;
+        if (ret == 0) {
+                pdev->pixfmt = pixelformat;
+                pwc_vidioc_fill_fmt(pdev, f);
+        }
 
+leave:
+        mutex_unlock(&pdev->udevlock);
+        return ret;
 }
 
 static int pwc_querycap(struct file *file, void *fh, struct v4l2_capability *cap)
@@ -580,18 +581,6 @@ static int pwc_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
                 container_of(ctrl->handler, struct pwc_device, ctrl_handler);
         int ret = 0;
 
-        /*
-         * Sometimes it can take quite long for the pwc to complete usb control
-         * transfers, so release the modlock to give streaming by another
-         * process / thread the chance to continue with a dqbuf.
-         */
-        mutex_unlock(&pdev->modlock);
-
-        /*
-         * Take the udev-lock to protect against the disconnect handler
-         * completing and setting dev->udev to NULL underneath us. Other code
-         * does not need to do this since it is protected by the modlock.
-         */
         mutex_lock(&pdev->udevlock);
 
         if (!pdev->udev) {
@@ -664,7 +653,6 @@ static int pwc_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 
 leave:
         mutex_unlock(&pdev->udevlock);
-        mutex_lock(&pdev->modlock);
         return ret;
 }
 
@@ -844,8 +832,6 @@ static int pwc_s_ctrl(struct v4l2_ctrl *ctrl)
                 container_of(ctrl->handler, struct pwc_device, ctrl_handler);
         int ret = 0;
 
-        /* See the comments on locking in pwc_g_volatile_ctrl */
-        mutex_unlock(&pdev->modlock);
         mutex_lock(&pdev->udevlock);
 
         if (!pdev->udev) {
@@ -951,7 +937,6 @@ static int pwc_s_ctrl(struct v4l2_ctrl *ctrl)
 
 leave:
         mutex_unlock(&pdev->udevlock);
-        mutex_lock(&pdev->modlock);
         return ret;
 }
 
@@ -981,9 +966,11 @@ static int pwc_g_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *f)
 {
         struct pwc_device *pdev = video_drvdata(file);
 
+        mutex_lock(&pdev->udevlock); /* To avoid race with s_fmt */
         PWC_DEBUG_IOCTL("ioctl(VIDIOC_G_FMT) return size %dx%d\n",
                         pdev->image.x, pdev->image.y);
         pwc_vidioc_fill_fmt(pdev, f);
+        mutex_unlock(&pdev->udevlock);
         return 0;
 }
 
@@ -999,12 +986,9 @@ static int pwc_reqbufs(struct file *file, void *fh,
 {
         struct pwc_device *pdev = video_drvdata(file);
 
-        if (pdev->capt_file != NULL &&
-            pdev->capt_file != file)
+        if (pwc_test_n_set_capt_file(pdev, file))
                 return -EBUSY;
 
-        pdev->capt_file = file;
-
         return vb2_reqbufs(&pdev->vb_queue, rb);
 }