This patch should fix the problem reported below by Daniel Dorau. The
oops was caused by inadequate synchronization between khubd and
hub_disconnect(). The hub interface-data pointer was reset by the
disconnect routine while khubd was still using it. The revised code
doesn't erase the pointer until after khubd has terminated.
The patch also improves the synchronization with hub_irq(). The current
code could, under the proper race conditions, resubmit the hub URB during
an rmmod. Alternatively, it could unlink the URB but then exit and unload
before the callback routine had run. Proper use of a struct completion
and an urb_active flag will prevent either of those errors.
Finally, the patch changes spin_lock_save() to spin_lock() within
the completion routine (which always runs with local interrupts disabled)
and replaces a couple of list_del()/INIT_LIST_HEAD() sequences with
list_del_init().
Alan Stern
On Wed, 29 Oct 2003, Daniel Dorau wrote:
> Hello,
> I have an Oops here on Linux 2.6.0test8. Hardware is ThinkPad X31, USB
> 2.0 hub connected. There was no device connected to that hub and no
> other device connected to the ThinkPad.
> After I booted Linux, I waited just a moment, then rebooted.
> On shutting down, I had that Oops. I hope you can pull enough
> information out of it to fix it. Here it is:
> Oct 24 21:33:16 haktar kernel: usb usb3: USB disconnect, address 1
> Oct 24 21:33:16 haktar kernel: usb usb3: usb_disable_device nuking all URBs
> Oct 24 21:33:16 haktar kernel: uhci_hcd 0000:00:1d.1: shutdown urb df74a860 pipe
> 40408180 ep1in-intr
> Oct 24 21:33:16 haktar kernel: usb usb3: unregistering interface 3-0:1.0
> Oct 24 21:33:16 haktar kernel: Unable to handle kernel NULL pointer dereference at
> virtual address 00000010
> Oct 24 21:33:16 haktar kernel: printing eip:
> Oct 24 21:33:16 haktar kernel: c0298e82
> Oct 24 21:33:16 haktar kernel: *pde = 00000000
> Oct 24 21:33:16 haktar kernel: Oops: 0000 [#1]
> Oct 24 21:33:16 haktar kernel: CPU: 0
> Oct 24 21:33:16 haktar kernel: EIP: 0060:[hub_port_status+22/136] Not tainted
> Oct 24 21:33:16 haktar kernel: EFLAGS: 00010286
> Oct 24 21:33:16 haktar kernel: EIP is at hub_port_status+0x16/0x88
> Oct 24 21:33:16 haktar kernel: eax: dfd223d4 ebx: 00000000 ecx: 00000000 edx:
> dfa04800
> Oct 24 21:33:16 haktar kernel: esi: 00000002 edi: dfa04800 ebp: 00000001 esp:
> dfda1f18
> Oct 24 21:33:16 haktar kernel: ds: 007b es: 007b ss: 0068
> Oct 24 21:33:16 haktar kernel: Process khubd (pid: 5, threadinfo=dfda0000
> task=c15fe040)
> Oct 24 21:33:16 haktar kernel: Stack: 00000002 00000002 0000004b c02991d1 dfa04800
> 00000001 dfda1f54 dfda1f56
> Oct 24 21:33:16 haktar kernel: 00000002 00000101 dfa04800 00000002 dfda1f56
> dfda1f54 dfda0000 00000101
> Oct 24 21:33:16 haktar kernel: c0299361 dfa04800 00000001 00000002 c16a2ca0
> 00000001 dfa04800 00000004
> Oct 24 21:33:16 haktar kernel: Call Trace:
> Oct 24 21:33:16 haktar kernel: [hub_port_debounce+121/296]
> hub_port_debounce+0x79/0x128
> Oct 24 21:33:16 haktar kernel: [hub_port_connect_change+225/816]
> hub_port_connect_change+0xe1/0x330
> Oct 24 21:33:16 haktar kernel: [hub_events+365/952] hub_events+0x16d/0x3b8
> Oct 24 21:33:16 haktar kernel: [hub_thread+45/244] hub_thread+0x2d/0xf4
> Oct 24 21:33:16 haktar kernel: [hub_thread+0/244] hub_thread+0x0/0xf4
> Oct 24 21:33:16 haktar kernel: [default_wake_function+0/32]
> default_wake_function+0x0/0x20
> Oct 24 21:33:16 haktar kernel: [kernel_thread_helper+5/12]
> kernel_thread_helper+0x5/0xc
> Oct 24 21:33:16 haktar kernel:
> Oct 24 21:33:16 haktar kernel: Code: 8b 43 10 50 8b 44 24 18 40 50 57 e8 3e f0 ff ff
> 89 c6 83 c4
===== hub.c 1.123 vs edited =====
--- 1.123/drivers/usb/core/hub.c Mon Sep 22 12:37:50 2003
+++ edited/drivers/usb/core/hub.c Wed Oct 29 15:51:32 2003
@@ -126,14 +126,20 @@
static void hub_irq(struct urb *urb, struct pt_regs *regs)
{
struct usb_hub *hub = (struct usb_hub *)urb->context;
- unsigned long flags;
int status;
+ spin_lock(&hub_event_lock);
+ hub->urb_active = 0;
+ if (hub->urb_complete) { /* disconnect or rmmod */
+ complete(hub->urb_complete);
+ goto done;
+ }
+
switch (urb->status) {
case -ENOENT: /* synchronous unlink */
case -ECONNRESET: /* async unlink */
case -ESHUTDOWN: /* hardware going away */
- return;
+ goto done;
default: /* presumably an error */
/* Cause a hub reset after 10 consecutive errors */
@@ -151,18 +157,20 @@
hub->nerrors = 0;
/* Something happened, let khubd figure it out */
- spin_lock_irqsave(&hub_event_lock, flags);
if (list_empty(&hub->event_list)) {
list_add(&hub->event_list, &hub_event_list);
wake_up(&khubd_wait);
}
- spin_unlock_irqrestore(&hub_event_lock, flags);
resubmit:
if ((status = usb_submit_urb (hub->urb, GFP_ATOMIC)) != 0
/* ENODEV means we raced disconnect() */
&& status != -ENODEV)
dev_err (&hub->intf->dev, "resubmit --> %d\n", urb->status);
+ if (status == 0)
+ hub->urb_active = 1;
+done:
+ spin_unlock(&hub_event_lock);
}
/* USB 2.0 spec Section 11.24.2.3 */
@@ -467,7 +475,8 @@
message = "couldn't submit status urb";
goto fail;
}
-
+ hub->urb_active = 1;
+
/* Wake up khubd */
wake_up(&khubd_wait);
@@ -486,18 +495,17 @@
{
struct usb_hub *hub = usb_get_intfdata (intf);
unsigned long flags;
+ DECLARE_COMPLETION(urb_complete);
if (!hub)
return;
- usb_set_intfdata (intf, NULL);
spin_lock_irqsave(&hub_event_lock, flags);
+ hub->urb_complete = &urb_complete;
/* Delete it and then reset it */
- list_del(&hub->event_list);
- INIT_LIST_HEAD(&hub->event_list);
- list_del(&hub->hub_list);
- INIT_LIST_HEAD(&hub->hub_list);
+ list_del_init(&hub->event_list);
+ list_del_init(&hub->hub_list);
spin_unlock_irqrestore(&hub_event_lock, flags);
@@ -510,6 +518,8 @@
if (hub->urb) {
usb_unlink_urb(hub->urb);
+ if (hub->urb_active)
+ wait_for_completion(&urb_complete);
usb_free_urb(hub->urb);
hub->urb = NULL;
}
@@ -530,6 +540,8 @@
hub->buffer_dma);
hub->buffer = NULL;
}
+
+ usb_set_intfdata (intf, NULL);
/* Free the memory */
kfree(hub);
===== hub.h 1.20 vs edited =====
--- 1.20/drivers/usb/core/hub.h Wed Jun 11 12:16:43 2003
+++ edited/drivers/usb/core/hub.h Wed Oct 29 15:20:10 2003
@@ -172,6 +172,8 @@
struct usb_hub {
struct usb_interface *intf; /* the "real" device */
struct urb *urb; /* for interrupt polling pipe */
+ struct completion *urb_complete; /* wait for urb to end */
+ unsigned int urb_active:1;
/* buffer for urb ... 1 bit each for hub and children, rounded up */
char (*buffer)[(USB_MAXCHILDREN + 1 + 7) / 8];
-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive? Does it
help you create better code? SHARE THE LOVE, and help us help
YOU! Click Here: http://sourceforge.net/donate/
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
