Re: [PATCH v2] [media] uvcvideo: Refactor teardown of uvc on USB disconnect

Re: [PATCH v2] [media] uvcvideo: Refactor teardown of uvc on USB disconnect

[Li Li]

https://www.spinics.net/lists/linux-media/msg115062.html

Thanks for Daniel to fix this old issue. I might overlooked it but I
didn't find it in the latest upstream kernel.

Are we going to merge this missing patch? Thanks!

Best,
Li

Re: [PATCH v2] [media] uvcvideo: Refactor teardown of uvc on USB disconnect

[Hans Verkuil]

Laurent, Kieran,

Can one of you look at this?

https://patchwork.linuxtv.org/patch/40941/

BTW, there are a *lot* of old patches delegated to you, Laurent. If neither
you or Kieran have time to look at them, then please undelegate them and I
can take a bunch of them. I see quite a few simple bug fixes (e.g.
https://patchwork.linuxtv.org/patch/42935/) that really should be merged.

Regards,

	Hans

On 04/23/2018 07:59 PM, Li Li wrote:
> https://www.spinics.net/lists/linux-media/msg115062.html
> 
> Thanks for Daniel to fix this old issue. I might overlooked it but I
> didn't find it in the latest upstream kernel.
> 
> Are we going to merge this missing patch? Thanks!
> 
> Best,
> Li
> 

Re: [PATCH v2] [media] uvcvideo: Refactor teardown of uvc on USB disconnect

[Kieran Bingham]

[-- Attachment #1.1: Type: text/plain, Size: 1349 bytes --]

Hi Hans,

On 23/04/18 20:12, Hans Verkuil wrote:
> Laurent, Kieran,
> 
> Can one of you look at this?
> 
> https://patchwork.linuxtv.org/patch/40941/

Looking through it now. Looks good so far - I'll try it against my Async UVC
work as well! It looks like there won't be conflicts though.

I now have a filter on my mailinglists so that I will see UVC related patches
posted to linux-media more prominently, but that's fairly recent. Perhaps I
should add myself to the MAINTAINERS file for UVC so I get CC'd on relevant
patches too.

> BTW, there are a *lot* of old patches delegated to you, Laurent. If neither
> you or Kieran have time to look at them, then please undelegate them and I
> can take a bunch of them. I see quite a few simple bug fixes (e.g.
> https://patchwork.linuxtv.org/patch/42935/) that really should be merged.

Is there anyway I can filter patchwork to see patches delegated to Laurent
(without signing in as Laurent that is :D )

Regards

Kieran


> Regards,
> 
> 	Hans
> 
> On 04/23/2018 07:59 PM, Li Li wrote:
>> https://www.spinics.net/lists/linux-media/msg115062.html
>>
>> Thanks for Daniel to fix this old issue. I might overlooked it but I
>> didn't find it in the latest upstream kernel.
>>
>> Are we going to merge this missing patch? Thanks!
>>
>> Best,
>> Li
>>
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] [media] uvcvideo: Refactor teardown of uvc on USB disconnect

[Hans Verkuil]

On 04/24/18 10:28, Kieran Bingham wrote:
> Hi Hans,
> 
> On 23/04/18 20:12, Hans Verkuil wrote:
>> Laurent, Kieran,
>>
>> Can one of you look at this?
>>
>> https://patchwork.linuxtv.org/patch/40941/
> 
> Looking through it now. Looks good so far - I'll try it against my Async UVC
> work as well! It looks like there won't be conflicts though.
> 
> I now have a filter on my mailinglists so that I will see UVC related patches
> posted to linux-media more prominently, but that's fairly recent. Perhaps I
> should add myself to the MAINTAINERS file for UVC so I get CC'd on relevant
> patches too.
> 
>> BTW, there are a *lot* of old patches delegated to you, Laurent. If neither
>> you or Kieran have time to look at them, then please undelegate them and I
>> can take a bunch of them. I see quite a few simple bug fixes (e.g.
>> https://patchwork.linuxtv.org/patch/42935/) that really should be merged.
> 
> Is there anyway I can filter patchwork to see patches delegated to Laurent
> (without signing in as Laurent that is :D )

Click on the 'Delegate' column and it will sort all the patches by delegate.

I also see a lot of patches that should be superseded, it really needs spring
cleaning :-)

Regards,

	Hans

> 
> Regards
> 
> Kieran
> 
> 
>> Regards,
>>
>> 	Hans
>>
>> On 04/23/2018 07:59 PM, Li Li wrote:
>>> https://www.spinics.net/lists/linux-media/msg115062.html
>>>
>>> Thanks for Daniel to fix this old issue. I might overlooked it but I
>>> didn't find it in the latest upstream kernel.
>>>
>>> Are we going to merge this missing patch? Thanks!
>>>
>>> Best,
>>> Li
>>>
>>
> 

[PATCH v2] [media] uvcvideo: Refactor teardown of uvc on USB disconnect

[Daniel Axtens]

Currently, disconnecting a USB webcam while it is in use prints out a
number of warnings, such as:

WARNING: CPU: 2 PID: 3118 at /build/linux-ezBi1T/linux-4.8.0/fs/sysfs/group.c:237 sysfs_remove_group+0x8b/0x90
sysfs group ffffffffa7cd0780 not found for kobject 'event13'

This has been noticed before. [0]

This is because of the order in which things are torn down.

If there are no streams active during a USB disconnect:

 - uvc_disconnect() is invoked via device_del() through the bus
   notifier mechanism.

 - this calls uvc_unregister_video().

 - uvc_unregister_video() unregisters the video device for each
   stream,

 - because there are no streams open, it calls uvc_delete()

 - uvc_delete() calls uvc_status_cleanup(), which cleans up the status
   input device.

 - uvc_delete() calls media_device_unregister(), which cleans up the
   media device

 - uvc_delete(), uvc_unregister_video() and uvc_disconnect() all
   return, and we end up back in device_del().

 - device_del() then cleans up the sysfs folder for the camera with
   dpm_sysfs_remove(). Because uvc_status_cleanup() and
   media_device_unregister() have already been called, this all works
   nicely.

If, on the other hand, there *are* streams active during a USB disconnect:

 - uvc_disconnect() is invoked

 - this calls uvc_unregister_video()

 - uvc_unregister_video() unregisters the video device for each
   stream,

 - uvc_unregister_video() and uvc_disconnect() return, and we end up
   back in device_del().

 - device_del() then cleans up the sysfs folder for the camera with
   dpm_sysfs_remove(). Because the status input device and the media
   device are children of the USB device, this also deletes their
   sysfs folders.

 - Sometime later, the final stream is closed, invoking uvc_release().

 - uvc_release() calls uvc_delete()

 - uvc_delete() calls uvc_status_cleanup(), which cleans up the status
   input device. Because the sysfs directory has already been removed,
   this causes a WARNing.

 - uvc_delete() calls media_device_unregister(), which cleans up the
   media device. Because the sysfs directory has already been removed,
   this causes another WARNing.

To fix this, we need to make sure the devices are always unregistered
before the end of uvc_disconnect(). To this, move the unregistration
into the disconnect path:

 - split uvc_status_cleanup() into two parts, one on disconnect that
   unregisters and one on delete that frees.

 - move v4l2_device_unregister() and media_device_unregister() into
   the disconnect path.

[0]: https://lkml.org/lkml/2016/12/8/657

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Dave Stevenson <linux-media@destevenson.freeserve.co.uk>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>

---

v2: Move v4l2_device_unregister() as well as media_device_unregister(),
    thanks Laurent.

Tested with cheese and yavta.

Laurent, I know you were concerned about race conditions so I plugged
and unplugged the camera over a dozen times, with lots of debugging
turned on.

In particular:
 - KASan was enabled and didn't trigger
 - If I unplugged while yavta was running, and replugged *while yatva
   was still running*, the video camera came up as video2, not video1.
   This indicates that yatva was successfully holding a reference to
   video1 - as it should.
 - No other debugging triggered any warning.
---
 drivers/media/usb/uvc/uvc_driver.c | 12 ++++++++----
 drivers/media/usb/uvc/uvc_status.c |  8 ++++++--
 drivers/media/usb/uvc/uvcvideo.h   |  1 +
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 46d6be0bb316..0d0541054ce2 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1812,11 +1812,7 @@ static void uvc_delete(struct uvc_device *dev)
 	usb_put_intf(dev->intf);
 	usb_put_dev(dev->udev);
 
-	if (dev->vdev.dev)
-		v4l2_device_unregister(&dev->vdev);
 #ifdef CONFIG_MEDIA_CONTROLLER
-	if (media_devnode_is_registered(dev->mdev.devnode))
-		media_device_unregister(&dev->mdev);
 	media_device_cleanup(&dev->mdev);
 #endif
 
@@ -1884,6 +1880,14 @@ static void uvc_unregister_video(struct uvc_device *dev)
 		uvc_debugfs_cleanup_stream(stream);
 	}
 
+	uvc_status_unregister(dev);
+	if (dev->vdev.dev)
+		v4l2_device_unregister(&dev->vdev);
+#ifdef CONFIG_MEDIA_CONTROLLER
+	if (media_devnode_is_registered(dev->mdev.devnode))
+		media_device_unregister(&dev->mdev);
+#endif
+
 	/* Decrement the stream count and call uvc_delete explicitly if there
 	 * are no stream left.
 	 */
diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
index f552ab997956..95709b23d3b4 100644
--- a/drivers/media/usb/uvc/uvc_status.c
+++ b/drivers/media/usb/uvc/uvc_status.c
@@ -198,12 +198,16 @@ int uvc_status_init(struct uvc_device *dev)
 	return 0;
 }
 
-void uvc_status_cleanup(struct uvc_device *dev)
+void uvc_status_unregister(struct uvc_device *dev)
 {
 	usb_kill_urb(dev->int_urb);
+	uvc_input_cleanup(dev);
+}
+
+void uvc_status_cleanup(struct uvc_device *dev)
+{
 	usb_free_urb(dev->int_urb);
 	kfree(dev->status);
-	uvc_input_cleanup(dev);
 }
 
 int uvc_status_start(struct uvc_device *dev, gfp_t flags)
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 15e415e32c7f..4b4814df35cd 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -712,6 +712,7 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
 
 /* Status */
 extern int uvc_status_init(struct uvc_device *dev);
+extern void uvc_status_unregister(struct uvc_device *dev);
 extern void uvc_status_cleanup(struct uvc_device *dev);
 extern int uvc_status_start(struct uvc_device *dev, gfp_t flags);
 extern void uvc_status_stop(struct uvc_device *dev);
-- 
2.11.0

Re: [PATCH v2] [media] uvcvideo: Refactor teardown of uvc on USB disconnect

[Greg KH]

On Sun, Apr 23, 2017 at 02:53:49PM +1000, Daniel Axtens wrote:
> Currently, disconnecting a USB webcam while it is in use prints out a
> number of warnings, such as:
> 
> WARNING: CPU: 2 PID: 3118 at /build/linux-ezBi1T/linux-4.8.0/fs/sysfs/group.c:237 sysfs_remove_group+0x8b/0x90
> sysfs group ffffffffa7cd0780 not found for kobject 'event13'
> 
> This has been noticed before. [0]
> 
> This is because of the order in which things are torn down.
> 
> If there are no streams active during a USB disconnect:
> 
>  - uvc_disconnect() is invoked via device_del() through the bus
>    notifier mechanism.
> 
>  - this calls uvc_unregister_video().
> 
>  - uvc_unregister_video() unregisters the video device for each
>    stream,
> 
>  - because there are no streams open, it calls uvc_delete()
> 
>  - uvc_delete() calls uvc_status_cleanup(), which cleans up the status
>    input device.
> 
>  - uvc_delete() calls media_device_unregister(), which cleans up the
>    media device
> 
>  - uvc_delete(), uvc_unregister_video() and uvc_disconnect() all
>    return, and we end up back in device_del().
> 
>  - device_del() then cleans up the sysfs folder for the camera with
>    dpm_sysfs_remove(). Because uvc_status_cleanup() and
>    media_device_unregister() have already been called, this all works
>    nicely.
> 
> If, on the other hand, there *are* streams active during a USB disconnect:
> 
>  - uvc_disconnect() is invoked
> 
>  - this calls uvc_unregister_video()
> 
>  - uvc_unregister_video() unregisters the video device for each
>    stream,
> 
>  - uvc_unregister_video() and uvc_disconnect() return, and we end up
>    back in device_del().
> 
>  - device_del() then cleans up the sysfs folder for the camera with
>    dpm_sysfs_remove(). Because the status input device and the media
>    device are children of the USB device, this also deletes their
>    sysfs folders.
> 
>  - Sometime later, the final stream is closed, invoking uvc_release().
> 
>  - uvc_release() calls uvc_delete()
> 
>  - uvc_delete() calls uvc_status_cleanup(), which cleans up the status
>    input device. Because the sysfs directory has already been removed,
>    this causes a WARNing.
> 
>  - uvc_delete() calls media_device_unregister(), which cleans up the
>    media device. Because the sysfs directory has already been removed,
>    this causes another WARNing.
> 
> To fix this, we need to make sure the devices are always unregistered
> before the end of uvc_disconnect(). To this, move the unregistration
> into the disconnect path:
> 
>  - split uvc_status_cleanup() into two parts, one on disconnect that
>    unregisters and one on delete that frees.
> 
>  - move v4l2_device_unregister() and media_device_unregister() into
>    the disconnect path.
> 
> [0]: https://lkml.org/lkml/2016/12/8/657
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Dave Stevenson <linux-media@destevenson.freeserve.co.uk>
> Cc: Greg KH <greg@kroah.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>


Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Very nice work, what a tangled web this is...

greg k-h

Re: [PATCH v2] [media] uvcvideo: Refactor teardown of uvc on USB disconnect

[Daniel Axtens]

Hi Laurent,

Just checking if this was OK with you - I hadn't heard anything and I
noticed it's not in -next so I just wanted to check to see if there were
any changes you wanted.

Regards,
Daniel

Daniel Axtens <dja@axtens.net> writes:

> Currently, disconnecting a USB webcam while it is in use prints out a
> number of warnings, such as:
>
> WARNING: CPU: 2 PID: 3118 at /build/linux-ezBi1T/linux-4.8.0/fs/sysfs/group.c:237 sysfs_remove_group+0x8b/0x90
> sysfs group ffffffffa7cd0780 not found for kobject 'event13'
>
> This has been noticed before. [0]
>
> This is because of the order in which things are torn down.
>
> If there are no streams active during a USB disconnect:
>
>  - uvc_disconnect() is invoked via device_del() through the bus
>    notifier mechanism.
>
>  - this calls uvc_unregister_video().
>
>  - uvc_unregister_video() unregisters the video device for each
>    stream,
>
>  - because there are no streams open, it calls uvc_delete()
>
>  - uvc_delete() calls uvc_status_cleanup(), which cleans up the status
>    input device.
>
>  - uvc_delete() calls media_device_unregister(), which cleans up the
>    media device
>
>  - uvc_delete(), uvc_unregister_video() and uvc_disconnect() all
>    return, and we end up back in device_del().
>
>  - device_del() then cleans up the sysfs folder for the camera with
>    dpm_sysfs_remove(). Because uvc_status_cleanup() and
>    media_device_unregister() have already been called, this all works
>    nicely.
>
> If, on the other hand, there *are* streams active during a USB disconnect:
>
>  - uvc_disconnect() is invoked
>
>  - this calls uvc_unregister_video()
>
>  - uvc_unregister_video() unregisters the video device for each
>    stream,
>
>  - uvc_unregister_video() and uvc_disconnect() return, and we end up
>    back in device_del().
>
>  - device_del() then cleans up the sysfs folder for the camera with
>    dpm_sysfs_remove(). Because the status input device and the media
>    device are children of the USB device, this also deletes their
>    sysfs folders.
>
>  - Sometime later, the final stream is closed, invoking uvc_release().
>
>  - uvc_release() calls uvc_delete()
>
>  - uvc_delete() calls uvc_status_cleanup(), which cleans up the status
>    input device. Because the sysfs directory has already been removed,
>    this causes a WARNing.
>
>  - uvc_delete() calls media_device_unregister(), which cleans up the
>    media device. Because the sysfs directory has already been removed,
>    this causes another WARNing.
>
> To fix this, we need to make sure the devices are always unregistered
> before the end of uvc_disconnect(). To this, move the unregistration
> into the disconnect path:
>
>  - split uvc_status_cleanup() into two parts, one on disconnect that
>    unregisters and one on delete that frees.
>
>  - move v4l2_device_unregister() and media_device_unregister() into
>    the disconnect path.
>
> [0]: https://lkml.org/lkml/2016/12/8/657
>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Dave Stevenson <linux-media@destevenson.freeserve.co.uk>
> Cc: Greg KH <greg@kroah.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
>
> ---
>
> v2: Move v4l2_device_unregister() as well as media_device_unregister(),
>     thanks Laurent.
>
> Tested with cheese and yavta.
>
> Laurent, I know you were concerned about race conditions so I plugged
> and unplugged the camera over a dozen times, with lots of debugging
> turned on.
>
> In particular:
>  - KASan was enabled and didn't trigger
>  - If I unplugged while yavta was running, and replugged *while yatva
>    was still running*, the video camera came up as video2, not video1.
>    This indicates that yatva was successfully holding a reference to
>    video1 - as it should.
>  - No other debugging triggered any warning.
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 12 ++++++++----
>  drivers/media/usb/uvc/uvc_status.c |  8 ++++++--
>  drivers/media/usb/uvc/uvcvideo.h   |  1 +
>  3 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 46d6be0bb316..0d0541054ce2 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1812,11 +1812,7 @@ static void uvc_delete(struct uvc_device *dev)
>  	usb_put_intf(dev->intf);
>  	usb_put_dev(dev->udev);
>  
> -	if (dev->vdev.dev)
> -		v4l2_device_unregister(&dev->vdev);
>  #ifdef CONFIG_MEDIA_CONTROLLER
> -	if (media_devnode_is_registered(dev->mdev.devnode))
> -		media_device_unregister(&dev->mdev);
>  	media_device_cleanup(&dev->mdev);
>  #endif
>  
> @@ -1884,6 +1880,14 @@ static void uvc_unregister_video(struct uvc_device *dev)
>  		uvc_debugfs_cleanup_stream(stream);
>  	}
>  
> +	uvc_status_unregister(dev);
> +	if (dev->vdev.dev)
> +		v4l2_device_unregister(&dev->vdev);
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +	if (media_devnode_is_registered(dev->mdev.devnode))
> +		media_device_unregister(&dev->mdev);
> +#endif
> +
>  	/* Decrement the stream count and call uvc_delete explicitly if there
>  	 * are no stream left.
>  	 */
> diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
> index f552ab997956..95709b23d3b4 100644
> --- a/drivers/media/usb/uvc/uvc_status.c
> +++ b/drivers/media/usb/uvc/uvc_status.c
> @@ -198,12 +198,16 @@ int uvc_status_init(struct uvc_device *dev)
>  	return 0;
>  }
>  
> -void uvc_status_cleanup(struct uvc_device *dev)
> +void uvc_status_unregister(struct uvc_device *dev)
>  {
>  	usb_kill_urb(dev->int_urb);
> +	uvc_input_cleanup(dev);
> +}
> +
> +void uvc_status_cleanup(struct uvc_device *dev)
> +{
>  	usb_free_urb(dev->int_urb);
>  	kfree(dev->status);
> -	uvc_input_cleanup(dev);
>  }
>  
>  int uvc_status_start(struct uvc_device *dev, gfp_t flags)
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 15e415e32c7f..4b4814df35cd 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -712,6 +712,7 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
>  
>  /* Status */
>  extern int uvc_status_init(struct uvc_device *dev);
> +extern void uvc_status_unregister(struct uvc_device *dev);
>  extern void uvc_status_cleanup(struct uvc_device *dev);
>  extern int uvc_status_start(struct uvc_device *dev, gfp_t flags);
>  extern void uvc_status_stop(struct uvc_device *dev);
> -- 
> 2.11.0

Re: [PATCH v2] [media] uvcvideo: Refactor teardown of uvc on USB disconnect

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Sunday, 23 April 2017 07:53:49 EET Daniel Axtens wrote:
> Currently, disconnecting a USB webcam while it is in use prints out a
> number of warnings, such as:
> 
> WARNING: CPU: 2 PID: 3118 at
> /build/linux-ezBi1T/linux-4.8.0/fs/sysfs/group.c:237
> sysfs_remove_group+0x8b/0x90 sysfs group ffffffffa7cd0780 not found for
> kobject 'event13'
> 
> This has been noticed before. [0]
> 
> This is because of the order in which things are torn down.
> 
> If there are no streams active during a USB disconnect:
> 
>  - uvc_disconnect() is invoked via device_del() through the bus
>    notifier mechanism.
> 
>  - this calls uvc_unregister_video().
> 
>  - uvc_unregister_video() unregisters the video device for each
>    stream,
> 
>  - because there are no streams open, it calls uvc_delete()
> 
>  - uvc_delete() calls uvc_status_cleanup(), which cleans up the status
>    input device.
> 
>  - uvc_delete() calls media_device_unregister(), which cleans up the
>    media device
> 
>  - uvc_delete(), uvc_unregister_video() and uvc_disconnect() all
>    return, and we end up back in device_del().
> 
>  - device_del() then cleans up the sysfs folder for the camera with
>    dpm_sysfs_remove(). Because uvc_status_cleanup() and
>    media_device_unregister() have already been called, this all works
>    nicely.
> 
> If, on the other hand, there *are* streams active during a USB disconnect:
> 
>  - uvc_disconnect() is invoked
> 
>  - this calls uvc_unregister_video()
> 
>  - uvc_unregister_video() unregisters the video device for each
>    stream,
> 
>  - uvc_unregister_video() and uvc_disconnect() return, and we end up
>    back in device_del().
> 
>  - device_del() then cleans up the sysfs folder for the camera with
>    dpm_sysfs_remove(). Because the status input device and the media
>    device are children of the USB device, this also deletes their
>    sysfs folders.
> 
>  - Sometime later, the final stream is closed, invoking uvc_release().
> 
>  - uvc_release() calls uvc_delete()
> 
>  - uvc_delete() calls uvc_status_cleanup(), which cleans up the status
>    input device. Because the sysfs directory has already been removed,
>    this causes a WARNing.
> 
>  - uvc_delete() calls media_device_unregister(), which cleans up the
>    media device. Because the sysfs directory has already been removed,
>    this causes another WARNing.
> 
> To fix this, we need to make sure the devices are always unregistered
> before the end of uvc_disconnect(). To this, move the unregistration
> into the disconnect path:
> 
>  - split uvc_status_cleanup() into two parts, one on disconnect that
>    unregisters and one on delete that frees.
> 
>  - move v4l2_device_unregister() and media_device_unregister() into
>    the disconnect path.
> 
> [0]: https://lkml.org/lkml/2016/12/8/657
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Dave Stevenson <linux-media@destevenson.freeserve.co.uk>
> Cc: Greg KH <greg@kroah.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> 
> ---
> 
> v2: Move v4l2_device_unregister() as well as media_device_unregister(),
>     thanks Laurent.
> 
> Tested with cheese and yavta.
> 
> Laurent, I know you were concerned about race conditions so I plugged
> and unplugged the camera over a dozen times, with lots of debugging
> turned on.
> 
> In particular:
>  - KASan was enabled and didn't trigger
>  - If I unplugged while yavta was running, and replugged *while yatva
>    was still running*, the video camera came up as video2, not video1.
>    This indicates that yatva was successfully holding a reference to
>    video1 - as it should.
>  - No other debugging triggered any warning.

Thank you for running those tests. I'm still a bit concerned but I'll live 
with it. The issues are in the MC and V4L2 core anyway, there isn't much we 
could do here to fix them.

> ---
>  drivers/media/usb/uvc/uvc_driver.c | 12 ++++++++----
>  drivers/media/usb/uvc/uvc_status.c |  8 ++++++--
>  drivers/media/usb/uvc/uvcvideo.h   |  1 +
>  3 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index 46d6be0bb316..0d0541054ce2
> 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1812,11 +1812,7 @@ static void uvc_delete(struct uvc_device *dev)
>  	usb_put_intf(dev->intf);
>  	usb_put_dev(dev->udev);
> 
> -	if (dev->vdev.dev)
> -		v4l2_device_unregister(&dev->vdev);
>  #ifdef CONFIG_MEDIA_CONTROLLER
> -	if (media_devnode_is_registered(dev->mdev.devnode))
> -		media_device_unregister(&dev->mdev);
>  	media_device_cleanup(&dev->mdev);
>  #endif
> 
> @@ -1884,6 +1880,14 @@ static void uvc_unregister_video(struct uvc_device
> *dev) uvc_debugfs_cleanup_stream(stream);
>  	}
> 
> +	uvc_status_unregister(dev);
> +	if (dev->vdev.dev)
> +		v4l2_device_unregister(&dev->vdev);
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +	if (media_devnode_is_registered(dev->mdev.devnode))
> +		media_device_unregister(&dev->mdev);
> +#endif
> +
>  	/* Decrement the stream count and call uvc_delete explicitly if there
>  	 * are no stream left.
>  	 */
> diff --git a/drivers/media/usb/uvc/uvc_status.c
> b/drivers/media/usb/uvc/uvc_status.c index f552ab997956..95709b23d3b4
> 100644
> --- a/drivers/media/usb/uvc/uvc_status.c
> +++ b/drivers/media/usb/uvc/uvc_status.c
> @@ -198,12 +198,16 @@ int uvc_status_init(struct uvc_device *dev)
>  	return 0;
>  }
> 
> -void uvc_status_cleanup(struct uvc_device *dev)
> +void uvc_status_unregister(struct uvc_device *dev)
>  {
>  	usb_kill_urb(dev->int_urb);
> +	uvc_input_cleanup(dev);

I think it would make sense to rename uvc_input_cleanup() to 
uvc_input_unregister(). I'll make the change locally so there's no need to 
resubmit this patch.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

and applied to my tree.

> +}
> +
> +void uvc_status_cleanup(struct uvc_device *dev)
> +{
>  	usb_free_urb(dev->int_urb);
>  	kfree(dev->status);
> -	uvc_input_cleanup(dev);
>  }
> 
>  int uvc_status_start(struct uvc_device *dev, gfp_t flags)

[snip]

-- 
Regards,

Laurent Pinchart