On Tue, Sep 11, 2012 at 11:34:56AM -0400, Alan Stern wrote:
> On Mon, 10 Sep 2012, Don Zickus wrote:
>
> > A customer of ours noticed that after a bunch of hot removals of the EHCI
> > PCI device, a panic occurs. This happened on a 2.6.32 RHEL-6 kernel, but
> > I believe is still applicable upstream.
> >
> > The panic was further simplified to enabling SLUB debug poisoning and running
> > the following command:
> >
> > while true; do find /proc/bus/usb -type f -exec cat {} >/dev/null \; ; done &
> >
> > This gets the machine to panic after 1 or 2 removals of the device.
>
> > This is likely because the following thread was in the process of removing the
> > device:
>
> > The fix that solved their problem was to deregister the usb bus first before
> > running usb_put_dev. After running multiple tests the panic disappeared.
> >
> > Deregistering the bus first to prevent future readers from accessing the device
> > before cleaning up the hcd, seemed like an appropriate way to go. I'll leave
> > it to the experts to validate that or provide a better solution. :-)
> >
> > I adapted their usb_remove_hcd fix for usb_add_hcd error path too to mimic
> > the same behaviour (a little code shuffling to handle the gotos cleanly).
>
> Moving things around in usb_add_hcd() is not a good way to attack this
> problem. Here's a better approach; please make sure that it does what
> you want. The idea is to indicate that a root hub is not registered by
> setting its devnum field to 0.
Thanks for the feedback. I will pass this along for testing and get back
to you.
Thanks,
Don
>
> Alan Stern
>
>
>
> Index: usb-3.6/drivers/usb/core/devices.c
> ===================================================================
> --- usb-3.6.orig/drivers/usb/core/devices.c
> +++ usb-3.6/drivers/usb/core/devices.c
> @@ -624,7 +624,7 @@ static ssize_t usb_device_read(struct fi
> /* print devices for all busses */
> list_for_each_entry(bus, &usb_bus_list, bus_list) {
> /* recurse through all children of the root hub */
> - if (!bus->root_hub)
> + if (!bus->root_hub || bus->root_hub->devnum == 0)
> continue;
> usb_lock_device(bus->root_hub);
> ret = usb_device_dump(&buf, &nbytes, &skip_bytes, ppos,
> Index: usb-3.6/drivers/usb/core/hcd.c
> ===================================================================
> --- usb-3.6.orig/drivers/usb/core/hcd.c
> +++ usb-3.6/drivers/usb/core/hcd.c
> @@ -980,6 +980,8 @@ static int register_root_hub(struct usb_
> const int devnum = 1;
> int retval;
>
> + mutex_lock(&usb_bus_list_lock);
> +
> usb_dev->devnum = devnum;
> usb_dev->bus->devnum_next = devnum + 1;
> memset (&usb_dev->bus->devmap.devicemap, 0,
> @@ -987,8 +989,6 @@ static int register_root_hub(struct usb_
> set_bit (devnum, usb_dev->bus->devmap.devicemap);
> usb_set_device_state(usb_dev, USB_STATE_ADDRESS);
>
> - mutex_lock(&usb_bus_list_lock);
> -
> usb_dev->ep0.desc.wMaxPacketSize = cpu_to_le16(64);
> retval = usb_get_device_descriptor(usb_dev, USB_DT_DEVICE_SIZE);
> if (retval != sizeof usb_dev->descriptor) {
> @@ -1011,6 +1011,7 @@ static int register_root_hub(struct usb_
> if (retval) {
> dev_err (parent_dev, "can't register root hub for %s, %d\n",
> dev_name(&usb_dev->dev), retval);
> + usb_dev->devnum = 0;
> }
> mutex_unlock(&usb_bus_list_lock);
>
> @@ -2527,6 +2528,7 @@ error_create_attr_group:
> #endif
> mutex_lock(&usb_bus_list_lock);
> usb_disconnect(&rhdev); /* Sets rhdev to NULL */
> + hcd->self.root_hub->devnum = 0;
> mutex_unlock(&usb_bus_list_lock);
> err_register_root_hub:
> hcd->rh_pollable = 0;
> @@ -2583,6 +2585,7 @@ void usb_remove_hcd(struct usb_hcd *hcd)
>
> mutex_lock(&usb_bus_list_lock);
> usb_disconnect(&rhdev); /* Sets rhdev to NULL */
> + hcd->self.root_hub->devnum = 0;
> mutex_unlock(&usb_bus_list_lock);
>
> /* Prevent any more root-hub status calls from the timer.
>
--
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
