On Tue, Feb 18, 2003 at 08:59:15AM -0800, David Brownell wrote:
> Greg KH wrote:
> >
> >Yes, this is a problem, the module can be unloaded and then the
> >completion fuction can be called.
>
> How? I don't see it. Unless the driver completely violates
> basic rules like implementing disconnect() correctly, which
> will break more than just module unloading.
Hm, let me think about this some more before responding...
> > I'm trying to fix up the usb device reference counting
> >logic right now and running into issues where host controllers try to
> >access urbs after their devices have been removed from the system :(
>
> Not supposed to happen ... when they call giveback(), that's supposed to
> mean they've forgotten about that URB. Is it buggy disconnect() logic,
> or some HCD calling giveback too early?
I'm seeing uhci_irq() get called for an urb that belongs to a usb device
that has already been disconnected. Things go wrong when
uhci_transfer_result() is called, which calls usb_release_bandwidth()
which expects dev->bus to be a valid pointer. I thought that after
deallocate() is called on the bus for the device, everything would be
cleaned up.
Here's the patch to the usb core that shows this quite nicely:
===== usb.c 1.190 vs edited =====
--- 1.190/drivers/usb/core/usb.c Sun Feb 16 05:48:02 2003
+++ edited/usb.c Sun Feb 16 09:03:33 2003
@@ -750,10 +750,7 @@
if (!udev)
return;
- if (udev->bus && udev->bus->op && udev->bus->op->deallocate)
- udev->bus->op->deallocate(udev);
usb_destroy_configuration (udev);
- usb_bus_put (udev->bus);
kfree (udev);
}
@@ -820,6 +817,7 @@
void usb_disconnect(struct usb_device **pdev)
{
struct usb_device * dev = *pdev;
+ struct usb_bus *bus;
int i;
if (!dev)
@@ -853,6 +851,17 @@
usbfs_remove_device(dev);
}
device_unregister(&dev->dev);
+
+ /* Detach the bus from the device right now, as it's no longer a
+ * valid link. This helps for drivers that keep a reference to
+ * the device around for a while if userspace holds a device
+ * open. It will cause all further urb submissions to fail,
+ * which is what we want to have happen. */
+ bus = dev->bus;
+ if (bus && bus->op && bus->op->deallocate)
+ bus->op->deallocate(dev);
+ dev->bus = NULL;
+ usb_bus_put (bus);
/* Decrement the reference count, it'll auto free everything when */
/* it hits 0 which could very well be now */
I have a patch to the usb serial code that tests this logic out by
holding a reference to the usb device structure while a port is opened
by a user. That way, if the device disappears, then all writes and
reads get a -ENODEV, and when the last consumer leaves the driver, the
device is finally deleted from memory properly. This seems to solve the
nasty race conditions that people have been having with the usb-serial
drivers lately.
Time to test this out on ohci to see how well it works...
thanks,
greg k-h
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
