diff options
author | jhb <jhb@FreeBSD.org> | 2008-08-04 19:19:18 +0000 |
---|---|---|
committer | jhb <jhb@FreeBSD.org> | 2008-08-04 19:19:18 +0000 |
commit | 82bee699d17057265199df7a5bc137655bdfae9e (patch) | |
tree | 3d1e8eab296c92d2e69ccb8f79d304033f98dd3a /sys/dev | |
parent | 448cbffe9ead78f0eef9320b85edc254321106d5 (diff) | |
download | FreeBSD-src-82bee699d17057265199df7a5bc137655bdfae9e.zip FreeBSD-src-82bee699d17057265199df7a5bc137655bdfae9e.tar.gz |
Add locking to snc(4) so it is MPSAFE:
- Add a mutex to the softc to protect the softc and device hardware.
- Use a private timer routine to drive the transmit watchdog timer instead
of using if_watchdog/if_timer.
- If if_alloc() fails during attach, fail the attach with an error rather
than panic'ing.
- Clear RUNNING and OACTIVE only in sncstop().
- Don't mess with IFF_UP.
- Don't leak 'struct ifnet' on detach.
- Setup interrupt handler after ether_ifattach().
- Call ether_ifdetach() rather than if_detach() in the pccard detach
routine.
Tested by: no one despite repeated requests
Diffstat (limited to 'sys/dev')
-rw-r--r-- | sys/dev/snc/dp83932.c | 137 | ||||
-rw-r--r-- | sys/dev/snc/dp83932var.h | 9 | ||||
-rw-r--r-- | sys/dev/snc/if_snc.c | 31 | ||||
-rw-r--r-- | sys/dev/snc/if_snc_cbus.c | 11 | ||||
-rw-r--r-- | sys/dev/snc/if_snc_pccard.c | 18 |
5 files changed, 130 insertions, 76 deletions
diff --git a/sys/dev/snc/dp83932.c b/sys/dev/snc/dp83932.c index 669c293..8108a33 100644 --- a/sys/dev/snc/dp83932.c +++ b/sys/dev/snc/dp83932.c @@ -63,6 +63,7 @@ #include "opt_inet.h" #include <sys/param.h> +#include <sys/kernel.h> #include <sys/systm.h> #include <sys/sockio.h> #include <sys/mbuf.h> @@ -85,11 +86,13 @@ #include <dev/snc/dp83932reg.h> #include <dev/snc/dp83932var.h> -static void sncwatchdog(struct ifnet *); +static void sncwatchdog(void *); static void sncinit(void *); +static void sncinit_locked(struct snc_softc *); static int sncstop(struct snc_softc *sc); static int sncioctl(struct ifnet *ifp, u_long cmd, caddr_t data); static void sncstart(struct ifnet *ifp); +static void sncstart_locked(struct ifnet *ifp); static void sncreset(struct snc_softc *sc); static void caminitialise(struct snc_softc *); @@ -138,7 +141,7 @@ void snc_mediastatus(struct ifnet *, struct ifmediareq *); int sncdebug = 0; -void +int sncconfig(sc, media, nmedia, defmedia, myea) struct snc_softc *sc; int *media, nmedia, defmedia; @@ -154,9 +157,10 @@ sncconfig(sc, media, nmedia, defmedia, myea) #endif ifp = sc->sc_ifp = if_alloc(IFT_ETHER); - if (ifp == NULL) - panic("%s: can not if_alloc()\n", - device_get_nameunit(sc->sc_dev)); + if (ifp == NULL) { + device_printf(sc->sc_dev, "can not if_alloc()\n"); + return (ENOMEM); + } #ifdef SNCDEBUG device_printf(sc->sc_dev, @@ -170,12 +174,10 @@ sncconfig(sc, media, nmedia, defmedia, myea) device_get_unit(sc->sc_dev)); ifp->if_ioctl = sncioctl; ifp->if_start = sncstart; - ifp->if_flags = - IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST | IFF_NEEDSGIANT; - ifp->if_watchdog = sncwatchdog; + ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST; ifp->if_init = sncinit; ifp->if_mtu = ETHERMTU; - ifp->if_snd.ifq_maxlen = IFQ_MAXLEN; + IFQ_SET_MAXLEN(&ifp->if_snd, IFQ_MAXLEN); /* Initialize media goo. */ ifmedia_init(&sc->sc_media, 0, snc_mediachange, @@ -190,14 +192,17 @@ sncconfig(sc, media, nmedia, defmedia, myea) } ether_ifattach(ifp, myea); + return (0); } void sncshutdown(arg) void *arg; { + struct snc_softc *sc = arg; - sncstop((struct snc_softc *)arg); + SNC_ASSERT_LOCKED(sc); + sncstop(sc); } /* @@ -208,10 +213,15 @@ snc_mediachange(ifp) struct ifnet *ifp; { struct snc_softc *sc = ifp->if_softc; + int error; + SNC_LOCK(sc); if (sc->sc_mediachange) - return ((*sc->sc_mediachange)(sc)); - return (EINVAL); + error = (*sc->sc_mediachange)(sc); + else + error = EINVAL; + SNC_UNLOCK(sc); + return (error); } /* @@ -224,14 +234,17 @@ snc_mediastatus(ifp, ifmr) { struct snc_softc *sc = ifp->if_softc; + SNC_LOCK(sc); if (sc->sc_enabled == 0) { ifmr->ifm_active = IFM_ETHER | IFM_NONE; ifmr->ifm_status = 0; + SNC_UNLOCK(sc); return; } if (sc->sc_mediastatus) (*sc->sc_mediastatus)(sc, ifmr); + SNC_UNLOCK(sc); } @@ -243,12 +256,12 @@ sncioctl(ifp, cmd, data) { struct ifreq *ifr; struct snc_softc *sc = ifp->if_softc; - int s = splimp(), err = 0; - int temp; + int err = 0; switch (cmd) { case SIOCSIFFLAGS: + SNC_LOCK(sc); if ((ifp->if_flags & IFF_UP) == 0 && (ifp->if_drv_flags & IFF_DRV_RUNNING) != 0) { /* @@ -256,7 +269,6 @@ sncioctl(ifp, cmd, data) * then stop it. */ sncstop(sc); - ifp->if_drv_flags &= ~IFF_DRV_RUNNING; snc_disable(sc); } else if ((ifp->if_flags & IFF_UP) != 0 && (ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) { @@ -266,28 +278,28 @@ sncioctl(ifp, cmd, data) */ if ((err = snc_enable(sc)) != 0) break; - sncinit(sc); + sncinit_locked(sc); } else if (sc->sc_enabled) { /* * reset the interface to pick up any other changes * in flags */ - temp = ifp->if_flags & IFF_UP; sncreset(sc); - ifp->if_flags |= temp; - sncstart(ifp); + sncstart_locked(ifp); } + SNC_UNLOCK(sc); break; case SIOCADDMULTI: case SIOCDELMULTI: + SNC_LOCK(sc); if (sc->sc_enabled == 0) { err = EIO; + SNC_UNLOCK(sc); break; } - temp = ifp->if_flags & IFF_UP; sncreset(sc); - ifp->if_flags |= temp; + SNC_UNLOCK(sc); err = 0; break; case SIOCGIFMEDIA: @@ -299,7 +311,6 @@ sncioctl(ifp, cmd, data) err = ether_ioctl(ifp, cmd, data); break; } - splx(s); return (err); } @@ -311,6 +322,17 @@ sncstart(ifp) struct ifnet *ifp; { struct snc_softc *sc = ifp->if_softc; + + SNC_LOCK(sc); + sncstart_locked(ifp); + SNC_UNLOCK(sc); +} + +static void +sncstart_locked(ifp) + struct ifnet *ifp; +{ + struct snc_softc *sc = ifp->if_softc; struct mbuf *m; int mtd_next; @@ -373,7 +395,7 @@ sncreset(sc) struct snc_softc *sc; { sncstop(sc); - sncinit(sc); + sncinit_locked(sc); } static void @@ -381,15 +403,21 @@ sncinit(xsc) void *xsc; { struct snc_softc *sc = xsc; + + SNC_LOCK(sc); + sncinit_locked(sc); + SNC_UNLOCK(sc); +} + +static void +sncinit_locked(struct snc_softc *sc) +{ u_long s_rcr; - int s; if (sc->sc_ifp->if_drv_flags & IFF_DRV_RUNNING) /* already running */ return; - s = splimp(); - NIC_PUT(sc, SNCR_CR, CR_RST); /* DCR only accessable in reset mode! */ /* config it */ @@ -438,8 +466,8 @@ sncinit(xsc) /* flag interface as "running" */ sc->sc_ifp->if_drv_flags |= IFF_DRV_RUNNING; sc->sc_ifp->if_drv_flags &= ~IFF_DRV_OACTIVE; + callout_reset(&sc->sc_timer, hz, sncwatchdog, sc); - splx(s); return; } @@ -453,7 +481,8 @@ sncstop(sc) struct snc_softc *sc; { struct mtd *mtd; - int s = splimp(); + + SNC_ASSERT_LOCKED(sc); /* stick chip in reset */ NIC_PUT(sc, SNCR_CR, CR_RST); @@ -469,11 +498,10 @@ sncstop(sc) if (++sc->mtd_hw == NTDA) sc->mtd_hw = 0; } - sc->sc_ifp->if_timer = 0; - sc->sc_ifp->if_drv_flags &= ~IFF_DRV_RUNNING; - sc->sc_ifp->if_flags &= ~IFF_UP; + callout_stop(&sc->sc_timer); + sc->sc_tx_timeout = 0; + sc->sc_ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE); - splx(s); return (0); } @@ -483,30 +511,30 @@ sncstop(sc) * will be handled by higher level protocol timeouts. */ static void -sncwatchdog(ifp) - struct ifnet *ifp; +sncwatchdog(void *arg) { - struct snc_softc *sc = ifp->if_softc; + struct snc_softc *sc = arg; struct mtd *mtd; - int temp; - if (sc->mtd_hw != sc->mtd_free) { - /* something still pending for transmit */ - mtd = &sc->mtda[sc->mtd_hw]; - if (SRO(sc, mtd->mtd_vtxp, TXP_STATUS) == 0) - log(LOG_ERR, "%s: Tx - timeout\n", - device_get_nameunit(sc->sc_dev)); - else - log(LOG_ERR, "%s: Tx - lost interrupt\n", - device_get_nameunit(sc->sc_dev)); - temp = ifp->if_flags & IFF_UP; - sncreset(sc); - ifp->if_flags |= temp; + SNC_ASSERT_LOCKED(sc); + if (sc->sc_tx_timeout && --sc->sc_tx_timeout == 0) { + if (sc->mtd_hw != sc->mtd_free) { + /* something still pending for transmit */ + mtd = &sc->mtda[sc->mtd_hw]; + if (SRO(sc, mtd->mtd_vtxp, TXP_STATUS) == 0) + log(LOG_ERR, "%s: Tx - timeout\n", + device_get_nameunit(sc->sc_dev)); + else + log(LOG_ERR, "%s: Tx - lost interrupt\n", + device_get_nameunit(sc->sc_dev)); + sncreset(sc); + } } + callout_reset(&sc->sc_timer, hz, sncwatchdog, sc); } /* - * stuff packet into sonic (at splnet) + * stuff packet into sonic */ static u_int sonicput(sc, m0, mtd_next) @@ -587,7 +615,9 @@ sonicput(sc, m0, mtd_next) wbflush(); NIC_PUT(sc, SNCR_CR, CR_TXP); wbflush(); - sc->sc_ifp->if_timer = 5; /* 5 seconds to watch for failing to transmit */ + + /* 5 seconds to watch for failing to transmit */ + sc->sc_tx_timeout = 5; return (totlen); } @@ -822,6 +852,7 @@ sncintr(arg) if (sc->sc_enabled == 0) return; + SNC_LOCK(sc); while ((isr = (NIC_GET(sc, SNCR_ISR) & ISR_ALL)) != 0) { /* scrub the interrupts that we are going to service */ NIC_PUT(sc, SNCR_ISR, isr); @@ -872,8 +903,9 @@ sncintr(arg) sc->sc_mptally++; #endif } - sncstart(sc->sc_ifp); + sncstart_locked(sc->sc_ifp); } + SNC_UNLOCK(sc); return; } @@ -920,6 +952,7 @@ sonictxint(sc) } #endif /* SNCDEBUG */ + sc->sc_tx_timeout = 0; ifp->if_drv_flags &= ~IFF_DRV_OACTIVE; if (mtd->mtd_mbuf != 0) { @@ -1088,7 +1121,9 @@ sonic_read(sc, pkt, len) #endif /* SNCDEBUG */ /* Pass the packet up. */ + SNC_UNLOCK(sc); (*ifp->if_input)(ifp, m); + SNC_LOCK(sc); return (1); } diff --git a/sys/dev/snc/dp83932var.h b/sys/dev/snc/dp83932var.h index 1ac2ee7..2ed9b9d 100644 --- a/sys/dev/snc/dp83932var.h +++ b/sys/dev/snc/dp83932var.h @@ -200,8 +200,15 @@ typedef struct snc_softc { void *sc_sh; /* shutdownhook cookie */ int gone; + struct mtx sc_lock; + struct callout sc_timer; + int sc_tx_timeout; } snc_softc_t; +#define SNC_LOCK(sc) mtx_lock(&(sc)->sc_lock) +#define SNC_UNLOCK(sc) mtx_unlock(&(sc)->sc_lock) +#define SNC_ASSERT_LOCKED(sc) mtx_assert(&(sc)->sc_lock, MA_OWNED) + /* * Accessing SONIC data structures and registers as 32 bit values * makes code endianess independent. The SONIC is however always in @@ -274,6 +281,6 @@ typedef struct snc_softc { #define CDA_ENABLE 64 /* mask enabling CAM entries */ #define CDA_SIZE(sc) ((4*16 + 1) * ((sc->bitmode) ? 4 : 2)) -void sncconfig(struct snc_softc *, int *, int, int, u_int8_t *); +int sncconfig(struct snc_softc *, int *, int, int, u_int8_t *); void sncintr(void *); void sncshutdown(void *); diff --git a/sys/dev/snc/if_snc.c b/sys/dev/snc/if_snc.c index 7cbdd80..e89e1c7 100644 --- a/sys/dev/snc/if_snc.c +++ b/sys/dev/snc/if_snc.c @@ -159,6 +159,10 @@ snc_release_resources(dev) sc->irq_rid, sc->irq); sc->irq = 0; } + if (sc->sc_ifp) { + if_free(sc->sc_ifp); + sc->sc_ifp = 0; + } } /**************************************************************** @@ -189,6 +193,7 @@ snc_attach(dev) { struct snc_softc *sc = device_get_softc(dev); u_int8_t myea[ETHER_ADDR_LEN]; + int error; if (snc_nec16_register_irq(sc, rman_get_start(sc->irq)) == 0 || snc_nec16_register_mem(sc, rman_get_start(sc->iomem)) == 0) { @@ -220,7 +225,25 @@ snc_attach(dev) return(ENOENT); } - sncconfig(sc, NULL, 0, 0, myea); + mtx_init(&sc->sc_lock, device_get_nameunit(dev), MTX_NETWORK_LOCK, + MTX_DEF); + callout_init_mtx(&sc->sc_timer, &sc->sc_lock, 0); + error = sncconfig(sc, NULL, 0, 0, myea); + if (error) { + snc_release_resources(dev); + mtx_destroy(&sc->sc_lock); + return (error); + } + + error = bus_setup_intr(dev, sc->irq, INTR_TYPE_NET | INTR_MPSAFE, + NULL, sncintr, sc, &sc->irq_handle); + if (error) { + printf("snc_isa_attach: bus_setup_intr() failed\n"); + ether_ifdetach(sc->sc_ifp); + snc_release_resources(dev); + mtx_destroy(&sc->sc_lock); + return (error); + } return 0; } @@ -233,5 +256,9 @@ void snc_shutdown(dev) device_t dev; { - sncshutdown(device_get_softc(dev)); + struct snc_softc *sc = device_get_softc(dev); + + SNC_LOCK(sc); + sncshutdown(sc); + SNC_UNLOCK(sc); } diff --git a/sys/dev/snc/if_snc_cbus.c b/sys/dev/snc/if_snc_cbus.c index f280de3..e859b7a 100644 --- a/sys/dev/snc/if_snc_cbus.c +++ b/sys/dev/snc/if_snc_cbus.c @@ -150,7 +150,7 @@ snc_isa_probe(dev) bus_set_resource(dev, SYS_RES_IOPORT, rid, port, SNEC_NREGS); res = bus_alloc_resource(dev, SYS_RES_IOPORT, &rid, - 0, ~0, SNEC_NREGS, + 0ul, ~0ul, SNEC_NREGS, 0 /* !RF_ACTIVE */); if (res) break; } @@ -181,7 +181,6 @@ snc_isa_attach(dev) device_t dev; { struct snc_softc *sc = device_get_softc(dev); - int error; bzero(sc, sizeof(struct snc_softc)); @@ -189,14 +188,6 @@ snc_isa_attach(dev) snc_alloc_memory(dev, 0); snc_alloc_irq(dev, 0, 0); - error = bus_setup_intr(dev, sc->irq, INTR_TYPE_NET, - NULL, sncintr, sc, &sc->irq_handle); - if (error) { - printf("snc_isa_attach: bus_setup_intr() failed\n"); - snc_release_resources(dev); - return (error); - } - /* This interface is always enabled. */ sc->sc_enabled = 1; diff --git a/sys/dev/snc/if_snc_pccard.c b/sys/dev/snc/if_snc_pccard.c index 5a0a2d5..05babd1 100644 --- a/sys/dev/snc/if_snc_pccard.c +++ b/sys/dev/snc/if_snc_pccard.c @@ -46,11 +46,11 @@ __FBSDID("$FreeBSD$"); #include <sys/bus.h> #include <machine/bus.h> +#include <net/ethernet.h> #include <net/if.h> #include <net/if_arp.h> #include <net/if_media.h> - #include <dev/snc/dp83932var.h> #include <dev/snc/if_sncvar.h> #include <dev/snc/if_sncreg.h> @@ -94,12 +94,15 @@ snc_pccard_detach(device_t dev) device_printf(dev, "already unloaded\n"); return (0); } + SNC_LOCK(sc); sncshutdown(sc); - ifp->if_drv_flags &= ~IFF_DRV_RUNNING; - if_detach(ifp); + SNC_UNLOCK(sc); + callout_drain(&sc->sc_timer); + ether_ifdetach(ifp); sc->gone = 1; bus_teardown_intr(dev, sc->irq, sc->irq_handle); snc_release_resources(dev); + mtx_destroy(&sc->sc_lock); return (0); } @@ -126,7 +129,6 @@ static int snc_pccard_attach(device_t dev) { struct snc_softc *sc = device_get_softc(dev); - int error; bzero(sc, sizeof(struct snc_softc)); @@ -134,14 +136,6 @@ snc_pccard_attach(device_t dev) snc_alloc_memory(dev, 0); snc_alloc_irq(dev, 0, 0); - error = bus_setup_intr(dev, sc->irq, INTR_TYPE_NET, - NULL, sncintr, sc, &sc->irq_handle); - if (error) { - printf("snc_isa_attach: bus_setup_intr() failed\n"); - snc_release_resources(dev); - return (error); - } - /* This interface is always enabled. */ sc->sc_enabled = 1; |