On Fri, Jul 31, 2020 at 02:06:20PM +0000, Peter Chen wrote: > On 20-07-31 14:25:20, Greg Kroah-Hartman wrote: > > On Fri, Jul 31, 2020 at 12:11:32PM +0000, Peter Chen wrote: > > > > Grab a reference from somewhere else and do not give it up for a long > > time. > > > > So wait_for_completion_timeout is suitable? NO!!! > The similar use case is when > we open the file at the USB Drive at Windows, and we click "Eject", it > will say "The device is currently in use", and refuse our "Eject" > operation. > > When we try to remove the gadget, if the gadget is in use, we could > refuse the remove operation, reasonable? Nope. Remove it please. > > > > > -static void usb_udc_nop_release(struct device *dev) > > > > > +static void usb_gadget_release(struct device *dev) > > > > > { > > > > > + struct usb_gadget *gadget; > > > > > + > > > > > dev_vdbg(dev, "%s\n", __func__); > > > > > + > > > > > + gadget = container_of(dev, struct usb_gadget, dev); > > > > > + complete(&gadget->done); > > > > > + memset(dev, 0x0, sizeof(*dev)); > > > > > > > > No, the memory should be freed here, not memset. > > > > > > > > > > This memory is allocated at UDC driver and is freed by UDC driver too. > > > > That's wrong, the release function should be where this is released. > > So, the release function should be at individual UDC driver, a common > release function is improper, right? Depends on how this all works, but again, the release function needs to free the memory, otherwise this is broken. > > And this no-op function is horrid. There used to be documentation in > > the kernel where I could rant about this, but instead, I'll just say, > > "why are people trying to work around warnings we put in the core kernel > > to fix common problems? Do they think we did that just because we > > wanted to be mean???" > > > > So, like kernel doc for device_initialize said, a proper fix for dwc3 > should be zeroed gadget device memory at its own driver before the > gadget device register to driver core, right? It should get a totally different, dynamically allocated structure. NEVER recycle them. thanks, greg k-h