[linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-13 Thread Roman Kagan
Hi, With 2.6.11 and 2.6.12-rc2 (and perhaps a few versions before) usb drivers for multi-interface devices, which do usb_driver_release_interface() in their disconnect(), make rmmod hang. It turns out to be due to a bug in drivers/base/bus.c:driver_detach(), that iterates over the list of attac

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-13 Thread Pete Zaitcev
On Wed, 13 Apr 2005 21:40:17 +0400 Roman Kagan <[EMAIL PROTECTED]> wrote: > +++ linux-2.6.12-rc2/drivers/base/bus.c 2005-04-13 08:45:48.0 > +0400 > @@ -405,9 +405,8 @@ void device_release_driver(struct device > > static void driver_detach(struct device_driver * drv) > { > -

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-13 Thread Duncan Sands
Hi Pete, > The suspect code is also easy to spot: a) uses list_for_each_safe, > b) uses container_of instead of list_entry. That's whole two "newbie > points". what's wrong with list_for_each_safe? In general I mean, not here. Ciao, Duncan. --

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-13 Thread Pete Zaitcev
On Wed, 13 Apr 2005 20:50:17 +0200 Duncan Sands <[EMAIL PROTECTED]> wrote: > > The suspect code is also easy to spot: a) uses list_for_each_safe, > > b) uses container_of instead of list_entry. That's whole two "newbie > > points". > > what's wrong with list_for_each_safe? In general I mean, not

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-14 Thread Roman Kagan
On Wed, Apr 13, 2005 at 11:11:54AM -0700, Pete Zaitcev wrote: > The suspect code is also easy to spot: a) uses list_for_each_safe, > b) uses container_of instead of list_entry. That's whole two "newbie > points". OK to bring the newbie score of drivers/base/bus.c down by another two points, here's

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-24 Thread Roman Kagan
On Wed, Apr 13, 2005 at 09:40:17PM +0400, Roman Kagan wrote: > With 2.6.11 and 2.6.12-rc2 (and perhaps a few versions before) usb > drivers for multi-interface devices, which do > usb_driver_release_interface() in their disconnect(), make rmmod hang. > > It turns out to be due to a bug in drivers/

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-25 Thread Alan Stern
On Mon, 25 Apr 2005, Roman Kagan wrote: > Ping? > > This patch fixes a real and easy to trigger problem, so IMHO it may be > worth applying before 2.6.12-final is out... You might also consider submitting this for 2.6.11.8, or whatever the next bugfix version is. > Still trying to get in touch

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-25 Thread Roman Kagan
On Mon, Apr 25, 2005 at 10:52:20AM -0400, Alan Stern wrote: > On Mon, 25 Apr 2005, Roman Kagan wrote: > > This patch fixes a real and easy to trigger problem, so IMHO it may be > > worth applying before 2.6.12-final is out... > > You might also consider submitting this for 2.6.11.8, or whatever th

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-25 Thread Alan Stern
On Mon, 25 Apr 2005, Roman Kagan wrote: > > > Still trying to get in touch with Pat Mochel on that and especially on > > > the more convoluted klist case, > > > > I've spent a fair amount of time going over the klist code too, and while > > there may still be a problem or two this doesn't appear

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-25 Thread Roman Kagan
On Mon, Apr 25, 2005 at 02:39:55PM -0400, Alan Stern wrote: > On Mon, 25 Apr 2005, Roman Kagan wrote: > > Well for non-klist version the test !list_empty(&dev->driver_list) in > > usb_driver_release_interface() was a good enough guard against recursion > > into device_release_driver(), so I'm not s

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-26 Thread Roman Kagan
On Mon, Apr 25, 2005 at 04:01:07PM -0400, Alan Stern wrote: > I think usb_driver_claim_interface is correct as it stands. It was a > mistake to leave out from usb_driver_release_interface originally the line > setting iface->condition to USB_INTERFACE_UNBINDING. But it would be asymmetric then.

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-26 Thread Alan Stern
On Mon, 25 Apr 2005, Roman Kagan wrote: > > But it is needed for the klist version. I didn't send a patch for that > > version because I don't have the -mm kernel source handy. The analogous > > change to the code should work, however. > > OK then a matching change to usb_driver_claim_interface

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-26 Thread Roman Kagan
On Mon, Apr 25, 2005 at 12:33:22PM -0400, Alan Stern wrote: > On Mon, 25 Apr 2005, Roman Kagan wrote: > > http://marc.theaimsgroup.com/?l=linux-usb-devel&m=111341625900343 > > I see. This may not be a problem so much with klists as with usbcore. > That is, device_release_driver isn't protected

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-26 Thread Alan Stern
On Mon, 25 Apr 2005, Roman Kagan wrote: > > I see. This may not be a problem so much with klists as with usbcore. > > That is, device_release_driver isn't protected against recursion, but > > perhaps it doesn't need to be since usbcore can be changed to prevent > > recursion in usb_driver_relea

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-26 Thread Alan Stern
On Tue, 26 Apr 2005, Roman Kagan wrote: > On Mon, Apr 25, 2005 at 04:01:07PM -0400, Alan Stern wrote: > > I think usb_driver_claim_interface is correct as it stands. It was a > > mistake to leave out from usb_driver_release_interface originally the line > > setting iface->condition to USB_INTER

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-28 Thread Alan Stern
Roman, Pat: Here is a fix for driver_detach(). It's a little ugly because it avoids using the klist iterator, but there's no way around it -- the iterator simply can't be made to work here. Not only does it prevent device_release_driver() from calling klist_remove(), but also it prevents doing g

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-29 Thread Roman Kagan
On Tue, Apr 26, 2005 at 11:57:34AM -0400, Alan Stern wrote: > On Tue, 26 Apr 2005, Roman Kagan wrote: > > > On Mon, Apr 25, 2005 at 04:01:07PM -0400, Alan Stern wrote: > > > I think usb_driver_claim_interface is correct as it stands. It was a > > > mistake to leave out from usb_driver_release_in

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-29 Thread Roman Kagan
On Thu, Apr 28, 2005 at 02:36:39PM -0400, Alan Stern wrote: > Here is a fix for driver_detach(). It's a little ugly because it avoids > using the klist iterator, but there's no way around it -- the iterator > simply can't be made to work here. Not only does it prevent > device_release_driver() fr

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-04-29 Thread Alan Stern
On Fri, 29 Apr 2005, Roman Kagan wrote: > On Tue, Apr 26, 2005 at 11:57:34AM -0400, Alan Stern wrote: > > On Tue, 26 Apr 2005, Roman Kagan wrote: > > > > > On Mon, Apr 25, 2005 at 04:01:07PM -0400, Alan Stern wrote: > > > > I think usb_driver_claim_interface is correct as it stands. It was a >

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-05-03 Thread Roman Kagan
On Fri, Apr 29, 2005 at 02:34:59PM -0400, Alan Stern wrote: > Yes, we do. Okay, are you still concerned about the new code being > asymmetric somehow? No :) > Does the patch look good? If you like it, I'll submit it to Greg. Yes it looks reasonable. At the moment I can't test the problematic

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-05-03 Thread Greg KH
On Wed, May 04, 2005 at 10:30:57AM +0400, Roman Kagan wrote: > On Fri, Apr 29, 2005 at 02:34:59PM -0400, Alan Stern wrote: > > Yes, we do. Okay, are you still concerned about the new code being > > asymmetric somehow? > > No :) > > > Does the patch look good? If you like it, I'll submit it to G

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-05-03 Thread Greg KH
On Thu, Apr 28, 2005 at 02:36:39PM -0400, Alan Stern wrote: > Roman, Pat: > > Here is a fix for driver_detach(). It's a little ugly because it avoids > using the klist iterator, but there's no way around it -- the iterator > simply can't be made to work here. Not only does it prevent > device_re

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-05-04 Thread Roman Kagan
On Wed, May 04, 2005 at 10:30:57AM +0400, Roman Kagan wrote: > On Fri, Apr 29, 2005 at 02:34:59PM -0400, Alan Stern wrote: > > Does the patch look good? If you like it, I'll submit it to Greg. > > Yes it looks reasonable. At the moment I can't test the problematic > multi-interface USB device ca

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-05-04 Thread Alan Stern
On Wed, 4 May 2005, Roman Kagan wrote: > > Does the patch look good? If you like it, I'll submit it to Greg. > > Yes it looks reasonable. At the moment I can't test the problematic > multi-interface USB device case where that bug revealed itself, which > was observed by Duncan Sands; I'm attach

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-05-04 Thread Greg KH
On Wed, May 04, 2005 at 11:19:15AM -0400, Alan Stern wrote: > Here's an idea I had recently to improve the klist library; tell me what > you think. As a space optimization, instead of storing a struct > completion in every klist_node, just put a wait_queue_header in struct > klist. Under normal

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-05-04 Thread Roman Kagan
On Wed, May 04, 2005 at 11:19:15AM -0400, Alan Stern wrote: > Here's an idea I had recently to improve the klist library; tell me what > you think. As a space optimization, instead of storing a struct > completion in every klist_node, just put a wait_queue_header in struct > klist. Under normal

Re: [linux-usb-devel] [PATCH 2.6.12-rc2] drivers/base/bus.c: fix iteration in driver_detach()

2005-05-04 Thread Greg KH
On Wed, May 04, 2005 at 11:32:10PM +0400, Roman Kagan wrote: > On Wed, May 04, 2005 at 11:19:15AM -0400, Alan Stern wrote: > > Here's an idea I had recently to improve the klist library; tell me what > > you think. As a space optimization, instead of storing a struct > > completion in every klis