cvs commit: src/sys/dev/fxp if_fxp.c if_fxpvar.h

John Baldwin jhb at FreeBSD.org
Thu May 1 07:37:25 PDT 2003


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 at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/


More information about the cvs-src mailing list