Date: Fri, 11 Apr 2003 02:13:16 +0200 From: Maxime Henrion <mux@freebsd.org> To: Nate Lawson <nate@root.org> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/fxp if_fxp.c Message-ID: <20030411001316.GN1750@elvis.mu.org> In-Reply-To: <Pine.BSF.4.21.0304101655330.32612-100000@root.org> References: <20030410231519.3348737B4A5@hub.freebsd.org> <Pine.BSF.4.21.0304101655330.32612-100000@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Nate Lawson wrote: > On Thu, 10 Apr 2003, Maxime Henrion wrote: > > Modified files: > > sys/dev/fxp if_fxp.c > > Log: > > - Clean up the fxp_release() and fxp_detach() functions. > > There's a version of this in the diff I just posted to current@. Feel free to commit it. > > - Be sure to teardown the interrupt first so that "kldunload if_fxp" > > doesn't panic the box. It's now deadlocking rather than crashing, > > which isn't really better, but I'm unsure this is fxp(4)'s fault. > > There's also a version of this in my diff. Same. > I have been testing my diff by loading and unloading fxp while doing a > large transfer and I cannot replicate this. Are you sure it's not a local > problem? I have never had a deadlock or a crash and loading fxp again > always works. I have no idea, this is why I was saying it's maybe not fxp(4)'s fault and it may indeed be a local problem. > > @@ -878,20 +874,23 @@ > > > > s = splimp(); > > > > - /* > > - * Stop DMA and drop transmit queue. > > - */ > > - fxp_stop(sc); > > - > > - /* > > - * Close down routes etc. > > - */ > > - ether_ifdetach(&sc->arpcom.ac_if); > > - > > - /* > > - * Free all media structures. > > - */ > > - ifmedia_removeall(&sc->sc_media); > > + if (device_is_alive(dev)) { > > + /* > > + * Stop DMA and drop transmit queue. > > + */ > > + if (bus_child_present(dev)) > > + fxp_stop(sc); > > + /* > > + * Close down routes etc. > > + */ > > + ether_ifdetach(&sc->arpcom.ac_if); > > + device_delete_child(dev, sc->miibus); > > + bus_generic_detach(dev); > > + /* > > + * Free all media structures. > > + */ > > + ifmedia_removeall(&sc->sc_media); > > + } > > > > splx(s); > > Um, fxp_detach() should not be called for any case where the device isn't > alive. fxp_detach should ONLY be called once attach has succeeded which > by definition means the device is alive. bus_child_present() is the > bus-specific method to see that the hardware is actually there; > device_is_alive only tells you that the device_t node is present in the > tree. fxp_release may be called in error cases. Rather than working > around your problem this way, please find what is calling fxp_detach when > the device is not alive. This way of doing things (ie: using device_is_alive()) is an almost verbatim copy of what you did in every network driver in sys/pci/. Now if it's wrong, feel free to change it, but I guess you'll have to change all those drivers too then. Cheers, Maxime
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030411001316.GN1750>