On Fri, Sep 19, 2003 at 06:35:19PM -0400, Alan Stern wrote: > On Fri, 19 Sep 2003, Greg KH wrote: > > > On Fri, Sep 19, 2003 at 05:17:22PM -0400, Alan Stern wrote: > > > > > > ===== config.c 1.25 vs edited ===== > > > --- 1.25/drivers/usb/core/config.c Mon Sep 15 10:14:46 2003 > > > +++ edited/drivers/usb/core/config.c Fri Sep 19 17:07:29 2003 > > > @@ -237,9 +237,6 @@ > > > memset(interface, 0, sizeof(struct usb_interface)); > > > interface->dev.release = usb_release_intf; > > > device_initialize(&interface->dev); > > > - > > > - /* put happens in usb_destroy_configuration */ > > > - get_device(&interface->dev); > > > } > > > > > > /* Go through the descriptors, checking their length and counting the > > > > > > > No, taking this out is wrong. Read the comment :) > > > > We need to increment the count by one, as we want the interfaces to > > stick around until the whole device is cleaned up. That happens from a > > call to usb_release_dev when the final kobject put is called on the > > struct usb_device. Then that calls usb_destroy_configuration() which > > cleans up the raw descriptors of the interfaces that were owned by the > > struct usb_device, and then calls the last put on the struct > > usb_interface, which then will finally free that memory. > > > > If this patch goes in, the interface memory will be freed when the > > interface is unregistered from the driver core (with the call to > > device_unregister(&interface->dev); in usb_disconnect) which is way too > > early in the cleanup sequence. > > > > Did you test that patch? What happened when you unplugged a device? > > I did test it, after applying David's usb_set_configuration rework patch. > It worked exactly right. Without my patch, the interface and device were > never released. > > Note that my patch was meant to apply on top of David's. He changed the > call to device_unregister(&interface->dev) you mentioned to device_del(). > That does make a difference.
Ah, ok, that changes things. I was thinking the first patch was on top of the tree today, without david's changes. Sorry. > Here's the situation with David's patch applied but not mine. The count > starts out at 1. Even if it's not incremented, the interface will stick > around until the whole device is cleaned up. At that time the sequence of > events you described takes place. The device_del() call doesn't change > the count. The last put of the struct usb_interface() decrements the > count to 0 and so releases the memory. But if the get_device() is left > in, the last put will decrement the count to 1 and the memory will never > be freed. > > To put it another, the original code has (when the interface is created): > device_initialize() > get_device() > when the config is set: > device_add() > when the device is disconnected: > device_del() > and when the configuration is destroyed: > put_device() > At the end the reference count is 1, not 0. Yes, with his patch, things are messed up, that's what I was trying to say. With your patch, things are a bit better :) But I still don't think we should be calling device_del() on the main usb device, as David's patch did. If we don't do that, one of your patches is no longer necessary. But yes, your first patch is needed for David's to work properly. David, care to make those changes, test it, and resubmit it to me? 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
