Date: Mon, 14 Sep 2009 08:52:48 -0400 From: John Baldwin <jhb@freebsd.org> To: Hans Petter Selasky <hselasky@c2i.net> Cc: Attilio Rao <attilio@freebsd.org>, arch@freebsd.org, freebsd-arch@freebsd.org Subject: Re: NEWBUS states (was Re: svn commit: r196779 - in head/sys: kern sys) Message-ID: <200909140852.49192.jhb@freebsd.org> In-Reply-To: <200909121009.22931.hselasky@c2i.net> References: <200909031340.n83Defkv034013@svn.freebsd.org> <200909080936.37603.jhb@freebsd.org> <200909121009.22931.hselasky@c2i.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Saturday 12 September 2009 4:09:21 am Hans Petter Selasky wrote: > On Tuesday 08 September 2009 15:36:37 John Baldwin wrote: > > On Friday 04 September 2009 6:46:03 pm Attilio Rao wrote: > > > We all agreed the one-state was the better option but it can't be done > > > in this way because of the device_is_attached() used in the detach > > > virtual functions. Using just one transition state will break > > > device_is_attached() in those parts. > > > The right fix, as pointed out in other e-mails, is to not use > > > device_is_attached() in detach virtual functions. The better fix, in > > > my idea would involve: > > > - replace the device_is_attached() usage in detach virtual functions, > > > with a more functional support > > > - use one-state transition > > > > > > But that is just too much job to push in before then 8.0-REL and if > > > that would mean to not commit a patch and make impossible a future > > > MFC, I prefer to go with a lesser-perfect-but-still-working-approach. > > > > Wait, all you need to MFC is the change to the enum. Fixing the various > > detach routines does _not_ have to be in 8.0. That could be merged after > > the release. > > Hi, > > http://svn.freebsd.org/viewvc/base/head/sys/kern/subr_bus.c?r1=196529&r2=196779 > > I'm sorry to say that the latest patches to subr_bus.c have broken USB. I've > got several reports on memory used after free, due to bus_generic_detach() > returning EBUSY when called from uhub_detach(). > > ... > bus_generic_detach(device_t dev) > { > device_t child; > int error; > > if (dev->state != DS_ATTACHED) > return (EBUSY); > > TAILQ_FOREACH(child, &dev->children, link) { > if ((error = device_detach(child)) != 0) > return (error); > } > > return (0); > } > > A fix for USB is available here: > > http://perforce.freebsd.org/chv.cgi?CH=168387 I think bus_generic_detach() needs to work when called from a foo_detach() routine, so I think you don't need the USB changes. Even if the new states get reintroduced bus_generic_detach() will still have to work correctly the way you had used it. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200909140852.49192.jhb>