Module Name:    src
Committed By:   dholland
Date:           Sun Mar 29 05:26:44 UTC 2009

Modified Files:
        src/sys/dev/pci: if_de.c if_devar.h

Log Message:
Merge patch from PR 12605, which tidies up allocation of transmit DMA maps.
Generalize it to also tidy up allocation of receive DMA maps. And change a
few of the symbol names involved to (1) make sure all uses have been fixed
and (2) make it clearer what's actually going on.

Previously the driver was using DMA maps off the free list without fully
allocating them, apparently in order to save two or three lines releasing
them on error paths. According to the submitter of the PR (H.Saito) this
was causing it to reuse a map already in use when under load, resulting
in panics.

I'm not sure if that ought to have been possible or if it reflected an
interrupt handling bug somewhere else, but the change is an improvement
regardless, so we'll go with it.

Compile-tested only, but I've crosschecked the diffs and all that and it's
a pretty noninvasive change.

(Is anyone actually using this driver rather than tlp?)


To generate a diff of this commit:
cvs rdiff -u -r1.130 -r1.131 src/sys/dev/pci/if_de.c
cvs rdiff -u -r1.50 -r1.51 src/sys/dev/pci/if_devar.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/pci/if_de.c
diff -u src/sys/dev/pci/if_de.c:1.130 src/sys/dev/pci/if_de.c:1.131
--- src/sys/dev/pci/if_de.c:1.130	Wed Mar 18 16:00:19 2009
+++ src/sys/dev/pci/if_de.c	Sun Mar 29 05:26:43 2009
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_de.c,v 1.130 2009/03/18 16:00:19 cegger Exp $	*/
+/*	$NetBSD: if_de.c,v 1.131 2009/03/29 05:26:43 dholland Exp $	*/
 
 /*-
  * Copyright (c) 1994-1997 Matt Thomas (m...@3am-software.com)
@@ -37,7 +37,7 @@
  *   board which support 21040, 21041, or 21140 (mostly).
  */
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_de.c,v 1.130 2009/03/18 16:00:19 cegger Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_de.c,v 1.131 2009/03/29 05:26:43 dholland Exp $");
 
 #define	TULIP_HDR_DATA
 
@@ -201,6 +201,32 @@
 #endif
 /* static void tulip_21140_map_media(tulip_softc_t *sc); */
 
+
+static bus_dmamap_t
+tulip_alloc_rxmap(tulip_softc_t *sc)
+{
+	return (sc->tulip_free_rxmaps[--sc->tulip_num_free_rxmaps]);
+}
+
+static void
+tulip_free_rxmap(tulip_softc_t *sc, bus_dmamap_t map)
+{
+	sc->tulip_free_rxmaps[sc->tulip_num_free_rxmaps++] = map;
+}
+
+static bus_dmamap_t
+tulip_alloc_txmap(tulip_softc_t *sc)
+{
+	return (sc->tulip_free_txmaps[--sc->tulip_num_free_txmaps]);
+}
+
+static void
+tulip_free_txmap(tulip_softc_t *sc, bus_dmamap_t map)
+{
+	sc->tulip_free_txmaps[sc->tulip_num_free_txmaps++] = map;
+}
+
+
 static void
 tulip_timeout_callback(
     void *arg)
@@ -3374,7 +3400,7 @@
 #if defined(TULIP_BUS_DMA) && !defined(TULIP_BUS_DMA_NOTX)
 	map = M_GETCTX(m, bus_dmamap_t);
 	bus_dmamap_unload(sc->tulip_dmatag, map);
-	sc->tulip_txmaps[sc->tulip_txmaps_free++] = map;
+	tulip_free_txmap(sc, map);
 #endif
 	m_freem(m);
     }
@@ -3420,7 +3446,7 @@
 #if defined(TULIP_BUS_DMA) && !defined(TULIP_BUS_DMA_NORX)
 	map = M_GETCTX(m, bus_dmamap_t);
 	bus_dmamap_unload(sc->tulip_dmatag, map);
-	sc->tulip_rxmaps[sc->tulip_rxmaps_free++] = map;
+	tulip_free_rxmap(sc, map);
 #endif
 	m_freem(m);
     }
@@ -3587,7 +3613,7 @@
 		map = M_GETCTX(me, bus_dmamap_t);
 		TULIP_RXMAP_POSTSYNC(sc, map);
 		bus_dmamap_unload(sc->tulip_dmatag, map);
-		sc->tulip_rxmaps[sc->tulip_rxmaps_free++] = map;
+		tulip_free_rxmap(sc, map);
 #if defined(DIAGNOSTIC)
 		M_SETCTX(me, NULL);
 #endif
@@ -3619,7 +3645,7 @@
 	    bus_dmamap_sync(sc->tulip_dmatag, map, 0, me->m_len,
 			    BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);
 	    bus_dmamap_unload(sc->tulip_dmatag, map);
-	    sc->tulip_rxmaps[sc->tulip_rxmaps_free++] = map;
+	    tulip_free_rxmap(sc, map);
 #if defined(DIAGNOSTIC)
 	    M_SETCTX(me, NULL);
 #endif
@@ -3681,7 +3707,7 @@
 #if defined(TULIP_BUS_DMA) && !defined(TULIP_BUS_DMA_NORX)
 	    map = M_GETCTX(me, bus_dmamap_t);
 	    bus_dmamap_unload(sc->tulip_dmatag, map);
-	    sc->tulip_rxmaps[sc->tulip_rxmaps_free++] = map;
+	    tulip_free_rxmap(sc, map);
 #if defined(DIAGNOSTIC)
 	    M_SETCTX(me, NULL);
 #endif
@@ -3781,8 +3807,8 @@
 	do {
 	    tulip_desc_t * const nextout = ri->ri_nextout;
 #if defined(TULIP_BUS_DMA) && !defined(TULIP_BUS_DMA_NORX)
-	    if (sc->tulip_rxmaps_free > 0) {
-		map = sc->tulip_rxmaps[--sc->tulip_rxmaps_free];
+	    if (sc->tulip_num_free_rxmaps > 0) {
+		map = tulip_alloc_rxmap(sc);
 	    } else {
 		m_freem(ms);
 		sc->tulip_flags |= TULIP_RXBUFSLOW;
@@ -3882,7 +3908,7 @@
 #if defined(TULIP_BUS_DMA) && !defined(TULIP_BUS_DMA_NOTX)
 		    bus_dmamap_t map = M_GETCTX(m, bus_dmamap_t);
 		    TULIP_TXMAP_POSTSYNC(sc, map);
-		    sc->tulip_txmaps[sc->tulip_txmaps_free++] = map;
+		    tulip_free_txmap(sc, map);
 #endif /* TULIP_BUS_DMA */
 #if NBPFILTER > 0
 		    if (sc->tulip_bpf != NULL)
@@ -4381,14 +4407,14 @@
     /*
      * Reclaim some DMA maps from if we are out.
      */
-    if (sc->tulip_txmaps_free == 0) {
+    if (sc->tulip_num_free_txmaps == 0) {
 #if defined(TULIP_DEBUG)
 	sc->tulip_dbg.dbg_no_txmaps++;
 #endif
 	freedescs += tulip_tx_intr(sc);
     }
-    if (sc->tulip_txmaps_free > 0) {
-	map = sc->tulip_txmaps[sc->tulip_txmaps_free-1];
+    if (sc->tulip_num_free_txmaps > 0) {
+	map = tulip_alloc_txmap(sc);
     } else {
 	sc->tulip_flags |= TULIP_WANTTXSTART;
 #if defined(TULIP_DEBUG)
@@ -4409,6 +4435,7 @@
 #if defined(TULIP_DEBUG)
 		sc->tulip_dbg.dbg_txput_finishes[2]++;
 #endif
+		tulip_free_txmap(sc, map);
 		goto finish;
 	    }
 	    error = bus_dmamap_load_mbuf(sc->tulip_dmatag, map, m, BUS_DMA_NOWAIT);
@@ -4419,6 +4446,7 @@
 #if defined(TULIP_DEBUG)
 	    sc->tulip_dbg.dbg_txput_finishes[3]++;
 #endif
+	    tulip_free_txmap(sc, map);
 	    goto finish;
 	}
     }
@@ -4438,6 +4466,7 @@
 	sc->tulip_dbg.dbg_txput_finishes[4]++;
 #endif
 	bus_dmamap_unload(sc->tulip_dmatag, map);
+	tulip_free_txmap(sc, map);
 	goto finish;
     }
     for (; map->dm_nsegs - segcnt > 1; segcnt += 2) {
@@ -4466,7 +4495,6 @@
     TULIP_TXMAP_PRESYNC(sc, map);
     M_SETCTX(m, map);
     map = NULL;
-    --sc->tulip_txmaps_free;		/* commit to using the dmamap */
 
 #else /* !TULIP_BUS_DMA */
 
@@ -5208,18 +5236,18 @@
     }
 
     /*
-     * Allocate dmamaps for each transmit descriptors
+     * Allocate dmamaps for each transmit descriptor, and place on the
+     * free list.
      */
     if (error == 0) {
-	while (error == 0 && sc->tulip_txmaps_free < TULIP_TXDESCS) {
+	while (error == 0 && sc->tulip_num_free_txmaps < TULIP_TXDESCS) {
 	    bus_dmamap_t map;
 	    if ((error = TULIP_TXMAP_CREATE(sc, &map)) == 0)
-		sc->tulip_txmaps[sc->tulip_txmaps_free++] = map;
+		tulip_free_txmap(sc, map);
 	}
 	if (error) {
-	    while (sc->tulip_txmaps_free > 0)
-		bus_dmamap_destroy(sc->tulip_dmatag,
-				   sc->tulip_txmaps[--sc->tulip_txmaps_free]);
+	    while (sc->tulip_num_free_txmaps > 0)
+		bus_dmamap_destroy(sc->tulip_dmatag, tulip_alloc_txmap(sc));
 	}
     }
 #else
@@ -5240,18 +5268,18 @@
     }
 
     /*
-     * Allocate dmamaps for each receive descriptors
+     * Allocate dmamaps for each receive descriptor, and place on the
+     * free list.
      */
     if (error == 0) {
-	while (error == 0 && sc->tulip_rxmaps_free < TULIP_RXDESCS) {
+	while (error == 0 && sc->tulip_num_free_rxmaps < TULIP_RXDESCS) {
 	    bus_dmamap_t map;
 	    if ((error = TULIP_RXMAP_CREATE(sc, &map)) == 0)
-		sc->tulip_rxmaps[sc->tulip_rxmaps_free++] = map;
+		tulip_free_rxmap(sc, map);
 	}
 	if (error) {
-	    while (sc->tulip_rxmaps_free > 0)
-		bus_dmamap_destroy(sc->tulip_dmatag,
-				   sc->tulip_rxmaps[--sc->tulip_rxmaps_free]);
+	    while (sc->tulip_num_free_rxmaps > 0)
+		bus_dmamap_destroy(sc->tulip_dmatag, tulip_alloc_rxmap(sc));
 	}
     }
 #else

Index: src/sys/dev/pci/if_devar.h
diff -u src/sys/dev/pci/if_devar.h:1.50 src/sys/dev/pci/if_devar.h:1.51
--- src/sys/dev/pci/if_devar.h:1.50	Thu Jun 12 22:44:47 2008
+++ src/sys/dev/pci/if_devar.h	Sun Mar 29 05:26:43 2009
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_devar.h,v 1.50 2008/06/12 22:44:47 cegger Exp $	*/
+/*	$NetBSD: if_devar.h,v 1.51 2009/03/29 05:26:43 dholland Exp $	*/
 
 /*-
  * Copyright (c) 1994-1997 Matt Thomas (m...@3am-software.com)
@@ -509,13 +509,13 @@
 #if !defined(TULIP_BUS_DMA_NOTX)
     bus_dmamap_t tulip_setupmap;
     bus_dmamap_t tulip_txdescmap;
-    bus_dmamap_t tulip_txmaps[TULIP_TXDESCS];
-    unsigned tulip_txmaps_free;
+    bus_dmamap_t tulip_free_txmaps[TULIP_TXDESCS];
+    unsigned tulip_num_free_txmaps;
 #endif
 #if !defined(TULIP_BUS_DMA_NORX)
     bus_dmamap_t tulip_rxdescmap;
-    bus_dmamap_t tulip_rxmaps[TULIP_RXDESCS];
-    unsigned tulip_rxmaps_free;
+    bus_dmamap_t tulip_free_rxmaps[TULIP_RXDESCS];
+    unsigned tulip_num_free_rxmaps;
 #endif
 #endif
 #if !defined(__NetBSD__)

Reply via email to