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

[Justin T. Gibbs]

> 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:

	ahc_lock(ahc);
	/*
	 * Disable hardware interrupts so we don't lock the
	 * machine once our handler is removed.
	 */
	ahc_enable_intr(ahc, FALSE);
	ahc_unlock(ahc);
	/*
	 * Called without our lock so that if we are in our interrupt
	 * handler the handler can complete.
	 */
	bus_teardown_intr();
	/* continue with detach. */

This means that all detaches must occur from a context that can
sleep, but that shouldn't be too hard to make happen.

--
Justin