On Fri, Jul 31, 2020 at 05:59:35PM +0800, Peter Chen wrote: > Per discussion[1], to avoid UDC driver possible freeing gadget device > memory before device core finishes using it, we add wait-complete > mechanism at usb_del_gadget_udc and gadget device .release callback. > After that, usb_del_gadget_udc will not return back until device > core finishes using gadget device. Ick, no, that's a sure way for a deadlock to happen. Why does the gadget core care about this at all? It shouldn't. > > For UDC drivers who have own .release callback, it needs to call > complete(&gadget->done) by themselves, if not, the UDC core will > handle it by default .release callback usb_gadget_release. > > [1] https://www.spinics.net/lists/linux-usb/msg198790.html > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > Signed-off-by: Peter Chen <peter.chen@xxxxxxx> > --- > If this RFC patch is ok, I will create the formal patches which will change > UDC drivers who have their own .release function. > > drivers/usb/gadget/udc/core.c | 14 +++++++++++--- > include/linux/usb/gadget.h | 2 ++ > 2 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > index ee226ad802a4..ed141e1a0dcf 100644 > --- a/drivers/usb/gadget/udc/core.c > +++ b/drivers/usb/gadget/udc/core.c > @@ -1138,9 +1138,15 @@ static void usb_udc_release(struct device *dev) > > static const struct attribute_group *usb_udc_attr_groups[]; > > -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. thanks, greg k-h
