From owner-svn-src-all@FreeBSD.ORG Mon Jul 25 19:13:51 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A0076106566C; Mon, 25 Jul 2011 19:13:51 +0000 (UTC) (envelope-from marius@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 8EF728FC12; Mon, 25 Jul 2011 19:13:51 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id p6PJDpdj073348; Mon, 25 Jul 2011 19:13:51 GMT (envelope-from marius@svn.freebsd.org) Received: (from marius@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p6PJDpkT073345; Mon, 25 Jul 2011 19:13:51 GMT (envelope-from marius@svn.freebsd.org) Message-Id: <201107251913.p6PJDpkT073345@svn.freebsd.org> From: Marius Strobl Date: Mon, 25 Jul 2011 19:13:51 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org X-SVN-Group: stable-7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r224397 - stable/7/sys/dev/cas X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Jul 2011 19:13:51 -0000 Author: marius Date: Mon Jul 25 19:13:51 2011 New Revision: 224397 URL: http://svn.freebsd.org/changeset/base/224397 Log: MFC: r223986 - Expand the scope of the lock in the interrupt routine to close races with checking IFF_DRV_RUNNING and simplify the code. This also involves holding the driver lock in the rx_ch callout. - Just use ifp instead of sc->sc_ifp. Submitted by: jhb (mostly) Modified: stable/7/sys/dev/cas/if_cas.c Directory Properties: stable/7/sys/ (props changed) stable/7/sys/cddl/contrib/opensolaris/ (props changed) stable/7/sys/contrib/dev/acpica/ (props changed) stable/7/sys/contrib/pf/ (props changed) Modified: stable/7/sys/dev/cas/if_cas.c ============================================================================== --- stable/7/sys/dev/cas/if_cas.c Mon Jul 25 19:13:51 2011 (r224396) +++ stable/7/sys/dev/cas/if_cas.c Mon Jul 25 19:13:51 2011 (r224397) @@ -203,7 +203,7 @@ cas_attach(struct cas_softc *sc) IFQ_SET_READY(&ifp->if_snd); callout_init_mtx(&sc->sc_tick_ch, &sc->sc_mtx, 0); - callout_init(&sc->sc_rx_ch, 1); + callout_init_mtx(&sc->sc_rx_ch, &sc->sc_mtx, 0); /* Create local taskq. */ TASK_INIT(&sc->sc_intr_task, 0, cas_intr_task, sc); TASK_INIT(&sc->sc_tx_task, 1, cas_tx_task, ifp); @@ -1597,7 +1597,7 @@ cas_rint_timeout(void *arg) { struct cas_softc *sc = arg; - CAS_LOCK_ASSERT(sc, MA_NOTOWNED); + CAS_LOCK_ASSERT(sc, MA_OWNED); cas_rint(sc); } @@ -1612,7 +1612,7 @@ cas_rint(struct cas_softc *sc) uint32_t rxhead; u_int idx, idx2, len, off, skip; - CAS_LOCK_ASSERT(sc, MA_NOTOWNED); + CAS_LOCK_ASSERT(sc, MA_OWNED); callout_stop(&sc->sc_rx_ch); @@ -1740,7 +1740,9 @@ cas_rint(struct cas_softc *sc) cas_rxcksum(m, CAS_GET(word4, CAS_RC4_TCP_CSUM)); /* Pass it on. */ + CAS_UNLOCK(sc); (*ifp->if_input)(ifp, m); + CAS_LOCK(sc); } else ifp->if_ierrors++; @@ -1836,7 +1838,9 @@ cas_rint(struct cas_softc *sc) cas_rxcksum(m, CAS_GET(word4, CAS_RC4_TCP_CSUM)); /* Pass it on. */ + CAS_UNLOCK(sc); (*ifp->if_input)(ifp, m); + CAS_LOCK(sc); } else ifp->if_ierrors++; @@ -1874,7 +1878,7 @@ cas_free(void *arg1, void *arg2) { struct cas_rxdsoft *rxds; struct cas_softc *sc; - u_int idx; + u_int idx, locked; #if __FreeBSD_version < 800016 rxds = arg2; @@ -1892,17 +1896,18 @@ cas_free(void *arg1, void *arg2) * NB: this function can be called via m_freem(9) within * this driver! */ - + if ((locked = CAS_LOCK_OWNED(sc)) == 0) + CAS_LOCK(sc); cas_add_rxdesc(sc, idx); + if (locked == 0) + CAS_UNLOCK(sc); } static inline void cas_add_rxdesc(struct cas_softc *sc, u_int idx) { - u_int locked; - if ((locked = CAS_LOCK_OWNED(sc)) == 0) - CAS_LOCK(sc); + CAS_LOCK_ASSERT(sc, MA_OWNED); bus_dmamap_sync(sc->sc_rdmatag, sc->sc_rxdsoft[idx].rxds_dmamap, BUS_DMASYNC_PREREAD); @@ -1920,9 +1925,6 @@ cas_add_rxdesc(struct cas_softc *sc, u_i CAS_WRITE_4(sc, CAS_RX_KICK, (sc->sc_rxdptr + CAS_NRXDESC - 4) & CAS_NRXDESC_MASK); } - - if (locked == 0) - CAS_UNLOCK(sc); } static void @@ -1930,7 +1932,7 @@ cas_eint(struct cas_softc *sc, u_int sta { struct ifnet *ifp = sc->sc_ifp; - CAS_LOCK_ASSERT(sc, MA_NOTOWNED); + CAS_LOCK_ASSERT(sc, MA_OWNED); ifp->if_ierrors++; @@ -1947,7 +1949,7 @@ cas_eint(struct cas_softc *sc, u_int sta printf("\n"); ifp->if_drv_flags &= ~IFF_DRV_RUNNING; - cas_init(sc); + cas_init_locked(sc); if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) taskqueue_enqueue(sc->sc_tq, &sc->sc_tx_task); } @@ -1984,6 +1986,7 @@ cas_intr_task(void *arg, int pending __u if (__predict_false((status & CAS_INTR_SUMMARY) == 0)) goto done; + CAS_LOCK(sc); #ifdef CAS_DEBUG CTR4(KTR_CAS, "%s: %s: cplt %x, status %x", device_get_name(sc->sc_dev), __func__, @@ -2023,6 +2026,7 @@ cas_intr_task(void *arg, int pending __u (CAS_INTR_TX_TAG_ERR | CAS_INTR_RX_TAG_ERR | CAS_INTR_RX_LEN_MMATCH | CAS_INTR_PCI_ERROR_INT)) != 0)) { cas_eint(sc, status); + CAS_UNLOCK(sc); return; } @@ -2030,7 +2034,7 @@ cas_intr_task(void *arg, int pending __u status2 = CAS_READ_4(sc, CAS_MAC_TX_STATUS); if ((status2 & (CAS_MAC_TX_UNDERRUN | CAS_MAC_TX_MAX_PKT_ERR)) != 0) - sc->sc_ifp->if_oerrors++; + ifp->if_oerrors++; else if ((status2 & ~CAS_MAC_TX_FRAME_XMTD) != 0) device_printf(sc->sc_dev, "MAC TX fault, status %x\n", status2); @@ -2039,7 +2043,7 @@ cas_intr_task(void *arg, int pending __u if (__predict_false(status & CAS_INTR_RX_MAC_INT)) { status2 = CAS_READ_4(sc, CAS_MAC_RX_STATUS); if ((status2 & CAS_MAC_RX_OVERFLOW) != 0) - sc->sc_ifp->if_ierrors++; + ifp->if_ierrors++; else if ((status2 & ~CAS_MAC_RX_FRAME_RCVD) != 0) device_printf(sc->sc_dev, "MAC RX fault, status %x\n", status2); @@ -2059,16 +2063,15 @@ cas_intr_task(void *arg, int pending __u } if ((status & - (CAS_INTR_TX_INT_ME | CAS_INTR_TX_ALL | CAS_INTR_TX_DONE)) != 0) { - CAS_LOCK(sc); + (CAS_INTR_TX_INT_ME | CAS_INTR_TX_ALL | CAS_INTR_TX_DONE)) != 0) cas_tint(sc); - CAS_UNLOCK(sc); - } - if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) + if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) { + CAS_UNLOCK(sc); return; - else if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + } else if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) taskqueue_enqueue(sc->sc_tq, &sc->sc_tx_task); + CAS_UNLOCK(sc); status = CAS_READ_4(sc, CAS_STATUS_ALIAS); if (__predict_false((status & CAS_INTR_SUMMARY) != 0)) { @@ -2408,7 +2411,7 @@ cas_mii_statchg(device_t dev) v |= CAS_MAC_XIF_CONF_FDXLED; CAS_WRITE_4(sc, CAS_MAC_XIF_CONF, v); - if ((sc->sc_ifp->if_drv_flags & IFF_DRV_RUNNING) != 0 && + if ((ifp->if_drv_flags & IFF_DRV_RUNNING) != 0 && (sc->sc_flags & CAS_LINK) != 0) { CAS_WRITE_4(sc, CAS_MAC_TX_CONF, txcfg | CAS_MAC_TX_CONF_EN);