Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 29 Apr 2003 14:20:41 -0400 (EDT)
From:      Andrew Gallatin <gallatin@cs.duke.edu>
To:        "M. Warner Losh" <imp@bsdimp.com>
Cc:        cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/dev/fxp if_fxp.c if_fxpvar.h
Message-ID:  <16046.49785.35005.126940@grasshopper.cs.duke.edu>
In-Reply-To: <20030429.115524.21927823.imp@bsdimp.com>
References:  <200304290545.h3T5j99Y076513@repoman.freebsd.org> <20030429133708.A84234@grasshopper.cs.duke.edu> <20030429.115524.21927823.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help 

M. Warner Losh writes:
 > In message: <20030429133708.A84234@grasshopper.cs.duke.edu>
 >             Andrew Gallatin <gallatin@cs.duke.edu> writes:
 > : What is this deadlock with bus_teardown_intr?  Could we possibly fix
 > : this some other way than by adding (mostly) useless code to the
 > : critical path? 
 > 
 > I can remove it if it really annoys you that much.  The locking code
 > itself will swamp the extra load/compare that happens.  It will put


"A million here, a million there, pretty soon it adds up"

I'm just disappointed to see things bloat, and bloat, and bloat more. 
FreeBSD used to be fast..


 > the race back into the code, however.  This likely means that some
 > higher level of locking is necessary so that we can make sure that the
 > interrupts can't happen once detach starts.
 > 
 > Warner


How about just replacing the intr handler with a dummy?  That seems
fairly easy and doesn't require pessimizing critical path code.

Would you mind trying something like the following (sorry, I'm in the
middle of moving and don't have machines handy to test ... most of my
lab is packed).  I don't even have a tree handy to do diffs.

Drew


Add this to kern/kern_intr.c:

void
ithread_orphan_handler(void *arg)
{
}

int
ithread_replace_handler(void *cookie, driver_intr_t func)
{
        struct intrhand *handler = (struct intrhand *)cookie;
        struct ithd *ithread;
#ifdef INVARIANTS
        struct intrhand *ih;
#endif
        ithread = handler->ih_ithread;
        KASSERT(ithread != NULL,
            ("interrupt handler \"%s\" has a NULL interrupt thread",
                handler->ih_name));

        if (handler == NULL)
                return (EINVAL);

	mtx_lock(&ithread->it_lock);
	
	handler->ih_handler = func;
	mtx_unlock(&ithread->it_lock);
	return (0);
}


And make if_fxp.c look like this:

/*
 * Detach interface.
 */
static int
fxp_detach(device_t dev)
{
        struct fxp_softc *sc = device_get_softc(dev);
        int s;

        FXP_LOCK(sc);
        s = splimp();

        sc->gone = 1;
        /*
         * Close down routes etc.
         */
        ether_ifdetach(&sc->arpcom.ac_if);

        /*
         * Stop DMA and drop transmit queue, but disable interruptsfirst.
         */
        CSR_WRITE_1(sc, FXP_CSR_SCB_INTRCNTL, FXP_SCB_INTR_DISABLE);
        fxp_stop(sc);
	if(ithread_replace_handler(sc->ih, ithread_orphan_handler))
		panic("ithread_replace_handler didn't");			  
        FXP_UNLOCK(sc);

        /*
         * Unhook interrupt before dropping lock. This is to prevent
         * races with fxp_intr().
         */
        bus_teardown_intr(sc->dev, sc->irq, sc->ih);
        sc->ih = NULL;

        splx(s);

        /* Release our allocated resources. */
        fxp_release(sc);
        return (0);
}

static void
fxp_intr(void *xsc)
{
        struct fxp_softc *sc = xsc;
        struct ifnet *ifp = &sc->sc_if;
        u_int8_t statack;

        FXP_LOCK(sc);
#ifdef DEVICE_POLLING
        if (ifp->if_flags & IFF_POLLING) {
<..>

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?16046.49785.35005.126940>