On Fri, Jul 21, 2023 at 04:57:51PM +0100, Dan Scally wrote: > Hi Avichal - thanks for all the detail > > On 20/07/2023 02:28, Avichal Rakesh wrote: > > This looks like a side effect of > > https://lore.kernel.org/lkml/20230608204517.105396-1-badhri@xxxxxxxxxx/. > > Effectively, UVC function tried to disconnect the gadget before > > cleaning up resources. However, usb_gadget_unregister_driver which is > > removing the function prevents the gadget from disconnecting until the > > function is unbound. > > A dirty Patch for option 2 is attached below which skips calling > > usb_function_deactivate if uvc_function_disable was called before. It seems > > to work okay in testing. Let me know if the analysis and solutions seems okay > > and I can upload a formal patch. > > > For what it's worth the analysis makes sense; the patch looks ok to me so if > the conclusion is to fix the problem that way I think it's fine, but I'm > more inclined to consider this a locking problem in core - it'd be better to > fix it there I think. I'm not so sure that handling this in the core is feasible. Removing the driver obviously needs to be synchronized with deactivation, since the two actions affect the same parts of the state (i.e., the pull-ups and the "connected" flag). Consequently I don't see how to avoid a deadlock if the driver's unbind callback does a deactivate. Besides, as the patch mentions, doing so is never necessary. However, even with that call removed we could still have a problem. I don't know much about how the UVC function driver works, but it would be reasonable for the driver to have a private mutex that gets held both during unbind and when the user application closes the V4L2 node. Then there's an ABBA locking issue: Unbind: The UDC core holds connect_lock while calling the UVC unbind handler, which needs to acquire the private mutex. Close node: The UVC driver holds the private mutex while doing a deactivate, which needs to acquire connect_lock. Any ideas on how to clear this up? Alan Stern
