2009/4/8 Ming Lei <tom.leiming@xxxxxxxxx>: > 2009/4/8 Greg KH <greg@xxxxxxxxx>: >> On Tue, Apr 07, 2009 at 11:08:34PM +0800, tom.leiming@xxxxxxxxx wrote: >>> From: Ming Lei <tom.leiming@xxxxxxxxx> >>> >>> The deadlock is caused by mutex_lock(&port->serial->disc_mutex) >>> (called in garmin_close() and usb_serial_disconnect() ). The garmin >>> usb_driver doesn't define .pre_reset and .post_reset, so >>> usb_reset_devie() will unbound garmin usb_driver before doing bus >>> reset, which makes .disconnect() called, then deadlock happens. >>> >>> This patch defines .pre_reset and .post_reset in garmin usb_driver >>> to prevent the usb driver from being unbound in usb_reset_device() >>> called by garmin_close. >>> >>> This patch fixes the bug reported in the link: >>> http://bugs.archlinux.org/task/12079 >>> >>> Signed-off-by: Ming Lei <tom.leiming@xxxxxxxxx> >>> Reported-by:Hal Murray <hmurray@xxxxxxxxxxxxxxx> >>> Tested-by:Hal Murray <hmurray@xxxxxxxxxxxxxxx> >> >> Didn't Alan Stern also do some work here on this patch? > > No, this patch fixes a dead lock bug in garmin_driver, and > Alan's patch is to fix a incorrect order of REMOVE uevent , > seems. > >> >>> +static int garmin_pre_reset(struct usb_interface *intf) >>> +{ >>> + return 0; >>> +} >>> + >>> +static int garmin_post_reset(struct usb_interface *intf) >>> +{ >>> + return 0; >>> +} >> >> That's it? Shouldn these be the "default" for all usb drivers if they >> don't provide such a function? This seems to be papering over a real >> problem here... > > If all usb drivers need this kind of default functions, it seems there > is a real problem > in usb_reset_device(). Maybe usb_reset_device() should not unbind a > driver if the driver > doesn't have .pre_reset and .post_reset, right? Alan, I don't know why you does unbind a driver before resetting the device if the driver has not .pre_reset and .post_reset. Would you mind giving a explanation? Thanks. > >> >>> + >>> static struct usb_driver garmin_driver = { >>> .name = "garmin_gps", >>> .probe = usb_serial_probe, >>> .disconnect = usb_serial_disconnect, >>> + .pre_reset = garmin_pre_reset, >>> + .post_reset = garmin_post_reset, >> >> Any reason to change the formatting? >> >> thanks, >> >> greg k-h >> > > > > -- > Lei Ming > -- Lei Ming -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
