pccbb crashes when detaching (unsafe interrupt handler)

Devices that attach to a pccbb end up getting screwed if they share
interrupt handlers with anything else and one comes in at such time
that the interrupt handler has been deallocated but not torn down.
There is no interrupt protection for the pccbb interrupt handler,
and the cleanest method I can think of is add a "don't run" count to
the real ithread interrupt handler (parent); however, this is a fair
bit of infrastructure if it's not something very useful, and in SMP,
concurrent accesses of the local interrupt handler list in pccbb do
need to be protected, not just preventing interrupt handlers.

This is what I have so far, but I'm not happy with it because it
seems like it would be so much nicer to increment a count on the
ithread's intrhand and drain any current interrupt out of the
ithread, but possibly making each interrupt a lot more expensive.

Index: pccbb.c
===================================================================
RCS file: /usr/ncvs/src/sys/dev/pccbb/pccbb.c,v
retrieving revision 1.114
diff -u -r1.114 pccbb.c
--- pccbb.c	23 Jun 2004 13:49:46 -0000	1.114
+++ pccbb.c	14 Jul 2004 21:59:49 -0000
_at__at_ -85,6 +85,7 _at__at_
 #include <sys/lock.h>
 #include <sys/malloc.h>
 #include <sys/mutex.h>
+#include <sys/sx.h>
 #include <sys/sysctl.h>
 #include <sys/kthread.h>
 #include <sys/bus.h>
_at__at_ -696,6 +697,7 _at__at_
 	parent = device_get_parent(brdev);
 	mtx_init(&sc->mtx, device_get_nameunit(brdev), "cbb", MTX_DEF);
 	cv_init(&sc->cv, "cbb cv");
+	sx_init(&sc->intrsx, "cbb intr sx");
 	sc->chipset = cbb_chipset(pci_get_devid(brdev), NULL);
 	sc->dev = brdev;
 	sc->cbdev = NULL;
_at__at_ -715,6 +717,7 _at__at_
 		device_printf(brdev, "Could not map register memory\n");
 		mtx_destroy(&sc->mtx);
 		cv_destroy(&sc->cv);
+		sx_destroy(&sc->intrsx);
 		return (ENOMEM);
 	} else {
 		DEVPRINTF((brdev, "Found memory at %08lx\n",
_at__at_ -813,6 +816,7 _at__at_
 	}
 	mtx_destroy(&sc->mtx);
 	cv_destroy(&sc->cv);
+	sx_destroy(&sc->intrsx);
 	return (ENOMEM);
 }
 
_at__at_ -852,6 +856,7 _at__at_
 	    sc->base_res);
 	mtx_destroy(&sc->mtx);
 	cv_destroy(&sc->cv);
+	sx_destroy(&sc->intrsx);
 	return (0);
 }
 
_at__at_ -903,7 +908,9 _at__at_
 	ih->intr = intr;
 	ih->arg = arg;
 	ih->flags = flags & INTR_MPSAFE;
+	sx_xlock(&sc->intrsx);
 	STAILQ_INSERT_TAIL(&sc->intr_handlers, ih, entries);
+	sx_xunlock(&sc->intrsx);
 	cbb_enable_func_intr(sc);
 	/*
 	 * XXX need to turn on ISA interrupts, if we ever support them, but
_at__at_ -921,7 +928,9 _at__at_
 
 	/* XXX Need to do different things for ISA interrupts. */
 	ih = (struct cbb_intrhand *) cookie;
+	sx_xlock(&sc->intrsx);
 	STAILQ_REMOVE(&sc->intr_handlers, ih, cbb_intrhand, entries);
+	sx_xunlock(&sc->intrsx);
 	free(ih, M_DEVBUF);
 	return (0);
 }
_at__at_ -1147,13 +1156,28 _at__at_
 	 * If the card is OK, call all the interrupt handlers.
  	 */
 	if (sc->flags & CBB_CARD_OK) {
+		int needgiant = 0;
+
+		sx_slock(&sc->intrsx);
 		STAILQ_FOREACH(ih, &sc->intr_handlers, entries) {
-			if ((ih->flags & INTR_MPSAFE) == 0)
-				mtx_lock(&Giant);
-			(*ih->intr)(ih->arg);
-			if ((ih->flags & INTR_MPSAFE) == 0)
-				mtx_unlock(&Giant);
+			if ((ih->flags & INTR_MPSAFE) == 0) {
+				needgiant = 1;
+				break;
+			}
+		}
+		if (!needgiant) {
+			STAILQ_FOREACH(ih, &sc->intr_handlers, entries)
+				(*ih->intr)(ih->arg);
+			sx_sunlock(&sc->intrsx);
+			return;
 		}
+		sx_sunlock(&sc->intrsx);
+		mtx_lock(&Giant);
+		sx_slock(&sc->intrsx);
+		STAILQ_FOREACH(ih, &sc->intr_handlers, entries)
+			(*ih->intr)(ih->arg);
+		sx_sunlock(&sc->intrsx);
+		mtx_unlock(&Giant);
 	}
 }
 
Index: pccbbvar.h
===================================================================
RCS file: /usr/ncvs/src/sys/dev/pccbb/pccbbvar.h,v
retrieving revision 1.20
diff -u -r1.20 pccbbvar.h
--- pccbbvar.h	21 May 2004 06:10:13 -0000	1.20
+++ pccbbvar.h	14 Jul 2004 21:58:22 -0000
_at__at_ -65,6 +65,7 _at__at_
 	u_int8_t	subbus;
 	struct mtx	mtx;
 	struct cv	cv;
+	struct sx	intrsx;
 	u_int32_t	flags;
 #define CBB_CARD_OK		0x08000000
 #define	CBB_16BIT_CARD		0x20000000


-- 
Brian Fundakowski Feldman                           \'[ FreeBSD ]''''''''''\
  <> green_at_FreeBSD.org                               \  The Power to Serve! \
 Opinions expressed are my own.                       \,,,,,,,,,,,,,,,,,,,,,,\