Re: [PATCH 2/2] [media] uvcvideo: Kill video URBs on disconnect

From: Daniel Axtens
Date: Sat Apr 22 2017 - 20:41:09 EST


Hi Laurent,

Apologies for takig so long to get back to you.

> I assume that the error message is caused by a race between disconnection and
> URB handling. When the device is disconnected I believe the USB core will
> cancel all in-progress URBs (I'm not very familiar with that part of the USB
> core anymore, so please don't consider this or any further related statement
> as true without double-checking), resulting in the URB completion handler
> being called with an error status. The completion handler should then avoid
> resubmitting the URB. However, if the completion handler is in progress when
> the device is disconnected, it won't notice that the device got disconnected,
> and will try to resubmit the URB.
>
> I'm not sure to see how this patch will fix the problem. If the URB completion
> handler is in progress when the device is being disconnected, won't it call
> usb_submit_urb() regardless of whether you call usb_kill_urb() in the
> disconnect handler, resulting in an error message being printed ?

You're completely right - I misunderstood the flow of the
function. Fixing this would require a different approach and would be
more invasive. Given that this does not cause anything more annoying
than some log messages, I will leave it for now.

Regards,
Daniel

>
>> ---
>> drivers/media/usb/uvc/uvc_driver.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_driver.c
>> b/drivers/media/usb/uvc/uvc_driver.c index 2390592f78e0..647e3d8a1256
>> 100644
>> --- a/drivers/media/usb/uvc/uvc_driver.c
>> +++ b/drivers/media/usb/uvc/uvc_driver.c
>> @@ -1877,6 +1877,8 @@ static void uvc_unregister_video(struct uvc_device
>> *dev) if (!video_is_registered(&stream->vdev))
>> continue;
>>
>> + uvc_video_enable(stream, 0);
>> +
>> video_unregister_device(&stream->vdev);
>>
>> uvc_debugfs_cleanup_stream(stream);
>
> --
> Regards,
>
> Laurent Pinchart