Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 14 Dec 2013 01:35:57 +0000 (UTC)
From:      Ian Lepore <ian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r259381 - stable/10/sys/arm/at91
Message-ID:  <201312140135.rBE1ZvkH025242@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ian
Date: Sat Dec 14 01:35:57 2013
New Revision: 259381
URL: http://svnweb.freebsd.org/changeset/base/259381

Log:
  MFC r259212, r259220:
  
    Fix one race and one fence post error. When the TX buffer was
    completely full, we'd not complete any of the mbufs due to the fence
    post error (this creates a large leak). When this is fixed, we still
    leak, but at a much smaller rate due to a race between ateintr and
    atestart_locked as well as an asymmetry where atestart_locked is
    called from elsewhere.  Ensure that we free in-flight packets that
    have completed there as well. Also remove needless check for NULL on
    mb, checked earlier in the loop and simplify a redundant if.

Modified:
  stable/10/sys/arm/at91/if_ate.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/arm/at91/if_ate.c
==============================================================================
--- stable/10/sys/arm/at91/if_ate.c	Sat Dec 14 01:34:24 2013	(r259380)
+++ stable/10/sys/arm/at91/if_ate.c	Sat Dec 14 01:35:57 2013	(r259381)
@@ -947,10 +947,8 @@ ate_intr(void *xsc)
 
 			} while (!done);
 
-			if (mb != NULL) {
-				ifp->if_ipackets++;
-				(*ifp->if_input)(ifp, mb);
-			}
+			ifp->if_ipackets++;
+			(*ifp->if_input)(ifp, mb);
 		}
 	}
 
@@ -974,16 +972,14 @@ ate_intr(void *xsc)
 				sc->tx_descs[sc->txtail + 1].status |= ETHB_TX_USED;
 		}
 
-		while (sc->txtail != sc->txhead &&
-		    sc->tx_descs[sc->txtail].status & ETHB_TX_USED ) {
-
+		while ((sc->tx_descs[sc->txtail].status & ETHB_TX_USED) &&
+		    sc->sent_mbuf[sc->txtail] != NULL) {
 			bus_dmamap_sync(sc->mtag, sc->tx_map[sc->txtail],
 			    BUS_DMASYNC_POSTWRITE);
 			bus_dmamap_unload(sc->mtag, sc->tx_map[sc->txtail]);
 			m_freem(sc->sent_mbuf[sc->txtail]);
 			sc->tx_descs[sc->txtail].addr = 0;
 			sc->sent_mbuf[sc->txtail] = NULL;
-
 			ifp->if_opackets++;
 			sc->txtail = NEXT_TX_IDX(sc, sc->txtail);
 		}
@@ -1118,12 +1114,10 @@ atestart_locked(struct ifnet *ifp)
 		 * xmit packets.  We use OACTIVE to indicate "we can stuff more
 		 * into our buffers (clear) or not (set)."
 		 */
-		if (!sc->is_emacb) {
-			/* RM9200 has only two hardware entries */
-			if (!sc->is_emacb && (RD4(sc, ETH_TSR) & ETH_TSR_BNQ) == 0) {
-				ifp->if_drv_flags |= IFF_DRV_OACTIVE;
-				return;
-			}
+		/* RM9200 has only two hardware entries */
+		if (!sc->is_emacb && (RD4(sc, ETH_TSR) & ETH_TSR_BNQ) == 0) {
+			ifp->if_drv_flags |= IFF_DRV_OACTIVE;
+			return;
 		}
 
 		IFQ_DRV_DEQUEUE(&ifp->if_snd, m);
@@ -1146,6 +1140,21 @@ atestart_locked(struct ifnet *ifp)
 			m_freem(m);
 			continue;
 		}
+
+		/*
+		 * There's a small race between the loop in ate_intr finishing
+		 * and the check above to see if the packet was finished, as well
+		 * as when atestart gets called via other paths. Lose the race
+		 * gracefully and free the mbuf...
+		 */
+		if (sc->sent_mbuf[sc->txhead] != NULL) {
+			bus_dmamap_sync(sc->mtag, sc->tx_map[sc->txtail],
+			    BUS_DMASYNC_POSTWRITE);
+			bus_dmamap_unload(sc->mtag, sc->tx_map[sc->txtail]);
+			m_free(sc->sent_mbuf[sc->txhead]);
+			ifp->if_opackets++;
+		}
+		
 		sc->sent_mbuf[sc->txhead] = m;
 
 		bus_dmamap_sync(sc->mtag, sc->tx_map[sc->txhead],



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