This revised patch (as893b) improves the method used by the hub driver to
release its private data structure. The current code is non-robust,
relying on a memory region not getting reused by another driver after
it has been freed.
Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
---
It turned out that making khubd responsible doing the final usb_put_intf()
wasn't such a good idea. It caused a problem with dummy_hcd. Doing
things this way should be better; now all device references held by the
hub driver will be dropped before hub_disconnect() returns.
Index: usb-2.6/drivers/usb/core/hub.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/hub.c
+++ usb-2.6/drivers/usb/core/hub.c
@@ -49,6 +49,9 @@ struct usb_hub {
int error; /* last reported error */
int nerrors; /* track consecutive errors */
+ struct completion *finish; /* signal that khubd is done
+ using this hub */
+
struct list_head event_list; /* hubs w/data or errs ready */
unsigned long event_bits[1]; /* status change bitmask */
unsigned long change_bits[1]; /* ports with logical connect
@@ -67,12 +70,15 @@ struct usb_hub {
unsigned limited_power:1;
unsigned quiescing:1;
unsigned activating:1;
+ unsigned disconnected:1;
unsigned has_indicators:1;
u8 indicator[USB_MAXCHILDREN];
struct delayed_work leds;
};
+/* Sentinel value for hub->finish to indicate khubd is using the hub */
+#define HUB_IN_USE(hub) ((struct completion *) (hub))
/* Protect struct usb_device->state and ->children members
* Note: Both are also protected by ->dev.sem, except that ->state can
@@ -324,7 +330,7 @@ static void kick_khubd(struct usb_hub *h
to_usb_interface(hub->intfdev)->pm_usage_cnt = 1;
spin_lock_irqsave(&hub_event_lock, flags);
- if (list_empty(&hub->event_list)) {
+ if (!hub->disconnected & list_empty(&hub->event_list)) {
list_add_tail(&hub->event_list, &hub_event_list);
wake_up(&khubd_wait);
}
@@ -333,6 +339,7 @@ static void kick_khubd(struct usb_hub *h
void usb_kick_khubd(struct usb_device *hdev)
{
+ /* FIXME: What if hdev isn't bound to the hub driver? */
kick_khubd(hdev_to_hub(hdev));
}
@@ -931,6 +938,21 @@ static void hub_disconnect(struct usb_in
{
struct usb_hub *hub = usb_get_intfdata (intf);
struct usb_device *hdev;
+ struct completion wait_for_khubd;
+
+ /* Take the hub off the event list */
+ spin_lock_irq(&hub_event_lock);
+ hub->disconnected = 1;
+ list_del_init(&hub->event_list);
+
+ /* Wait for khubd to finish using the hub */
+ if (hub->finish == HUB_IN_USE(hub)) {
+ hub->finish = &wait_for_khubd;
+ init_completion(&wait_for_khubd);
+ }
+ spin_unlock_irq(&hub_event_lock);
+ if (hub->finish)
+ wait_for_completion(&wait_for_khubd);
/* Disconnect all children and quiesce the hub */
hub->error = 0;
@@ -945,10 +967,6 @@ static void hub_disconnect(struct usb_in
usb_free_urb(hub->urb);
hub->urb = NULL;
- spin_lock_irq(&hub_event_lock);
- list_del_init(&hub->event_list);
- spin_unlock_irq(&hub_event_lock);
-
kfree(hub->descriptor);
hub->descriptor = NULL;
@@ -2600,19 +2618,19 @@ static void hub_events(void)
* safe since we delete the hub from the event list.
* Not the most efficient, but avoids deadlocks.
*/
+ spin_lock_irq(&hub_event_lock);
while (1) {
/* Grab the first entry at the beginning of the list */
- spin_lock_irq(&hub_event_lock);
- if (list_empty(&hub_event_list)) {
- spin_unlock_irq(&hub_event_lock);
+ if (list_empty(&hub_event_list))
break;
- }
-
tmp = hub_event_list.next;
list_del_init(tmp);
hub = list_entry(tmp, struct usb_hub, event_list);
+ hub->finish = HUB_IN_USE(hub);
+ spin_unlock_irq(&hub_event_lock);
+
hdev = hub->hdev;
intf = to_usb_interface(hub->intfdev);
hub_dev = &intf->dev;
@@ -2625,13 +2643,10 @@ static void hub_events(void)
(u16) hub->change_bits[0],
(u16) hub->event_bits[0]);
- usb_get_intf(intf);
- spin_unlock_irq(&hub_event_lock);
-
/* Lock the device, then check to see if we were
* disconnected while waiting for the lock to succeed. */
usb_lock_device(hdev);
- if (hub != usb_get_intfdata(intf))
+ if (unlikely(hub->disconnected))
goto loop;
/* If the hub has died, clean up after it */
@@ -2794,9 +2809,17 @@ loop_autopm:
usb_autopm_enable(intf);
loop:
usb_unlock_device(hdev);
- usb_put_intf(intf);
+ /* If hub_disconnect() is waiting for us to finish using
+ * the hub, tell it we are now done.
+ */
+ spin_lock_irq(&hub_event_lock);
+ if (hub->finish == HUB_IN_USE(hub))
+ hub->finish = NULL;
+ else
+ complete(hub->finish);
} /* end while (1) */
+ spin_unlock_irq(&hub_event_lock);
}
static int hub_thread(void *__unused)
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
