Date: Tue, 3 Jun 2003 10:34:04 -0700 (PDT) From: Nate Lawson <nate@root.org> To: Sam Leffler <sam@errno.com> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/ubsec ubsec.c ubsecvar.h Message-ID: <20030603101652.V23167@root.org> In-Reply-To: <088401c329de$e82f7a50$52557f42@errno.com> References: <20030602233211.2492D37B4A2@hub.freebsd.org> <20030602234329.T22029@root.org> <088401c329de$e82f7a50$52557f42@errno.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 3 Jun 2003, Sam Leffler wrote:
> > On Mon, 2 Jun 2003, Sam Leffler wrote:
> > > o don't use locking on detach; disabling interrupts is sufficient (I
> think)
> >
> > Unfortunately it's not if you're on a shared interrupt. You need to
> > call bus_teardown_intr() before you are detached from the ithread. See
> > the thread near this message:
> > Message-ID: <1742240000.1051807110@aslan.btc.adaptec.com>
>
> If you're interested look at the change. I understood the device could be
> on a shared irq.
Reviewing the change is what prompted the comment:
####
@@ -500,11 +502,11 @@ ubsec_detach(device_t dev)
{
struct ubsec_softc *sc = device_get_softc(dev);
-KASSERT(sc != NULL, ("ubsec_detach: null software carrier"));
-
/* XXX wait/abort active ops */
-UBSEC_LOCK(sc);
+/* disable interrupts */
+WRITE_REG(sc, BS_CTRL, READ_REG(sc, BS_CTRL) &~
+(BS_CTRL_MCR2INT | BS_CTRL_MCR1INT | BS_CTRL_DMAERR));
callout_stop(&sc->sc_rngto);
####
That disables the device's interrupts but does not unhook the ih. That
happens further down. So if interrupts occur, you are depending on
ubsec_intr()'s check whether it is enabled or not (BS_STAT) to see if the
interrupt is for ubsec. That seems fine to me (after further review).
But if an instance of ubsec_intr() is already active, might there be
problems? I'm thinking of it locking mcr1lock and then detach() calling
mtx_destroy() while it is active or other similar issues. This is
probably why you have "/* XXX wait/abort active ops */" at the top. My
question was whether this should be a function of the underlying ithread
subsystem and not the responsibility of each driver.
-Nate
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030603101652.V23167>