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

M. Warner Losh imp at bsdimp.com
Sat May 3 10:44:39 PDT 2003


In message: <XFMail.20030501140515.jhb at FreeBSD.org>
            John Baldwin <jhb at FreeBSD.org> writes:
: 
: On 01-May-2003 M. Warner Losh wrote:
: > In message: <1721460000.1051803729 at aslan.btc.adaptec.com>
: >             "Justin T. Gibbs" <gibbs at scsiguy.com> writes:
: >: >> 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.
: >: 
: >: All doable things for all but really broken hardware.  fxp is not broken.
: > 
: > The whole reason for the gone flag may be misunderstood here.  You can
: > easily turn off the fxp device, and there will be no more interrupts
: > from it.  However, its ISR can and will still be called from time to
: > time until the bus_teardown_intr() is complete?  Why you ask?  Because
: > of shared interrupts.  If fxp shares an interrupt with another device,
: > your ISR will execute even if you write 0 into the interrupt enable
: > register if that other device gets an interrupt between the time you
: > write to this register and the time bus_teardown_intr is called, even
: > on a single CPU machine:
: > 
: > 
: >       fxp_detach()
: > [4]   LOCK
: > [a]   write 0 to dis intr
: > [5]                           device B on same intr interrupts here
: >                               fxp_intr()
: >                               LOCK (->sleep)
: > [b]   gone = 0;
: >       UNLOCK
: > [1]                           if (gone) return;
: > [2]   bus_teardown_intr();
: > [3]   bus_teardown_intr returns
: > 
: > 
: > [1] and [2] can happen in any order, but you know both of them have
: > happened by [3].
: > 
: > The order of [a] and [b] don't really matter because fxp (or anything
: > that shares its interrupt) could generate an interrupt after the lock
: > is taken out at [4] and you'd still have a fxp_intr sleeping thread.
: > The important thing is that an interrupt[5] happens after [4].  This
: > can happen on both the single CPU case and the SMP case.
: > 
: > This might argue for blocking interrupts during a device detach.  I
: > think there might be problems with that apprach as well, although I'd
: > have to think about it a bit to be sure.
: 
: Ok, here's a question.  Why is it bad for fxp_intr() to finish while
: you are blocked in bus_teardown_intr()?  Put another way, perhaps
: fxp_detach() should do the teardown_intr() a lot sooner and postpone
: some of it's cleanups until after that retuns.  I.e.
: 
:         FXP_LOCK()
:         disable_interrupts_in_hardware()
:         FXP_UNLOCK()
:         bus_teardown_intr()
:         FXP_LOCK()
:         do_rest_of_detach()

This means you don't have to call the 'is my hardware there' routine
in the isr.  That's what gone buys you.  I'm not sure that's
necessarily a big thing...  This does imply that we may want two ISRs,
one for hot-swappable hardware and one for not, at least for the high
performance drivers.  Lower performance ones can elect to not care
about the slight extra overhead of checking.

Warner


More information about the cvs-src mailing list