Date: Mon, 4 Oct 2004 14:51:35 +0900 From: Pyun YongHyeon <yongari@kt-is.co.kr> To: freebsd-sparc64@freebsd.org Subject: Call for testers, hme(4) lock patch Message-ID: <20041004055135.GA1226@kt-is.co.kr>
next in thread | raw e-mail | index | archive | help
--n8g4imXOkfNTN/H1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Ok, there is a patch that needs extensive testing which removes Giant lock in hme(4) driver. The patch was lightly tested on U2/AXe and had no problems. If you encounter a panic related with the patch, please let me know.(Don't forget to send your 'backtrace' output.) Thanks. -- Pyun YongHyeon <http://www.kr.freebsd.org/~yongari>; --n8g4imXOkfNTN/H1 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="hme.lock.patch" --- sys/dev/hme/if_hme.c.orig Tue Aug 17 11:22:06 2004 +++ sys/dev/hme/if_hme.c Mon Oct 4 14:27:37 2004 @@ -99,11 +99,13 @@ #include <dev/hme/if_hmevar.h> static void hme_start(struct ifnet *); +static void hme_start_locked(struct ifnet *); static void hme_stop(struct hme_softc *); static int hme_ioctl(struct ifnet *, u_long, caddr_t); static void hme_tick(void *); static void hme_watchdog(struct ifnet *); static void hme_init(void *); +static void hme_init_locked(void *); static int hme_add_rxbuf(struct hme_softc *, unsigned int, int); static int hme_meminit(struct hme_softc *); static int hme_mac_bitflip(struct hme_softc *, u_int32_t, u_int32_t, @@ -195,8 +197,11 @@ * */ + HME_LOCK_ASSERT(sc, MA_NOTOWNED); /* Make sure the chip is stopped. */ + HME_LOCK(sc); hme_stop(sc); + HME_UNLOCK(sc); /* * Allocate DMA capable memory @@ -284,8 +289,7 @@ if_initname(ifp, device_get_name(sc->sc_dev), device_get_unit(sc->sc_dev)); ifp->if_mtu = ETHERMTU; - ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST | - IFF_NEEDSGIANT; + ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST; ifp->if_start = hme_start; ifp->if_ioctl = hme_ioctl; ifp->if_init = hme_init; @@ -294,7 +298,9 @@ ifp->if_snd.ifq_drv_maxlen = HME_NTXQ; IFQ_SET_READY(&ifp->if_snd); + HME_LOCK(sc); hme_mifinit(sc); + HME_UNLOCK(sc); if ((error = mii_phy_probe(sc->sc_dev, &sc->sc_miibus, hme_mediachange, hme_mediastatus)) != 0) { @@ -339,7 +345,7 @@ ifp->if_hwassist |= sc->sc_csum_features; ifp->if_capenable |= IFCAP_VLAN_MTU | IFCAP_HWCSUM; - callout_init(&sc->sc_tick_ch, 0); + callout_init(&sc->sc_tick_ch, CALLOUT_MPSAFE); return (0); fail_txdesc: @@ -373,8 +379,12 @@ struct ifnet *ifp = &sc->sc_arpcom.ac_if; int i; + HME_LOCK_ASSERT(sc, MA_NOTOWNED); + ether_ifdetach(ifp); + HME_LOCK(sc); hme_stop(sc); + HME_UNLOCK(sc); device_delete_child(sc->sc_dev, sc->sc_miibus); for (i = 0; i < HME_NTXQ; i++) { @@ -400,7 +410,9 @@ hme_suspend(struct hme_softc *sc) { + HME_LOCK(sc); hme_stop(sc); + HME_UNLOCK(sc); } void @@ -408,8 +420,10 @@ { struct ifnet *ifp = &sc->sc_arpcom.ac_if; + HME_LOCK(sc); if ((ifp->if_flags & IFF_UP) != 0) - hme_init(ifp); + hme_init_locked(ifp); + HME_UNLOCK(sc); } static void @@ -441,9 +455,11 @@ { int s; + HME_LOCK(sc); s = splnet(); - hme_init(sc); + hme_init_locked(sc); splx(s); + HME_UNLOCK(sc); } static void @@ -671,10 +687,21 @@ hme_init(void *xsc) { struct hme_softc *sc = (struct hme_softc *)xsc; + + HME_LOCK(sc); + hme_init_locked(sc); + HME_UNLOCK(sc); +} + +static void +hme_init_locked(void *xsc) +{ + struct hme_softc *sc = (struct hme_softc *)xsc; struct ifnet *ifp = &sc->sc_arpcom.ac_if; u_int8_t *ea; u_int32_t n, v; + HME_LOCK_ASSERT(sc, MA_OWNED); /* * Initialization sequence. The numbered steps below correspond * to the sequence outlined in section 6.3.5.1 in the Ethernet @@ -857,7 +884,7 @@ ifp->if_flags |= IFF_RUNNING; ifp->if_flags &= ~IFF_OACTIVE; ifp->if_timer = 0; - hme_start(ifp); + hme_start_locked(ifp); } struct hme_txdma_arg { @@ -1059,12 +1086,24 @@ if (ifp->if_capenable & IFCAP_RXCSUM) hme_rxcksum(m, flags); /* Pass the packet up. */ + HME_UNLOCK(sc); (*ifp->if_input)(ifp, m); + HME_LOCK(sc); } static void hme_start(struct ifnet *ifp) { + struct hme_softc *sc = ifp->if_softc; + + HME_LOCK(sc); + hme_start_locked(ifp); + HME_UNLOCK(sc); +} + +static void +hme_start_locked(struct ifnet *ifp) +{ struct hme_softc *sc = (struct hme_softc *)ifp->if_softc; struct mbuf *m; int error, enq = 0; @@ -1173,7 +1212,7 @@ /* Update ring */ sc->sc_rb.rb_tdtail = ri; - hme_start(ifp); + hme_start_locked(ifp); if (sc->sc_rb.rb_td_nbusy == 0) ifp->if_timer = 0; @@ -1302,6 +1341,7 @@ struct hme_softc *sc = (struct hme_softc *)v; u_int32_t status; + HME_LOCK(sc); status = HME_SEB_READ_4(sc, HME_SEBI_STAT); CTR1(KTR_HME, "hme_intr: status %#x", (u_int)status); @@ -1313,6 +1353,7 @@ if ((status & HME_SEB_STAT_RXTOHOST) != 0) hme_rint(sc); + HME_UNLOCK(sc); } @@ -1322,12 +1363,16 @@ struct hme_softc *sc = ifp->if_softc; #ifdef HMEDEBUG u_int32_t status; +#endif + HME_LOCK(sc); +#ifdef HMEDEBUG status = HME_SEB_READ_4(sc, HME_SEBI_STAT); CTR1(KTR_HME, "hme_watchdog: status %x", (u_int)status); #endif device_printf(sc->sc_dev, "device timeout\n"); ++ifp->if_oerrors; + HME_UNLOCK(sc); hme_reset(sc); } @@ -1340,6 +1385,8 @@ { u_int32_t v; + HME_LOCK_ASSERT(sc, MA_OWNED); + /* Configure the MIF in frame mode */ v = HME_MIF_READ_4(sc, HME_MIFI_CFG); v &= ~HME_MIF_CFG_BBMODE; @@ -1356,6 +1403,7 @@ int n; u_int32_t v; + HME_LOCK(sc); /* Select the desired PHY in the MIF configuration register */ v = HME_MIF_READ_4(sc, HME_MIFI_CFG); /* Clear PHY select bit */ @@ -1376,11 +1424,14 @@ for (n = 0; n < 100; n++) { DELAY(1); v = HME_MIF_READ_4(sc, HME_MIFI_FO); - if (v & HME_MIF_FO_TALSB) + if (v & HME_MIF_FO_TALSB) { + HME_UNLOCK(sc); return (v & HME_MIF_FO_DATA); + } } device_printf(sc->sc_dev, "mii_read timeout\n"); + HME_UNLOCK(sc); return (0); } @@ -1391,6 +1442,7 @@ int n; u_int32_t v; + HME_LOCK(sc); /* Select the desired PHY in the MIF configuration register */ v = HME_MIF_READ_4(sc, HME_MIFI_CFG); /* Clear PHY select bit */ @@ -1412,11 +1464,14 @@ for (n = 0; n < 100; n++) { DELAY(1); v = HME_MIF_READ_4(sc, HME_MIFI_FO); - if (v & HME_MIF_FO_TALSB) + if (v & HME_MIF_FO_TALSB) { + HME_UNLOCK(sc); return (1); + } } device_printf(sc->sc_dev, "mii_write timeout\n"); + HME_UNLOCK(sc); return (0); } @@ -1424,10 +1479,13 @@ hme_mii_statchg(device_t dev) { struct hme_softc *sc = device_get_softc(dev); - int instance = IFM_INST(sc->sc_mii->mii_media.ifm_cur->ifm_media); - int phy = sc->sc_phys[instance]; + int instance; + int phy; u_int32_t v; + HME_LOCK(sc); + instance = IFM_INST(sc->sc_mii->mii_media.ifm_cur->ifm_media); + phy = sc->sc_phys[instance]; #ifdef HMEDEBUG if (sc->sc_debug) printf("hme_mii_statchg: status change: phy = %d\n", phy); @@ -1442,15 +1500,20 @@ /* Set the MAC Full Duplex bit appropriately */ v = HME_MAC_READ_4(sc, HME_MACI_TXCFG); - if (!hme_mac_bitflip(sc, HME_MACI_TXCFG, v, HME_MAC_TXCFG_ENABLE, 0)) + if (!hme_mac_bitflip(sc, HME_MACI_TXCFG, v, HME_MAC_TXCFG_ENABLE, 0)) { + HME_UNLOCK(sc); return; + } if ((IFM_OPTIONS(sc->sc_mii->mii_media_active) & IFM_FDX) != 0) v |= HME_MAC_TXCFG_FULLDPLX; else v &= ~HME_MAC_TXCFG_FULLDPLX; HME_MAC_WRITE_4(sc, HME_MACI_TXCFG, v); - if (!hme_mac_bitflip(sc, HME_MACI_TXCFG, v, 0, HME_MAC_TXCFG_ENABLE)) + if (!hme_mac_bitflip(sc, HME_MACI_TXCFG, v, 0, HME_MAC_TXCFG_ENABLE)) { + HME_UNLOCK(sc); return; + } + HME_UNLOCK(sc); } static int @@ -1466,12 +1529,18 @@ { struct hme_softc *sc = ifp->if_softc; - if ((ifp->if_flags & IFF_UP) == 0) + HME_LOCK(sc); + if ((ifp->if_flags & IFF_UP) == 0) { + HME_UNLOCK(sc); return; + } + HME_UNLOCK(sc); mii_pollstat(sc->sc_mii); + HME_LOCK(sc); ifmr->ifm_active = sc->sc_mii->mii_media_active; ifmr->ifm_status = sc->sc_mii->mii_media_status; + HME_UNLOCK(sc); } /* @@ -1484,6 +1553,7 @@ struct ifreq *ifr = (struct ifreq *)data; int s, error = 0; + HME_LOCK(sc); s = splnet(); switch (cmd) { @@ -1502,13 +1572,13 @@ * If interface is marked up and it is stopped, then * start it. */ - hme_init(sc); + hme_init_locked(sc); } else if ((ifp->if_flags & IFF_UP) != 0) { /* * Reset the interface to pick up changes in any other * flags that affect hardware registers. */ - hme_init(sc); + hme_init_locked(sc); } if ((ifp->if_flags & IFF_LINK0) != 0) sc->sc_csum_features |= CSUM_UDP; @@ -1528,7 +1598,9 @@ break; case SIOCGIFMEDIA: case SIOCSIFMEDIA: + HME_UNLOCK(sc); error = ifmedia_ioctl(ifp, ifr, &sc->sc_mii->mii_media, cmd); + HME_LOCK(sc); break; case SIOCSIFCAP: ifp->if_capenable = ifr->ifr_reqcap; @@ -1538,11 +1610,14 @@ ifp->if_hwassist = 0; break; default: + HME_UNLOCK(sc); error = ether_ioctl(ifp, cmd, data); + HME_LOCK(sc); break; } splx(s); + HME_UNLOCK(sc); return (error); } @@ -1558,6 +1633,7 @@ u_int32_t hash[4]; u_int32_t macc; + HME_LOCK_ASSERT(sc, MA_OWNED); /* Clear hash table */ hash[3] = hash[2] = hash[1] = hash[0] = 0; --- sys/dev/hme/if_hme_pci.c.orig Tue Aug 17 11:22:06 2004 +++ sys/dev/hme/if_hme_pci.c Fri Oct 1 20:35:54 2004 @@ -185,6 +185,8 @@ sc->sc_pci = 1; sc->sc_dev = dev; + mtx_init(&sc->sc_lock, device_get_nameunit(dev), MTX_NETWORK_LOCK, + MTX_DEF); /* * Map five register banks: @@ -201,7 +203,8 @@ &hsc->hsc_srid, RF_ACTIVE); if (hsc->hsc_sres == NULL) { device_printf(dev, "could not map device registers\n"); - return (ENXIO); + error = ENXIO; + goto fail_mtx; } hsc->hsc_irid = 0; hsc->hsc_ires = bus_alloc_resource_any(dev, SYS_RES_IRQ, @@ -347,8 +350,8 @@ goto fail_ires; } - if ((error = bus_setup_intr(dev, hsc->hsc_ires, INTR_TYPE_NET, hme_intr, - sc, &hsc->hsc_ih)) != 0) { + if ((error = bus_setup_intr(dev, hsc->hsc_ires, INTR_TYPE_NET | + INTR_MPSAFE, hme_intr, sc, &hsc->hsc_ih)) != 0) { device_printf(dev, "couldn't establish interrupt\n"); hme_detach(sc); goto fail_ires; @@ -359,6 +362,8 @@ bus_release_resource(dev, SYS_RES_IRQ, hsc->hsc_irid, hsc->hsc_ires); fail_sres: bus_release_resource(dev, SYS_RES_MEMORY, hsc->hsc_srid, hsc->hsc_sres); +fail_mtx: + mtx_destroy(&sc->sc_lock); return (error); } @@ -368,9 +373,8 @@ struct hme_pci_softc *hsc = device_get_softc(dev); struct hme_softc *sc = &hsc->hsc_hme; - hme_detach(sc); - bus_teardown_intr(dev, hsc->hsc_ires, hsc->hsc_ih); + hme_detach(sc); bus_release_resource(dev, SYS_RES_IRQ, hsc->hsc_irid, hsc->hsc_ires); bus_release_resource(dev, SYS_RES_MEMORY, hsc->hsc_srid, hsc->hsc_sres); return (0); --- sys/dev/hme/if_hme_sbus.c.orig Tue Aug 17 11:22:06 2004 +++ sys/dev/hme/if_hme_sbus.c Fri Oct 1 20:35:54 2004 @@ -152,6 +152,8 @@ u_long start, count; int error = 0; + mtx_init(&sc->sc_lock, device_get_nameunit(dev), MTX_NETWORK_LOCK, + MTX_DEF); /* * Map five register banks: * @@ -167,7 +169,8 @@ &hsc->hsc_seb_rid, RF_ACTIVE); if (hsc->hsc_seb_res == NULL) { device_printf(dev, "cannot map SEB registers\n"); - return (ENXIO); + error = ENXIO; + goto fail_mtx_res; } sc->sc_sebt = rman_get_bustag(hsc->hsc_seb_res); sc->sc_sebh = rman_get_bushandle(hsc->hsc_seb_res); @@ -265,8 +268,8 @@ goto fail_ires; } - if ((error = bus_setup_intr(dev, hsc->hsc_ires, INTR_TYPE_NET, hme_intr, - sc, &hsc->hsc_ih)) != 0) { + if ((error = bus_setup_intr(dev, hsc->hsc_ires, INTR_TYPE_NET | + INTR_MPSAFE, hme_intr, sc, &hsc->hsc_ih)) != 0) { device_printf(dev, "couldn't establish interrupt\n"); hme_detach(sc); goto fail_ires; @@ -292,6 +295,8 @@ fail_seb_res: bus_release_resource(dev, SYS_RES_MEMORY, hsc->hsc_seb_rid, hsc->hsc_seb_res); +fail_mtx_res: + mtx_destroy(&sc->sc_lock); return (error); } @@ -301,9 +306,8 @@ struct hme_sbus_softc *hsc = device_get_softc(dev); struct hme_softc *sc = &hsc->hsc_hme; - hme_detach(sc); - bus_teardown_intr(dev, hsc->hsc_ires, hsc->hsc_ih); + hme_detach(sc); if (hsc->hsc_mif_res != NULL) { bus_release_resource(dev, SYS_RES_MEMORY, hsc->hsc_mif_rid, hsc->hsc_mif_res); @@ -316,6 +320,7 @@ hsc->hsc_etx_res); bus_release_resource(dev, SYS_RES_MEMORY, hsc->hsc_seb_rid, hsc->hsc_seb_res); + mtx_destroy(&sc->sc_lock); return (0); } --- sys/dev/hme/if_hmevar.h.orig Tue Aug 17 11:22:06 2004 +++ sys/dev/hme/if_hmevar.h Fri Oct 1 20:36:10 2004 @@ -141,7 +141,12 @@ struct hme_ring sc_rb; int sc_debug; + struct mtx sc_lock; }; + +#define HME_LOCK(_sc) mtx_lock(&(_sc)->sc_lock) +#define HME_UNLOCK(_sc) mtx_unlock(&(_sc)->sc_lock) +#define HME_LOCK_ASSERT(_sc, _what) mtx_assert(&(_sc)->sc_lock, (_what)) extern devclass_t hme_devclass; --n8g4imXOkfNTN/H1--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20041004055135.GA1226>