From owner-freebsd-current@FreeBSD.ORG Wed Sep 29 15:24:03 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 55D4416A4CF; Wed, 29 Sep 2004 15:24:03 +0000 (GMT) Received: from telecom.net.et (sparrow.telecom.net.et [213.55.64.50]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0CF6743D48; Wed, 29 Sep 2004 15:23:58 +0000 (GMT) (envelope-from mtm@identd.net) Received: from [213.55.68.250] (HELO rogue.acs.lan) by telecom.net.et (CommuniGate Pro SMTP 3.4.8) with ESMTP id 58581950; Wed, 29 Sep 2004 18:16:42 +0300 Received: by rogue.acs.lan (Postfix, from userid 1000) id D8F44B833; Wed, 29 Sep 2004 18:24:03 +0300 (EAT) Date: Wed, 29 Sep 2004 18:24:03 +0300 From: Mike Makonnen To: freebsd-current@freebsd.org, freebsd-net@freebsd.org Message-ID: <20040929152403.GA37746@rogue.acs.lan> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="2fHTh5uZTiUOsy+g" Content-Disposition: inline User-Agent: Mutt/1.4.2.1i X-Operating-System: FreeBSD/6.0-CURRENT (i386) Subject: Making bfe MPSAFE X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Sep 2004 15:24:03 -0000 --2fHTh5uZTiUOsy+g Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi folks, I've cleaned-up the bfe driver's locking. I've also fixed it up so it doesn't need a recursive mute anymore. I'm running it locally and it's working with mpsafenet=1. Please test it on your bfe nics and let me know how it goes. I would also like to here from the networking folks as to the correctness of the cleanups. Cheers. -- Mike Makonnen | GPG-KEY: http://www.identd.net/~mtm/mtm.asc mtm@identd.net | Fingerprint: AC7B 5672 2D11 F4D0 EBF8 5279 5359 2B82 7CD4 1F55 mtm@FreeBSD.Org| FreeBSD - Unleash the Daemon ! --2fHTh5uZTiUOsy+g Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=diff Index: sys/dev/bfe/if_bfe.c =================================================================== RCS file: /home/ncvs/src/sys/dev/bfe/if_bfe.c,v retrieving revision 1.16 diff -u -r1.16 if_bfe.c --- sys/dev/bfe/if_bfe.c 1 Sep 2004 06:10:11 -0000 1.16 +++ sys/dev/bfe/if_bfe.c 29 Sep 2004 08:24:53 -0000 @@ -94,8 +94,10 @@ static void bfe_release_resources (struct bfe_softc *); static void bfe_intr (void *); static void bfe_start (struct ifnet *); +static void bfe_start_locked (struct ifnet *); static int bfe_ioctl (struct ifnet *, u_long, caddr_t); static void bfe_init (void *); +static void bfe_init_locked (void *); static void bfe_stop (struct bfe_softc *); static void bfe_watchdog (struct ifnet *); static void bfe_shutdown (device_t); @@ -329,7 +331,7 @@ sc = device_get_softc(dev); mtx_init(&sc->bfe_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK, - MTX_DEF | MTX_RECURSE); + MTX_DEF); unit = device_get_unit(dev); sc->bfe_dev = dev; @@ -411,7 +413,9 @@ bfe_get_config(sc); /* Reset the chip and turn on the PHY */ + BFE_LOCK(sc); bfe_chip_reset(sc); + BFE_UNLOCK(sc); if (mii_phy_probe(dev, &sc->bfe_miibus, bfe_ifmedia_upd, bfe_ifmedia_sts)) { @@ -433,7 +437,7 @@ /* * Hook interrupt last to avoid having to lock softc */ - error = bus_setup_intr(dev, sc->bfe_irq, INTR_TYPE_NET, + error = bus_setup_intr(dev, sc->bfe_irq, INTR_TYPE_NET | INTR_MPSAFE, bfe_intr, sc, &sc->bfe_intrhand); if (error) { @@ -456,7 +460,7 @@ sc = device_get_softc(dev); KASSERT(mtx_initialized(&sc->bfe_mtx), ("bfe mutex not initialized")); - BFE_LOCK(scp); + BFE_LOCK(sc); ifp = &sc->arpcom.ac_if; @@ -674,15 +678,13 @@ { u_long reg; - BFE_LOCK(sc); + BFE_LOCK_ASSERT(sc); CSR_WRITE_4(sc, BFE_MIB_CTRL, BFE_MIB_CLR_ON_READ); for (reg = BFE_TX_GOOD_O; reg <= BFE_TX_PAUSE; reg += 4) CSR_READ_4(sc, reg); for (reg = BFE_RX_GOOD_O; reg <= BFE_RX_NPAUSE; reg += 4) CSR_READ_4(sc, reg); - - BFE_UNLOCK(sc); } static int @@ -690,23 +692,20 @@ { u_int32_t val; - BFE_LOCK(sc); bfe_writephy(sc, 0, BMCR_RESET); DELAY(100); bfe_readphy(sc, 0, &val); if (val & BMCR_RESET) { printf("bfe%d: PHY Reset would not complete.\n", sc->bfe_unit); - BFE_UNLOCK(sc); return (ENXIO); } - BFE_UNLOCK(sc); return (0); } static void bfe_chip_halt(struct bfe_softc *sc) { - BFE_LOCK(sc); + BFE_LOCK_ASSERT(sc); /* disable interrupts - not that it actually does..*/ CSR_WRITE_4(sc, BFE_IMASK, 0); CSR_READ_4(sc, BFE_IMASK); @@ -717,8 +716,6 @@ CSR_WRITE_4(sc, BFE_DMARX_CTRL, 0); CSR_WRITE_4(sc, BFE_DMATX_CTRL, 0); DELAY(10); - - BFE_UNLOCK(sc); } static void @@ -726,7 +723,7 @@ { u_int32_t val; - BFE_LOCK(sc); + BFE_LOCK_ASSERT(sc); /* Set the interrupt vector for the enet core */ bfe_pci_setup(sc, BFE_INTVEC_ENET0); @@ -804,8 +801,6 @@ bfe_resetphy(sc); bfe_setupphy(sc); - - BFE_UNLOCK(sc); } static void @@ -1032,7 +1027,6 @@ { int err; - BFE_LOCK(sc); /* Clear MII ISR */ CSR_WRITE_4(sc, BFE_EMAC_ISTAT, BFE_EMAC_INT_MII); CSR_WRITE_4(sc, BFE_MDIO_DATA, (BFE_MDIO_SB_START | @@ -1043,7 +1037,6 @@ err = bfe_wait_bit(sc, BFE_EMAC_ISTAT, BFE_EMAC_INT_MII, 100, 0); *val = CSR_READ_4(sc, BFE_MDIO_DATA) & BFE_MDIO_DATA_DATA; - BFE_UNLOCK(sc); return (err); } @@ -1052,7 +1045,6 @@ { int status; - BFE_LOCK(sc); CSR_WRITE_4(sc, BFE_EMAC_ISTAT, BFE_EMAC_INT_MII); CSR_WRITE_4(sc, BFE_MDIO_DATA, (BFE_MDIO_SB_START | (BFE_MDIO_OP_WRITE << BFE_MDIO_OP_SHIFT) | @@ -1061,7 +1053,6 @@ (BFE_MDIO_TA_VALID << BFE_MDIO_TA_SHIFT) | (val & BFE_MDIO_DATA_DATA))); status = bfe_wait_bit(sc, BFE_EMAC_ISTAT, BFE_EMAC_INT_MII, 100, 0); - BFE_UNLOCK(sc); return (status); } @@ -1074,7 +1065,6 @@ bfe_setupphy(struct bfe_softc *sc) { u_int32_t val; - BFE_LOCK(sc); /* Enable activity LED */ bfe_readphy(sc, 26, &val); @@ -1085,7 +1075,6 @@ bfe_readphy(sc, 27, &val); bfe_writephy(sc, 27, val | (1 << 6)); - BFE_UNLOCK(sc); return (0); } @@ -1111,7 +1100,7 @@ struct ifnet *ifp; int i, chipidx; - BFE_LOCK(sc); + BFE_LOCK_ASSERT(sc); ifp = &sc->arpcom.ac_if; @@ -1141,8 +1130,6 @@ ifp->if_timer = 0; else ifp->if_timer = 5; - - BFE_UNLOCK(sc); } /* Pass a received packet up the stack */ @@ -1156,7 +1143,7 @@ int cons; u_int32_t status, current, len, flags; - BFE_LOCK(sc); + BFE_LOCK_ASSERT(sc); cons = sc->bfe_rx_cons; status = CSR_READ_4(sc, BFE_DMARX_STAT); current = (status & BFE_STAT_CDMASK) / sizeof(struct bfe_desc); @@ -1206,7 +1193,6 @@ BFE_INC(cons, BFE_RX_LIST_CNT); } sc->bfe_rx_cons = cons; - BFE_UNLOCK(sc); } static void @@ -1248,7 +1234,7 @@ ifp->if_ierrors++; ifp->if_flags &= ~IFF_RUNNING; - bfe_init(sc); + bfe_init_locked(sc); } /* A packet was received */ @@ -1261,7 +1247,7 @@ /* We have packets pending, fire them out */ if (ifp->if_flags & IFF_RUNNING && !IFQ_DRV_IS_EMPTY(&ifp->if_snd)) - bfe_start(ifp); + bfe_start_locked(ifp); BFE_UNLOCK(sc); } @@ -1350,11 +1336,22 @@ } /* - * Set up to transmit a packet + * Set up to transmit a packet. */ static void bfe_start(struct ifnet *ifp) { + BFE_LOCK((struct bfe_softc *)ifp->if_softc); + bfe_start_locked(ifp); + BFE_UNLOCK((struct bfe_softc *)ifp->if_softc); +} + +/* + * Set up to transmit a packet. The softc is already locked. + */ +static void +bfe_start_locked(struct ifnet *ifp) +{ struct bfe_softc *sc; struct mbuf *m_head = NULL; int idx; @@ -1362,21 +1359,17 @@ sc = ifp->if_softc; idx = sc->bfe_tx_prod; - BFE_LOCK(sc); + BFE_LOCK_ASSERT(sc); /* * Not much point trying to send if the link is down * or we have nothing to send. */ - if (!sc->bfe_link && ifp->if_snd.ifq_len < 10) { - BFE_UNLOCK(sc); + if (!sc->bfe_link && ifp->if_snd.ifq_len < 10) return; - } - if (ifp->if_flags & IFF_OACTIVE) { - BFE_UNLOCK(sc); + if (ifp->if_flags & IFF_OACTIVE) return; - } while(sc->bfe_tx_ring[idx].bfe_mbuf == NULL) { IFQ_DRV_DEQUEUE(&ifp->if_snd, m_head); @@ -1409,21 +1402,26 @@ * Set a timeout in case the chip goes out to lunch. */ ifp->if_timer = 5; - BFE_UNLOCK(sc); } static void bfe_init(void *xsc) { + BFE_LOCK((struct bfe_softc *)xsc); + bfe_init_locked(xsc); + BFE_UNLOCK((struct bfe_softc *)xsc); +} + +static void +bfe_init_locked(void *xsc) +{ struct bfe_softc *sc = (struct bfe_softc*)xsc; struct ifnet *ifp = &sc->arpcom.ac_if; - BFE_LOCK(sc); + BFE_LOCK_ASSERT(sc); - if (ifp->if_flags & IFF_RUNNING) { - BFE_UNLOCK(sc); + if (ifp->if_flags & IFF_RUNNING) return; - } bfe_stop(sc); bfe_chip_reset(sc); @@ -1447,7 +1445,6 @@ ifp->if_flags &= ~IFF_OACTIVE; sc->bfe_stat_ch = timeout(bfe_tick, sc, hz); - BFE_UNLOCK(sc); } /* @@ -1461,8 +1458,6 @@ sc = ifp->if_softc; - BFE_LOCK(sc); - mii = device_get_softc(sc->bfe_miibus); sc->bfe_link = 0; if (mii->mii_instance) { @@ -1473,7 +1468,6 @@ } mii_mediachg(mii); - BFE_UNLOCK(sc); return (0); } @@ -1486,14 +1480,10 @@ struct bfe_softc *sc = ifp->if_softc; struct mii_data *mii; - BFE_LOCK(sc); - mii = device_get_softc(sc->bfe_miibus); mii_pollstat(mii); ifmr->ifm_active = mii->mii_media_active; ifmr->ifm_status = mii->mii_media_status; - - BFE_UNLOCK(sc); } static int @@ -1504,22 +1494,24 @@ struct mii_data *mii; int error = 0; - BFE_LOCK(sc); - switch(command) { case SIOCSIFFLAGS: + BFE_LOCK(sc); if(ifp->if_flags & IFF_UP) if(ifp->if_flags & IFF_RUNNING) bfe_set_rx_mode(sc); else - bfe_init(sc); + bfe_init_locked(sc); else if(ifp->if_flags & IFF_RUNNING) bfe_stop(sc); + BFE_UNLOCK(sc); break; case SIOCADDMULTI: case SIOCDELMULTI: + BFE_LOCK(sc); if(ifp->if_flags & IFF_RUNNING) bfe_set_rx_mode(sc); + BFE_UNLOCK(sc); break; case SIOCGIFMEDIA: case SIOCSIFMEDIA: @@ -1532,7 +1524,6 @@ break; } - BFE_UNLOCK(sc); return (error); } @@ -1548,7 +1539,7 @@ printf("bfe%d: watchdog timeout -- resetting\n", sc->bfe_unit); ifp->if_flags &= ~IFF_RUNNING; - bfe_init(sc); + bfe_init_locked(sc); ifp->if_oerrors++; @@ -1593,7 +1584,7 @@ { struct ifnet *ifp; - BFE_LOCK(sc); + BFE_LOCK_ASSERT(sc); untimeout(bfe_tick, sc, sc->bfe_stat_ch); @@ -1604,6 +1595,4 @@ bfe_rx_ring_free(sc); ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE); - - BFE_UNLOCK(sc); } Index: sys/dev/bfe/if_bfereg.h =================================================================== RCS file: /home/ncvs/src/sys/dev/bfe/if_bfereg.h,v retrieving revision 1.4 diff -u -r1.4 if_bfereg.h --- sys/dev/bfe/if_bfereg.h 1 Sep 2004 06:10:11 -0000 1.4 +++ sys/dev/bfe/if_bfereg.h 28 Sep 2004 19:37:25 -0000 @@ -445,8 +445,9 @@ #define BFE_AND(sc, name, val) \ CSR_WRITE_4(sc, name, CSR_READ_4(sc, name) & val) -#define BFE_LOCK(scp) mtx_lock(&sc->bfe_mtx) -#define BFE_UNLOCK(scp) mtx_unlock(&sc->bfe_mtx) +#define BFE_LOCK_ASSERT(_sc) mtx_assert(&(_sc)->bfe_mtx, MA_OWNED) +#define BFE_LOCK(_sc) mtx_lock(&(_sc)->bfe_mtx) +#define BFE_UNLOCK(_sc) mtx_unlock(&(_sc)->bfe_mtx) #define BFE_INC(x, y) (x) = ((x) == ((y)-1)) ? 0 : (x)+1 --2fHTh5uZTiUOsy+g--