>From stern+4e0774f0@rowland.harvard.edu Wed Jul 6 14:03:47 2011 Return-Path: X-OfflineIMAP: 2303515278-5823848424 Delivered-To: unknown Received: from imap.suse.de ([unix socket]) by imap-int (Cyrus v2.2.12) with LMTPA; Wed, 06 Jul 2011 23:03:47 +0200 X-Sieve: CMU Sieve 2.2 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "relay.suse.de", Issuer "CAcert Class 3 Root" (verified OK)) by imap.suse.de (Postfix) with ESMTPS id 685C43C539A9 for ; Wed, 6 Jul 2011 23:03:47 +0200 (CEST) Received: by relay2.suse.de (Postfix) id 5F09B1855766; Wed, 6 Jul 2011 23:03:47 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by relay2.suse.de (Postfix) with ESMTP id 53D4018552DE for ; Wed, 6 Jul 2011 23:03:47 +0200 (CEST) Received: from relay2.suse.de ([127.0.0.1]) by localhost (localhost [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 07676-11 for ; Wed, 6 Jul 2011 23:03:47 +0200 (CEST) Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 0B33018552CC for ; Wed, 6 Jul 2011 23:03:47 +0200 (CEST) Received: from iolanthe.rowland.org (iolanthe.rowland.org [192.131.102.54]) by mx2.suse.de (Postfix) with SMTP id A86F58B2F9 for ; Wed, 6 Jul 2011 23:03:46 +0200 (CEST) Received: (qmail 6472 invoked by uid 2102); 6 Jul 2011 17:03:45 -0400 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 6 Jul 2011 17:03:45 -0400 Date: Wed, 6 Jul 2011 17:03:45 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Greg KH Cc: Arkadiusz Miskiewicz , =?iso-8859-1?Q?=C9ric?= Piel , Sarah Sharp , LKML , USB list , "Rafael J. Wysocki" Subject: [PATCH] USB: additional regression fix for device removal In-Reply-To: <20110706204647.GC1460@suse.de> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Virus-Scanned: by amavisd-new at localhost X-Spam-Status: No, score=0.001 tagged_above=-20 required=5 tests=[BAYES_50=0.001] X-Spam-Score: 0.001 X-Spam-Level: Status: RO Content-Length: 2323 Commit e534c5b831c8b8e9f5edee5c8a37753c808b80dc (USB: fix regression occurring during device removal) didn't go far enough. It failed to take into account that when a driver claims multiple interfaces, it may release them all at the same time. As a result, some interfaces can get released before they are unregistered, and we deadlock trying to acquire the bandwidth_mutex that we already own. This patch (asl478) handles this case by setting the "unregistering" flag on all the interfaces before removing any of them. Signed-off-by: Alan Stern CC: --- This should take care of Eric's problem as well as Arkadiusz, since they seemed to be hitting the same thing (cdc_ether claiming multiple interfaces and hanging while releasing them). Still, we need to rewrite this stuff. A possible race remains, because a driver may try to change an altsetting at the same time as the device is removed. Either the driver's disconnect routine would hang waiting for the altsetting change (which is waiting to acquire the bandwidth_mutex) or else the altsetting change would go through after the driver was unbound from the device. Neither alternative is good. Alan Stern drivers/usb/core/message.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) Index: usb-3.0/drivers/usb/core/message.c =================================================================== --- usb-3.0.orig/drivers/usb/core/message.c +++ usb-3.0/drivers/usb/core/message.c @@ -1147,6 +1147,14 @@ void usb_disable_device(struct usb_devic * any drivers bound to them (a key side effect) */ if (dev->actconfig) { + /* + * FIXME: In order to avoid self-deadlock involving the + * bandwidth_mutex, we have to mark all the interfaces + * before unregistering any of them. + */ + for (i = 0; i < dev->actconfig->desc.bNumInterfaces; i++) + dev->actconfig->interface[i]->unregistering = 1; + for (i = 0; i < dev->actconfig->desc.bNumInterfaces; i++) { struct usb_interface *interface; @@ -1156,7 +1164,6 @@ void usb_disable_device(struct usb_devic continue; dev_dbg(&dev->dev, "unregistering interface %s\n", dev_name(&interface->dev)); - interface->unregistering = 1; remove_intf_ep_devs(interface); device_del(&interface->dev); }