Date: Thu, 01 May 2003 10:37:25 -0400 (EDT) From: John Baldwin <jhb@FreeBSD.org> To: "Justin T. Gibbs" <gibbs@scsiguy.com> Cc: Andrew Gallatin <gallatin@cs.duke.edu> Subject: Re: cvs commit: src/sys/dev/fxp if_fxp.c if_fxpvar.h Message-ID: <XFMail.20030501101142.jhb@FreeBSD.org> In-Reply-To: <1417270000.1051735953@aslan.btc.adaptec.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 30-Apr-2003 Justin T. Gibbs wrote: >> First off, it's a gross API violation. ithread's are supposed to be >> transparent (except that you can use locks in your interrupt handlers). >> Secondly, you still have the same race as if you just removed the gone >> condition. We don't hold the ithread lock while executing handlers, >> so there is nothing preventing the handler from being executed on another >> CPU concurrently with your detach function. In fact, it could easily >> be blocked on the FXP lock. You do your magic pointer foo, then unlock >> the fxp. The unlock releases the interrupt handler on another CPU >> which happily executes _after_ the completion of bus_teardown_intr() >> which has all sorts of bad failure modes, like panicing when it tries >> to FXP_UNLOCK at the end because back on the first CPU you've done a >> mtx_destroy(). > > I could have sworn I talked to you about this a *long* time ago and > requested that bus_teardown_intr() blocked until you were guaranteed > to be out of the interrupt handler. The aic7xxx and aic79xx drivers > depend on this behavior: Yes, bus_teardown_intr() does block and people were complaining that it does. The only problem that needs to be worked around (which you probably already do) is to make sure that if the handler is currently blocked on the lock when you do the teardown, it will not hose anything (like turn interrupts back on) when it executes. For fxp(4), Warner used a 'gone' flag to make it so that fxp_intr() returned immediately in that case. > This means that all detaches must occur from a context that can > sleep, but that shouldn't be too hard to make happen. People can't hold the driver lock across bus_teardown_intr() with this model, which does require a possibly smarter interrupt routine or maybe a better detach that only disables interrupts then does a teardown, then finishes shutting down the rest of the hardware along with an interrupt handler that doesn't re-enable interrupts in the shutdown case. -- John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20030501101142.jhb>