MPSAFE patch for fxp(4)
Nate Lawson
nate at root.org
Thu Apr 10 16:48:47 PDT 2003
I have completed a patch for fxp(4) to enable MPSAFE operation. Please
test it if you have one of these devices. Particularly interesting cases
are DEVICE_POLLING and SMP and non-i386 archs. I have tested it
thoroughly on an i386 UP box both as a module and compiled into the
kernel. The one necessary fix by gallatin@ to vm/uma_core.c is included
as well.
-Nate
Make fxp(4) INTR_MPSAFE:
- Add fxp_start_body() and change fxp_start() to just acquire locks and
then call fxp_start_body(). Places that would call fxp_start() with
locks held (mutex recursion) now call fxp_start_body() directly. Remove
MTX_RECURSE flag from sc_mtx. [gallatin]
- Change fxp_attach() to work without the softc lock, saving interrupt
hooking until the head of fxp_attach().
- Call ether_ifattach() before overriding ifp parameters. This reverts
part of 1.155.
- Remove multiple error paths in fxp_attach().
- Teardown interrupt in fxp_detach() before unlocking the softc.
- Add locking to fxp_suspend, fxp_resume, fxp_start, fxp_intr,
fxp_poll, fxp_tick, fxp_ioctl, fxp_watchdog.
- Pass in ifp to fxp_intr_body since its callers sometimes already use it.
- Add compatibility define for INTR_MPSAFE for 4.x. [gallatin]
Ideas from: gallatin, mux
Tested by: >200M packets of dd/ssh, NFS, ping on i386 UP
-------------- next part --------------
Index: if_fxp.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/fxp/if_fxp.c,v
retrieving revision 1.166
diff -u -r1.166 if_fxp.c
--- if_fxp.c 8 Apr 2003 18:56:45 -0000 1.166
+++ if_fxp.c 10 Apr 2003 23:51:06 -0000
@@ -188,6 +188,7 @@
static void fxp_init(void *xsc);
static void fxp_tick(void *xsc);
static void fxp_powerstate_d0(device_t dev);
+static void fxp_start_body(struct ifnet *ifp);
static void fxp_start(struct ifnet *ifp);
static void fxp_stop(struct fxp_softc *sc);
static void fxp_release(struct fxp_softc *sc);
@@ -375,14 +376,13 @@
int i, rid, m1, m2, prefer_iomap, maxtxseg;
int s;
- bzero(sc, sizeof(*sc));
sc->dev = dev;
callout_handle_init(&sc->stat_ch);
sysctl_ctx_init(&sc->sysctl_ctx);
mtx_init(&sc->sc_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK,
- MTX_DEF | MTX_RECURSE);
+ MTX_DEF);
- s = splimp();
+ s = splimp();
/*
* Enable bus mastering. Enable memory space too, in case
@@ -474,8 +474,10 @@
sc->sysctl_tree = SYSCTL_ADD_NODE(&sc->sysctl_ctx,
SYSCTL_STATIC_CHILDREN(_hw), OID_AUTO,
device_get_nameunit(dev), CTLFLAG_RD, 0, "");
- if (sc->sysctl_tree == NULL)
+ if (sc->sysctl_tree == NULL) {
+ error = ENXIO;
goto fail;
+ }
SYSCTL_ADD_PROC(&sc->sysctl_ctx, SYSCTL_CHILDREN(sc->sysctl_tree),
OID_AUTO, "int_delay", CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_PRISON,
&sc->tunable_int_delay, 0, sysctl_hw_fxp_int_delay, "I",
@@ -610,7 +612,7 @@
error = bus_dmamem_alloc(sc->fxp_stag, (void **)&sc->fxp_stats,
BUS_DMA_NOWAIT, &sc->fxp_smap);
if (error)
- goto failmem;
+ goto fail;
error = bus_dmamap_load(sc->fxp_stag, sc->fxp_smap, sc->fxp_stats,
sizeof(struct fxp_stats), fxp_dma_map_addr, &sc->stats_addr, 0);
if (error) {
@@ -630,7 +632,7 @@
error = bus_dmamem_alloc(sc->cbl_tag, (void **)&sc->fxp_desc.cbl_list,
BUS_DMA_NOWAIT, &sc->cbl_map);
if (error)
- goto failmem;
+ goto fail;
bzero(sc->fxp_desc.cbl_list, FXP_TXCB_SZ);
error = bus_dmamap_load(sc->cbl_tag, sc->cbl_map,
@@ -652,7 +654,7 @@
error = bus_dmamem_alloc(sc->mcs_tag, (void **)&sc->mcsp,
BUS_DMA_NOWAIT, &sc->mcs_map);
if (error)
- goto failmem;
+ goto fail;
error = bus_dmamap_load(sc->mcs_tag, sc->mcs_map, sc->mcsp,
sizeof(struct fxp_cb_mcs), fxp_dma_map_addr, &sc->mcs_addr, 0);
if (error) {
@@ -688,8 +690,10 @@
device_printf(dev, "can't create DMA map for RX\n");
goto fail;
}
- if (fxp_add_rfabuf(sc, rxp) != 0)
- goto failmem;
+ if (fxp_add_rfabuf(sc, rxp) != 0) {
+ error = ENOMEM;
+ goto fail;
+ }
}
/*
@@ -751,7 +755,6 @@
ifp->if_watchdog = fxp_watchdog;
/* Enable checksum offload for 82550 or better chips */
-
if (sc->flags & FXP_FLAG_EXT_RFA) {
ifp->if_hwassist = FXP_CSUM_FEATURES;
ifp->if_capabilities = IFCAP_HWCSUM;
@@ -759,6 +762,11 @@
}
/*
+ * Attach the interface.
+ */
+ ether_ifattach(ifp, sc->arpcom.ac_enaddr);
+
+ /*
* Tell the upper layer(s) we support long frames.
*/
ifp->if_data.ifi_hdrlen = sizeof(struct ether_vlan_header);
@@ -770,32 +778,23 @@
*/
ifp->if_snd.ifq_maxlen = FXP_NTXCB - 1;
- /*
- * Attach the interface.
- */
- ether_ifattach(ifp, sc->arpcom.ac_enaddr);
-
- error = bus_setup_intr(dev, sc->irq, INTR_TYPE_NET,
- fxp_intr, sc, &sc->ih);
+ /* Hook our interrupt after all initialization is complete. */
+ error = bus_setup_intr(dev, sc->irq, INTR_TYPE_NET | INTR_MPSAFE,
+ fxp_intr, sc, &sc->ih);
if (error) {
device_printf(dev, "could not setup irq\n");
goto fail;
}
- splx(s);
- return (0);
-
-failmem:
- device_printf(dev, "Failed to malloc memory\n");
- error = ENOMEM;
fail:
splx(s);
- fxp_release(sc);
+ if (error)
+ fxp_release(sc);
return (error);
}
/*
- * release all resources
+ * Release all resources. The softc lock should not be held.
*/
static void
fxp_release(struct fxp_softc *sc)
@@ -827,9 +826,11 @@
bus_dmamap_destroy(sc->fxp_mtag, txp->tx_map);
}
- bus_generic_detach(sc->dev);
+ mtx_assert(&sc->sc_mtx, MA_NOTOWNED);
if (sc->miibus)
device_delete_child(sc->dev, sc->miibus);
+ if (device_is_alive(sc->dev))
+ bus_generic_detach(sc->dev);
if (sc->fxp_desc.cbl_list) {
bus_dmamap_unload(sc->cbl_tag, sc->cbl_map);
@@ -873,11 +874,12 @@
struct fxp_softc *sc = device_get_softc(dev);
int s;
+ FXP_LOCK(sc);
+ s = splimp();
+
/* disable interrupts */
CSR_WRITE_1(sc, FXP_CSR_SCB_INTRCNTL, FXP_SCB_INTR_DISABLE);
- s = splimp();
-
/*
* Stop DMA and drop transmit queue.
*/
@@ -893,6 +895,11 @@
*/
ifmedia_removeall(&sc->sc_media);
+ /* Unhook interrupt before dropping lock. */
+ bus_teardown_intr(sc->dev, sc->irq, sc->ih);
+ sc->ih = NULL;
+
+ FXP_UNLOCK(sc);
splx(s);
/* Release our allocated resources. */
@@ -929,6 +936,7 @@
struct fxp_softc *sc = device_get_softc(dev);
int i, s;
+ FXP_LOCK(sc);
s = splimp();
fxp_stop(sc);
@@ -942,6 +950,7 @@
sc->suspended = 1;
+ FXP_UNLOCK(sc);
splx(s);
return (0);
}
@@ -959,6 +968,7 @@
u_int16_t pci_command;
int i, s;
+ FXP_LOCK(sc);
s = splimp();
fxp_powerstate_d0(dev);
@@ -985,6 +995,7 @@
sc->suspended = 0;
+ FXP_UNLOCK(sc);
splx(s);
return (0);
}
@@ -1206,12 +1217,27 @@
}
/*
- * Start packet transmission on the interface.
+ * Grab the softc lock and call the real fxp_start_body() routine
*/
static void
fxp_start(struct ifnet *ifp)
{
struct fxp_softc *sc = ifp->if_softc;
+
+ FXP_LOCK(sc);
+ fxp_start_body(ifp);
+ FXP_UNLOCK(sc);
+}
+
+/*
+ * Start packet transmission on the interface.
+ * This routine must be called with the softc lock held, and is an
+ * internal entry point only.
+ */
+static void
+fxp_start_body(struct ifnet *ifp)
+{
+ struct fxp_softc *sc = ifp->if_softc;
struct fxp_tx *txp;
struct mbuf *mb_head;
int error;
@@ -1426,7 +1452,8 @@
}
}
-static void fxp_intr_body(struct fxp_softc *sc, u_int8_t statack, int count);
+static void fxp_intr_body(struct fxp_softc *sc, struct ifnet *ifp,
+ u_int8_t statack, int count);
#ifdef DEVICE_POLLING
static poll_handler_t fxp_poll;
@@ -1437,8 +1464,10 @@
struct fxp_softc *sc = ifp->if_softc;
u_int8_t statack;
+ FXP_LOCK(sc);
if (cmd == POLL_DEREGISTER) { /* final call, enable interrupts */
CSR_WRITE_1(sc, FXP_CSR_SCB_INTRCNTL, 0);
+ FXP_UNLOCK(sc);
return;
}
statack = FXP_SCB_STATACK_CXTNO | FXP_SCB_STATACK_CNA |
@@ -1447,15 +1476,18 @@
u_int8_t tmp;
tmp = CSR_READ_1(sc, FXP_CSR_SCB_STATACK);
- if (tmp == 0xff || tmp == 0)
+ if (tmp == 0xff || tmp == 0) {
+ FXP_UNLOCK(sc);
return; /* nothing to do */
+ }
tmp &= ~statack;
/* ack what we can */
if (tmp != 0)
CSR_WRITE_1(sc, FXP_CSR_SCB_STATACK, tmp);
statack |= tmp;
}
- fxp_intr_body(sc, statack, count);
+ fxp_intr_body(sc, ifp, statack, count);
+ FXP_UNLOCK(sc);
}
#endif /* DEVICE_POLLING */
@@ -1466,22 +1498,26 @@
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
- struct ifnet *ifp = &sc->sc_if;
-
- if (ifp->if_flags & IFF_POLLING)
+ if (ifp->if_flags & IFF_POLLING) {
+ FXP_UNLOCK(sc);
return;
+ }
if (ether_poll_register(fxp_poll, ifp)) {
/* disable interrupts */
CSR_WRITE_1(sc, FXP_CSR_SCB_INTRCNTL, FXP_SCB_INTR_DISABLE);
fxp_poll(ifp, 0, 1);
+ FXP_UNLOCK(sc);
return;
}
#endif
if (sc->suspended) {
+ FXP_UNLOCK(sc);
return;
}
@@ -1492,15 +1528,18 @@
* all bits are set, this may indicate that the card has
* been physically ejected, so ignore it.
*/
- if (statack == 0xff)
+ if (statack == 0xff) {
+ FXP_UNLOCK(sc);
return;
+ }
/*
* First ACK all the interrupts in this pass.
*/
CSR_WRITE_1(sc, FXP_CSR_SCB_STATACK, statack);
- fxp_intr_body(sc, statack, -1);
+ fxp_intr_body(sc, ifp, statack, -1);
}
+ FXP_UNLOCK(sc);
}
static void
@@ -1528,9 +1567,9 @@
}
static void
-fxp_intr_body(struct fxp_softc *sc, u_int8_t statack, int count)
+fxp_intr_body(struct fxp_softc *sc, struct ifnet *ifp, u_int8_t statack,
+ int count)
{
- struct ifnet *ifp = &sc->sc_if;
struct mbuf *m;
struct fxp_rx *rxp;
struct fxp_rfa *rfa;
@@ -1571,7 +1610,7 @@
* Try to start more packets transmitting.
*/
if (ifp->if_snd.ifq_head != NULL)
- fxp_start(ifp);
+ fxp_start_body(ifp);
}
/*
@@ -1695,6 +1734,8 @@
struct fxp_stats *sp = sc->fxp_stats;
int s;
+ FXP_LOCK(sc);
+ s = splimp();
bus_dmamap_sync(sc->fxp_stag, sc->fxp_smap, BUS_DMASYNC_POSTREAD);
ifp->if_opackets += le32toh(sp->tx_good);
ifp->if_collisions += le32toh(sp->tx_total_collisions);
@@ -1721,7 +1762,7 @@
if (tx_threshold < 192)
tx_threshold += 64;
}
- s = splimp();
+
/*
* Release any xmit buffers that have completed DMA. This isn't
* strictly necessary to do here, but it's advantagous for mbufs
@@ -1774,11 +1815,12 @@
}
if (sc->miibus != NULL)
mii_tick(device_get_softc(sc->miibus));
- splx(s);
/*
* Schedule another timeout one second from now.
*/
sc->stat_ch = timeout(fxp_tick, sc, hz);
+ FXP_UNLOCK(sc);
+ splx(s);
}
/*
@@ -1842,10 +1884,12 @@
{
struct fxp_softc *sc = ifp->if_softc;
+ FXP_LOCK(sc);
device_printf(sc->dev, "device timeout\n");
ifp->if_oerrors++;
fxp_init(sc);
+ FXP_UNLOCK(sc);
}
static void
@@ -2107,12 +2151,12 @@
else
#endif /* DEVICE_POLLING */
CSR_WRITE_1(sc, FXP_CSR_SCB_INTRCNTL, 0);
- splx(s);
/*
* Start stats updater.
*/
sc->stat_ch = timeout(fxp_tick, sc, hz);
+ splx(s);
}
static int
@@ -2295,6 +2339,7 @@
struct mii_data *mii;
int s, error = 0;
+ FXP_LOCK(sc);
s = splimp();
switch (command) {
@@ -2351,8 +2396,15 @@
break;
default:
+ /*
+ * ether_ioctl() will eventually call fxp_start() which
+ * will result in mutex recursion so drop it first.
+ */
+ FXP_UNLOCK(sc);
error = ether_ioctl(ifp, command, data);
}
+ if (mtx_owned(&sc->sc_mtx))
+ FXP_UNLOCK(sc);
splx(s);
return (error);
}
Index: if_fxpvar.h
===================================================================
RCS file: /home/ncvs/src/sys/dev/fxp/if_fxpvar.h,v
retrieving revision 1.24
diff -u -r1.24 if_fxpvar.h
--- if_fxpvar.h 2 Apr 2003 16:47:16 -0000 1.24
+++ if_fxpvar.h 9 Apr 2003 16:55:34 -0000
@@ -104,6 +104,7 @@
#if __FreeBSD_version < 500000
#define FXP_LOCK(_sc)
#define FXP_UNLOCK(_sc)
+#define INTR_MPSAFE 0
#define mtx_init(a, b, c, d)
#define mtx_destroy(a)
struct mtx { int dummy; };
Index: /sys/vm/uma_core.c
===================================================================
RCS file: /home/ncvs/src/sys/vm/uma_core.c,v
retrieving revision 1.51
diff -u -r1.51 uma_core.c
--- /sys/vm/uma_core.c 26 Mar 2003 18:44:53 -0000 1.51
+++ /sys/vm/uma_core.c 6 Apr 2003 00:03:38 -0000
@@ -703,10 +703,15 @@
wait &= ~M_ZERO;
if (booted || (zone->uz_flags & UMA_ZFLAG_PRIVALLOC)) {
- mtx_lock(&Giant);
- mem = zone->uz_allocf(zone,
- zone->uz_ppera * UMA_SLAB_SIZE, &flags, wait);
- mtx_unlock(&Giant);
+ if ((wait & M_NOWAIT) == 0) {
+ mtx_lock(&Giant);
+ mem = zone->uz_allocf(zone,
+ zone->uz_ppera * UMA_SLAB_SIZE, &flags, wait);
+ mtx_unlock(&Giant);
+ } else {
+ mem = zone->uz_allocf(zone,
+ zone->uz_ppera * UMA_SLAB_SIZE, &flags, wait);
+ }
if (mem == NULL) {
ZONE_LOCK(zone);
return (NULL);
More information about the freebsd-hackers
mailing list