NetBSD Problem Report #12605

Received: (qmail 29966 invoked from network); 11 Apr 2001 00:57:12 -0000
Message-Id: <200104100427.f3A4ROG01002@vw4.densan.co.jp>
Date: Mon, 9 Apr 2001 21:27:24 -0700 (PDT)
From: "H.Saito" <saito@densan.co.jp>
Reply-To: saito@densan.co.jp
To: gnats-bugs@gnats.netbsd.org
Subject: panic: _bus_dmamap_sync
X-Send-Pr-Version: 3.95

>Number:         12605
>Category:       kern
>Synopsis:       bug of sys/dev/pci/if_de.c
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Apr 11 00:58:00 +0000 2001
>Closed-Date:    Sun Mar 29 05:33:34 +0000 2009
>Last-Modified:  Sun Mar 29 05:33:34 +0000 2009
>Originator:     H.Saito
>Release:        1.5
>Organization:
>Environment:
    DCP-R5000/30(DENSAN Compact-PCI bus Board)
    big-endian, mips1, ELF
System: NetBSD vw5 1.5 NetBSD 1.5 (GENERIC) #4: Mon Apr 9 16:40:41 JST 2001 saito@vw4:/user5/NetBSD-1.5/usr/src/sys/arch/devos/compile/GENERIC devos


>Description:
    A duplicate dma maps pointer was allocated from tulip_txput()
    of sys/dev/pci/if_de.c.

    Mayby, this Program is wrong, please see below.

    if (sc->tulip_txmaps_free > 0) {
        map = sc->tulip_txmaps[sc->tulip_txmaps_free-1];
    } else {
        ...
    }
    ...
    --sc->tulip_txmaps_free;            /* commit to using the dmamap */

>How-To-Repeat:
    If NetBSD-1.5 transfer too many data with another machine by TCP/IP,
    and cp a large file to /mnt that mounted by nfs with another machine,
    immediately crashed(panic: _bus_dmamap_sync).

>Fix:
--- if_de.c.org	Thu Mar 23 16:01:37 2000
+++ if_de.c	Mon Apr  9 15:14:38 2001
@@ -3296,6 +3303,30 @@
 #endif
 }
 
+
+static void put_tulip_txmaps(tulip_softc_t * const sc, bus_dmamap_t map);
+static bus_dmamap_t get_tulip_txmaps(tulip_softc_t * const sc);
+
+static void
+put_tulip_txmaps(tulip_softc_t * const sc, bus_dmamap_t map)
+{
+#if defined(TULIP_TXMAPS_DEBUG)
+	int n;
+
+	for (n = 0; n < sc->tulip_txmaps_free; n++) {
+		if (sc->tulip_txmaps[n] == map)
+			printf("put_tulip_txmaps: %d: 0x%x\n", n, (int)map);
+	}
+#endif
+	sc->tulip_txmaps[sc->tulip_txmaps_free++] = map;
+}
+
+static bus_dmamap_t
+get_tulip_txmaps(tulip_softc_t * const sc)
+{
+	return (sc->tulip_txmaps[--sc->tulip_txmaps_free]);
+}
+
 static void
 tulip_reset(
     tulip_softc_t * const sc)
@@ -3360,7 +3396,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;
+	put_tulip_txmaps(sc, map);
 #endif
 	m_freem(m);
     }
@@ -3868,7 +3904,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;
+		    put_tulip_txmaps(sc, map);
 #endif /* TULIP_BUS_DMA */
 #if NBPFILTER > 0
 		    if (sc->tulip_bpf != NULL)
@@ -4374,7 +4410,7 @@
 	free += tulip_tx_intr(sc);
     }
     if (sc->tulip_txmaps_free > 0) {
-	map = sc->tulip_txmaps[sc->tulip_txmaps_free-1];
+	map = get_tulip_txmaps(sc);
     } else {
 	sc->tulip_flags |= TULIP_WANTTXSTART;
 #if defined(TULIP_DEBUG)
@@ -4395,6 +4431,7 @@
 #if defined(TULIP_DEBUG)
 		sc->tulip_dbg.dbg_txput_finishes[2]++;
 #endif
+		put_tulip_txmaps(sc, map);
 		goto finish;
 	    }
 	    error = bus_dmamap_load_mbuf(sc->tulip_dmatag, map, m, BUS_DMA_NOWAIT);
@@ -4405,6 +4442,7 @@
 #if defined(TULIP_DEBUG)
 	    sc->tulip_dbg.dbg_txput_finishes[3]++;
 #endif
+	    put_tulip_txmaps(sc, map);
 	    goto finish;
 	}
     }
@@ -4424,6 +4462,7 @@
 	sc->tulip_dbg.dbg_txput_finishes[4]++;
 #endif
 	bus_dmamap_unload(sc->tulip_dmatag, map);
+	put_tulip_txmaps(sc, map);
 	goto finish;
     }
     for (; map->dm_nsegs - segcnt > 1; segcnt += 2) {
@@ -4452,7 +4491,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 */

@@ -5237,12 +5275,11 @@
 	while (error == 0 && sc->tulip_txmaps_free < TULIP_TXDESCS) {
 	    bus_dmamap_t map;
 	    if ((error = TULIP_TXMAP_CREATE(sc, &map)) == 0)
-		sc->tulip_txmaps[sc->tulip_txmaps_free++] = map;
+		put_tulip_txmaps(sc, map);
 	}
 	if (error) {
 	    while (sc->tulip_txmaps_free > 0) 
-		bus_dmamap_destroy(sc->tulip_dmatag,
-				   sc->tulip_txmaps[--sc->tulip_txmaps_free]);
+		bus_dmamap_destroy(sc->tulip_dmatag, get_tulip_txmaps(sc));
 	}
     }
 #else
>Release-Note:
>Audit-Trail:
State-Changed-From-To: open->feedback
State-Changed-By: pooka@narn.netbsd.org
State-Changed-When: Sat, 19 Jan 2008 19:40:44 +0200
State-Changed-Why:
does using the tlp driver solve the problem?


From: "David A. Holland" <dholland@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/12605 CVS commit: src/sys/dev/pci
Date: Sun, 29 Mar 2009 05:26:44 +0000

 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.

State-Changed-From-To: feedback->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 29 Mar 2009 05:33:34 +0000
State-Changed-Why:
patch merged.


>Unformatted:

Home	PR Database Search