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

[Daniel Axtens <dja@axtens.net>]

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