Commit ceede9fa authored by Hans de Goede's avatar Hans de Goede Committed by Mauro Carvalho Chehab

[media] pwc: Fix locking

My last locking rework for pwc mistakenly assumed that videbuf2 does its
own locking, but it does not! This patch fixes the missing locking by
moving over the the video_device lock, and introducing a separate lock
for the videobuf2_queue.
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent a67e1722
This diff is collapsed.
...@@ -464,26 +464,24 @@ static int pwc_s_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *f) ...@@ -464,26 +464,24 @@ static int pwc_s_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *f)
struct pwc_device *pdev = video_drvdata(file); struct pwc_device *pdev = video_drvdata(file);
int ret, pixelformat, compression = 0; int ret, pixelformat, compression = 0;
if (pwc_test_n_set_capt_file(pdev, file))
return -EBUSY;
ret = pwc_vidioc_try_fmt(pdev, f); ret = pwc_vidioc_try_fmt(pdev, f);
if (ret < 0) if (ret < 0)
return ret; return ret;
pixelformat = f->fmt.pix.pixelformat; if (mutex_lock_interruptible(&pdev->vb_queue_lock))
return -ERESTARTSYS;
mutex_lock(&pdev->udevlock); ret = pwc_test_n_set_capt_file(pdev, file);
if (!pdev->udev) { if (ret)
ret = -ENODEV;
goto leave; goto leave;
}
if (pdev->iso_init) { if (pdev->vb_queue.streaming) {
ret = -EBUSY; ret = -EBUSY;
goto leave; goto leave;
} }
pixelformat = f->fmt.pix.pixelformat;
PWC_DEBUG_IOCTL("Trying to set format to: width=%d height=%d fps=%d " PWC_DEBUG_IOCTL("Trying to set format to: width=%d height=%d fps=%d "
"format=%c%c%c%c\n", "format=%c%c%c%c\n",
f->fmt.pix.width, f->fmt.pix.height, pdev->vframes, f->fmt.pix.width, f->fmt.pix.height, pdev->vframes,
...@@ -499,7 +497,7 @@ static int pwc_s_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *f) ...@@ -499,7 +497,7 @@ static int pwc_s_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *f)
pwc_vidioc_fill_fmt(f, pdev->width, pdev->height, pdev->pixfmt); pwc_vidioc_fill_fmt(f, pdev->width, pdev->height, pdev->pixfmt);
leave: leave:
mutex_unlock(&pdev->udevlock); mutex_unlock(&pdev->vb_queue_lock);
return ret; return ret;
} }
...@@ -507,9 +505,6 @@ static int pwc_querycap(struct file *file, void *fh, struct v4l2_capability *cap ...@@ -507,9 +505,6 @@ static int pwc_querycap(struct file *file, void *fh, struct v4l2_capability *cap
{ {
struct pwc_device *pdev = video_drvdata(file); struct pwc_device *pdev = video_drvdata(file);
if (!pdev->udev)
return -ENODEV;
strcpy(cap->driver, PWC_NAME); strcpy(cap->driver, PWC_NAME);
strlcpy(cap->card, pdev->vdev.name, sizeof(cap->card)); strlcpy(cap->card, pdev->vdev.name, sizeof(cap->card));
usb_make_path(pdev->udev, cap->bus_info, sizeof(cap->bus_info)); usb_make_path(pdev->udev, cap->bus_info, sizeof(cap->bus_info));
...@@ -540,15 +535,12 @@ static int pwc_s_input(struct file *file, void *fh, unsigned int i) ...@@ -540,15 +535,12 @@ static int pwc_s_input(struct file *file, void *fh, unsigned int i)
return i ? -EINVAL : 0; return i ? -EINVAL : 0;
} }
static int pwc_g_volatile_ctrl_unlocked(struct v4l2_ctrl *ctrl) static int pwc_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
{ {
struct pwc_device *pdev = struct pwc_device *pdev =
container_of(ctrl->handler, struct pwc_device, ctrl_handler); container_of(ctrl->handler, struct pwc_device, ctrl_handler);
int ret = 0; int ret = 0;
if (!pdev->udev)
return -ENODEV;
switch (ctrl->id) { switch (ctrl->id) {
case V4L2_CID_AUTO_WHITE_BALANCE: case V4L2_CID_AUTO_WHITE_BALANCE:
if (pdev->color_bal_valid && if (pdev->color_bal_valid &&
...@@ -615,18 +607,6 @@ static int pwc_g_volatile_ctrl_unlocked(struct v4l2_ctrl *ctrl) ...@@ -615,18 +607,6 @@ static int pwc_g_volatile_ctrl_unlocked(struct v4l2_ctrl *ctrl)
return ret; return ret;
} }
static int pwc_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
{
struct pwc_device *pdev =
container_of(ctrl->handler, struct pwc_device, ctrl_handler);
int ret;
mutex_lock(&pdev->udevlock);
ret = pwc_g_volatile_ctrl_unlocked(ctrl);
mutex_unlock(&pdev->udevlock);
return ret;
}
static int pwc_set_awb(struct pwc_device *pdev) static int pwc_set_awb(struct pwc_device *pdev)
{ {
int ret; int ret;
...@@ -648,7 +628,7 @@ static int pwc_set_awb(struct pwc_device *pdev) ...@@ -648,7 +628,7 @@ static int pwc_set_awb(struct pwc_device *pdev)
if (pdev->auto_white_balance->val == awb_indoor || if (pdev->auto_white_balance->val == awb_indoor ||
pdev->auto_white_balance->val == awb_outdoor || pdev->auto_white_balance->val == awb_outdoor ||
pdev->auto_white_balance->val == awb_fl) pdev->auto_white_balance->val == awb_fl)
pwc_g_volatile_ctrl_unlocked(pdev->auto_white_balance); pwc_g_volatile_ctrl(pdev->auto_white_balance);
} }
if (pdev->auto_white_balance->val != awb_manual) if (pdev->auto_white_balance->val != awb_manual)
return 0; return 0;
...@@ -812,13 +792,6 @@ static int pwc_s_ctrl(struct v4l2_ctrl *ctrl) ...@@ -812,13 +792,6 @@ static int pwc_s_ctrl(struct v4l2_ctrl *ctrl)
container_of(ctrl->handler, struct pwc_device, ctrl_handler); container_of(ctrl->handler, struct pwc_device, ctrl_handler);
int ret = 0; int ret = 0;
mutex_lock(&pdev->udevlock);
if (!pdev->udev) {
ret = -ENODEV;
goto leave;
}
switch (ctrl->id) { switch (ctrl->id) {
case V4L2_CID_BRIGHTNESS: case V4L2_CID_BRIGHTNESS:
ret = pwc_set_u8_ctrl(pdev, SET_LUM_CTL, ret = pwc_set_u8_ctrl(pdev, SET_LUM_CTL,
...@@ -915,8 +888,6 @@ static int pwc_s_ctrl(struct v4l2_ctrl *ctrl) ...@@ -915,8 +888,6 @@ static int pwc_s_ctrl(struct v4l2_ctrl *ctrl)
if (ret) if (ret)
PWC_ERROR("s_ctrl %s error %d\n", ctrl->name, ret); PWC_ERROR("s_ctrl %s error %d\n", ctrl->name, ret);
leave:
mutex_unlock(&pdev->udevlock);
return ret; return ret;
} }
...@@ -949,11 +920,9 @@ static int pwc_g_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *f) ...@@ -949,11 +920,9 @@ static int pwc_g_fmt_vid_cap(struct file *file, void *fh, struct v4l2_format *f)
if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL; return -EINVAL;
mutex_lock(&pdev->udevlock); /* To avoid race with s_fmt */
PWC_DEBUG_IOCTL("ioctl(VIDIOC_G_FMT) return size %dx%d\n", PWC_DEBUG_IOCTL("ioctl(VIDIOC_G_FMT) return size %dx%d\n",
pdev->width, pdev->height); pdev->width, pdev->height);
pwc_vidioc_fill_fmt(f, pdev->width, pdev->height, pdev->pixfmt); pwc_vidioc_fill_fmt(f, pdev->width, pdev->height, pdev->pixfmt);
mutex_unlock(&pdev->udevlock);
return 0; return 0;
} }
...@@ -968,70 +937,98 @@ static int pwc_reqbufs(struct file *file, void *fh, ...@@ -968,70 +937,98 @@ static int pwc_reqbufs(struct file *file, void *fh,
struct v4l2_requestbuffers *rb) struct v4l2_requestbuffers *rb)
{ {
struct pwc_device *pdev = video_drvdata(file); struct pwc_device *pdev = video_drvdata(file);
int ret;
if (pwc_test_n_set_capt_file(pdev, file)) if (mutex_lock_interruptible(&pdev->vb_queue_lock))
return -EBUSY; return -ERESTARTSYS;
return vb2_reqbufs(&pdev->vb_queue, rb); ret = pwc_test_n_set_capt_file(pdev, file);
if (ret == 0)
ret = vb2_reqbufs(&pdev->vb_queue, rb);
mutex_unlock(&pdev->vb_queue_lock);
return ret;
} }
static int pwc_querybuf(struct file *file, void *fh, struct v4l2_buffer *buf) static int pwc_querybuf(struct file *file, void *fh, struct v4l2_buffer *buf)
{ {
struct pwc_device *pdev = video_drvdata(file); struct pwc_device *pdev = video_drvdata(file);
int ret;
return vb2_querybuf(&pdev->vb_queue, buf); if (mutex_lock_interruptible(&pdev->vb_queue_lock))
return -ERESTARTSYS;
ret = pwc_test_n_set_capt_file(pdev, file);
if (ret == 0)
ret = vb2_querybuf(&pdev->vb_queue, buf);
mutex_unlock(&pdev->vb_queue_lock);
return ret;
} }
static int pwc_qbuf(struct file *file, void *fh, struct v4l2_buffer *buf) static int pwc_qbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
{ {
struct pwc_device *pdev = video_drvdata(file); struct pwc_device *pdev = video_drvdata(file);
int ret;
if (!pdev->udev) if (mutex_lock_interruptible(&pdev->vb_queue_lock))
return -ENODEV; return -ERESTARTSYS;
if (pdev->capt_file != file) ret = pwc_test_n_set_capt_file(pdev, file);
return -EBUSY; if (ret == 0)
ret = vb2_qbuf(&pdev->vb_queue, buf);
return vb2_qbuf(&pdev->vb_queue, buf); mutex_unlock(&pdev->vb_queue_lock);
return ret;
} }
static int pwc_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf) static int pwc_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
{ {
struct pwc_device *pdev = video_drvdata(file); struct pwc_device *pdev = video_drvdata(file);
int ret;
if (!pdev->udev) if (mutex_lock_interruptible(&pdev->vb_queue_lock))
return -ENODEV; return -ERESTARTSYS;
if (pdev->capt_file != file) ret = pwc_test_n_set_capt_file(pdev, file);
return -EBUSY; if (ret == 0)
ret = vb2_dqbuf(&pdev->vb_queue, buf,
file->f_flags & O_NONBLOCK);
return vb2_dqbuf(&pdev->vb_queue, buf, file->f_flags & O_NONBLOCK); mutex_unlock(&pdev->vb_queue_lock);
return ret;
} }
static int pwc_streamon(struct file *file, void *fh, enum v4l2_buf_type i) static int pwc_streamon(struct file *file, void *fh, enum v4l2_buf_type i)
{ {
struct pwc_device *pdev = video_drvdata(file); struct pwc_device *pdev = video_drvdata(file);
int ret;
if (!pdev->udev) if (mutex_lock_interruptible(&pdev->vb_queue_lock))
return -ENODEV; return -ERESTARTSYS;
if (pdev->capt_file != file) ret = pwc_test_n_set_capt_file(pdev, file);
return -EBUSY; if (ret == 0)
ret = vb2_streamon(&pdev->vb_queue, i);
return vb2_streamon(&pdev->vb_queue, i); mutex_unlock(&pdev->vb_queue_lock);
return ret;
} }
static int pwc_streamoff(struct file *file, void *fh, enum v4l2_buf_type i) static int pwc_streamoff(struct file *file, void *fh, enum v4l2_buf_type i)
{ {
struct pwc_device *pdev = video_drvdata(file); struct pwc_device *pdev = video_drvdata(file);
int ret;
if (!pdev->udev) if (mutex_lock_interruptible(&pdev->vb_queue_lock))
return -ENODEV; return -ERESTARTSYS;
if (pdev->capt_file != file) ret = pwc_test_n_set_capt_file(pdev, file);
return -EBUSY; if (ret == 0)
ret = vb2_streamoff(&pdev->vb_queue, i);
return vb2_streamoff(&pdev->vb_queue, i); mutex_unlock(&pdev->vb_queue_lock);
return ret;
} }
static int pwc_enum_framesizes(struct file *file, void *fh, static int pwc_enum_framesizes(struct file *file, void *fh,
...@@ -1119,19 +1116,17 @@ static int pwc_s_parm(struct file *file, void *fh, ...@@ -1119,19 +1116,17 @@ static int pwc_s_parm(struct file *file, void *fh,
parm->parm.capture.timeperframe.numerator == 0) parm->parm.capture.timeperframe.numerator == 0)
return -EINVAL; return -EINVAL;
if (pwc_test_n_set_capt_file(pdev, file))
return -EBUSY;
fps = parm->parm.capture.timeperframe.denominator / fps = parm->parm.capture.timeperframe.denominator /
parm->parm.capture.timeperframe.numerator; parm->parm.capture.timeperframe.numerator;
mutex_lock(&pdev->udevlock); if (mutex_lock_interruptible(&pdev->vb_queue_lock))
if (!pdev->udev) { return -ERESTARTSYS;
ret = -ENODEV;
ret = pwc_test_n_set_capt_file(pdev, file);
if (ret)
goto leave; goto leave;
}
if (pdev->iso_init) { if (pdev->vb_queue.streaming) {
ret = -EBUSY; ret = -EBUSY;
goto leave; goto leave;
} }
...@@ -1142,7 +1137,7 @@ static int pwc_s_parm(struct file *file, void *fh, ...@@ -1142,7 +1137,7 @@ static int pwc_s_parm(struct file *file, void *fh,
pwc_g_parm(file, fh, parm); pwc_g_parm(file, fh, parm);
leave: leave:
mutex_unlock(&pdev->udevlock); mutex_unlock(&pdev->vb_queue_lock);
return ret; return ret;
} }
......
...@@ -221,9 +221,17 @@ struct pwc_device ...@@ -221,9 +221,17 @@ struct pwc_device
struct video_device vdev; struct video_device vdev;
struct v4l2_device v4l2_dev; struct v4l2_device v4l2_dev;
/* Pointer to our usb_device, may be NULL after unplug */ /* videobuf2 queue and queued buffers list */
struct usb_device *udev; struct vb2_queue vb_queue;
struct mutex udevlock; struct list_head queued_bufs;
spinlock_t queued_bufs_lock; /* Protects queued_bufs */
/* Note if taking both locks v4l2_lock must always be locked first! */
struct mutex v4l2_lock; /* Protects everything else */
struct mutex vb_queue_lock; /* Protects vb_queue and capt_file */
/* Pointer to our usb_device, will be NULL after unplug */
struct usb_device *udev; /* Both mutexes most be hold when setting! */
/* type of cam (645, 646, 675, 680, 690, 720, 730, 740, 750) */ /* type of cam (645, 646, 675, 680, 690, 720, 730, 740, 750) */
int type; int type;
...@@ -232,7 +240,6 @@ struct pwc_device ...@@ -232,7 +240,6 @@ struct pwc_device
/*** Video data ***/ /*** Video data ***/
struct file *capt_file; /* file doing video capture */ struct file *capt_file; /* file doing video capture */
struct mutex capt_file_lock;
int vendpoint; /* video isoc endpoint */ int vendpoint; /* video isoc endpoint */
int vcinterface; /* video control interface */ int vcinterface; /* video control interface */
int valternate; /* alternate interface needed */ int valternate; /* alternate interface needed */
...@@ -251,12 +258,6 @@ struct pwc_device ...@@ -251,12 +258,6 @@ struct pwc_device
unsigned char *ctrl_buf; unsigned char *ctrl_buf;
struct urb *urbs[MAX_ISO_BUFS]; struct urb *urbs[MAX_ISO_BUFS];
char iso_init;
/* videobuf2 queue and queued buffers list */
struct vb2_queue vb_queue;
struct list_head queued_bufs;
spinlock_t queued_bufs_lock;
/* /*
* Frame currently being filled, this only gets touched by the * Frame currently being filled, this only gets touched by the
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment