Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 25 Jul 2011 19:13:51 +0000 (UTC)
From:      Marius Strobl <marius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org
Subject:   svn commit: r224397 - stable/7/sys/dev/cas
Message-ID:  <201107251913.p6PJDpkT073345@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
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);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201107251913.p6PJDpkT073345>