The only thread safe way to stop the status handler is with uvc_status. Let's remove all the code paths partially stopping uvc_status. Reviewed-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx> --- drivers/media/usb/uvc/uvc_ctrl.c | 4 ---- drivers/media/usb/uvc/uvc_driver.c | 2 +- drivers/media/usb/uvc/uvc_status.c | 8 ++++---- drivers/media/usb/uvc/uvc_v4l2.c | 2 +- drivers/media/usb/uvc/uvcvideo.h | 2 +- 5 files changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index e59a463c2761..8e22a07e3e7b 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -2765,10 +2765,6 @@ void uvc_ctrl_cleanup_device(struct uvc_device *dev) struct uvc_entity *entity; unsigned int i; - /* Can be uninitialized if we are aborting on probe error. */ - if (dev->async_ctrl.work.func) - cancel_work_sync(&dev->async_ctrl.work); - /* Free controls and control mappings for all entities. */ list_for_each_entry(entity, &dev->entities, list) { for (i = 0; i < entity->ncontrols; ++i) { diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index ded2cb6ce14f..d5dbf2644272 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -2282,7 +2282,7 @@ static int uvc_suspend(struct usb_interface *intf, pm_message_t message) UVC_SC_VIDEOCONTROL) { mutex_lock(&dev->lock); if (dev->users) - uvc_status_stop(dev); + uvc_status_stop(dev, true); mutex_unlock(&dev->lock); return 0; } diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c index a78a88c710e2..9c5da1244999 100644 --- a/drivers/media/usb/uvc/uvc_status.c +++ b/drivers/media/usb/uvc/uvc_status.c @@ -292,7 +292,7 @@ int uvc_status_init(struct uvc_device *dev) void uvc_status_unregister(struct uvc_device *dev) { - usb_kill_urb(dev->int_urb); + uvc_status_stop(dev, false); uvc_input_unregister(dev); } @@ -310,7 +310,7 @@ int uvc_status_start(struct uvc_device *dev, gfp_t flags) return usb_submit_urb(dev->int_urb, flags); } -void uvc_status_stop(struct uvc_device *dev) +void uvc_status_stop(struct uvc_device *dev, bool run_async_work) { struct uvc_ctrl_work *w = &dev->async_ctrl; @@ -326,7 +326,7 @@ void uvc_status_stop(struct uvc_device *dev) * Cancel any pending asynchronous work. If any status event was queued, * process it synchronously. */ - if (cancel_work_sync(&w->work)) + if (cancel_work_sync(&w->work) && run_async_work) uvc_ctrl_status_event(w->chain, w->ctrl, w->data); /* Kill the urb. */ @@ -338,7 +338,7 @@ void uvc_status_stop(struct uvc_device *dev) * cancelled before returning or it could then race with a future * uvc_status_start() call. */ - if (cancel_work_sync(&w->work)) + if (cancel_work_sync(&w->work) && run_async_work) uvc_ctrl_status_event(w->chain, w->ctrl, w->data); /* diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index f4988f03640a..f90206263ff4 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -672,7 +672,7 @@ static int uvc_v4l2_release(struct file *file) mutex_lock(&stream->dev->lock); if (--stream->dev->users == 0) - uvc_status_stop(stream->dev); + uvc_status_stop(stream->dev, false); mutex_unlock(&stream->dev->lock); usb_autopm_put_interface(stream->dev->intf); diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 6fb0a78b1b00..ba8f8c1f2c83 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -745,7 +745,7 @@ int uvc_status_init(struct uvc_device *dev); void uvc_status_unregister(struct uvc_device *dev); void uvc_status_cleanup(struct uvc_device *dev); int uvc_status_start(struct uvc_device *dev, gfp_t flags); -void uvc_status_stop(struct uvc_device *dev); +void uvc_status_stop(struct uvc_device *dev, bool run_async_work); /* Controls */ extern const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited; -- 2.43.0.rc1.413.gea7ed67945-goog