svn commit: r259212 - head/sys/arm/at91

[Warner Losh]

Author: imp
Date: Wed Dec 11 05:32:29 2013
New Revision: 259212
URL: http://svnweb.freebsd.org/changeset/base/259212

Log:
  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.
  
  MFC after:	3 days

Modified:
  head/sys/arm/at91/if_ate.c

Modified: head/sys/arm/at91/if_ate.c
==============================================================================
--- head/sys/arm/at91/if_ate.c	Wed Dec 11 04:31:40 2013	(r259211)
+++ head/sys/arm/at91/if_ate.c	Wed Dec 11 05:32:29 2013	(r259212)
@@ -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. Loose 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],