misc/120884: bktr driver always stores the contigmalloc'ed memory pointers

[Tobias Mohrlueder]

>Number:         120884
>Category:       misc
>Synopsis:       bktr driver always stores the contigmalloc'ed memory pointers
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Feb 20 14:40:02 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator:     Tobias Mohrlueder
>Release:        RELENG_7
>Organization:
>Environment:
FreeBSD acheron.local.lan 7.0-PRERELEASE FreeBSD 7.0-PRERELEASE #0: Fri Feb  8 19:54:00 CET 2008 tobiasmo at acheron.local.lan:/usr/obj/usr/src/sys/ACHERON  i386
>Description:
While building world on my machine, I experienced a problem with the bktr driver module. One contigmalloc() call failed and as a result, the module was left in a defective state. Any further load attempts were futile, because the bktr memory loader cannot be unloaded (and the driver keeps thinking that there already is enough memory allocated).

>From /var/log/messages:
Jan 15 17:01:13 acheron kernel: bktr0: Unable to allocate 3555328 bytes of memory.

bktr_mem.c's sole purpose is to avoid the situation of having to contigmalloc() memory everytime the bktr module is loaded, so maybe this is a known "problem" and the solution is to load the module at boot time.
>How-To-Repeat:
Although not an exact guide how to repeat the problem;

1) Put the machine under load (e.g. make buildworld)

2) kldload bktr
>Fix:
Load the module at boot time.

Or: Maybe the attached patch helps. It looks like a trivial problem, but I am not a seasoned programmer.

Patch attached with submission follows:

--- /usr/src/sys/dev/bktr/bktr_core.c	2005-12-04 11:06:03.000000000 +0100
+++ bktr_core.c	2008-02-19 17:15:01.000000000 +0100
@@ -528,11 +528,17 @@
 
 /* If this is a module, save the current contiguous memory */
 #if defined(BKTR_FREEBSD_MODULE)
-bktr_store_address(unit, BKTR_MEM_DMA_PROG,     bktr->dma_prog);
-bktr_store_address(unit, BKTR_MEM_ODD_DMA_PROG, bktr->odd_dma_prog);
-bktr_store_address(unit, BKTR_MEM_VBIDATA,      bktr->vbidata);
-bktr_store_address(unit, BKTR_MEM_VBIBUFFER,    bktr->vbibuffer);
-bktr_store_address(unit, BKTR_MEM_BUF,          buf);
+
+	/* Only save when all relevant get_bktr_mem() calls succeeded */
+	if ( bktr->dma_prog != 0 && bktr->odd_dma_prog != 0 &&
+	     bktr->vbidata != 0 && bktr->vbibuffer != 0 &&
+	     ( BROOKTREE_ALLOC ? buf != 0 : 1 ) ) {
+		bktr_store_address(unit, BKTR_MEM_DMA_PROG,     bktr->dma_prog);
+		bktr_store_address(unit, BKTR_MEM_ODD_DMA_PROG, bktr->odd_dma_prog);
+		bktr_store_address(unit, BKTR_MEM_VBIDATA,      bktr->vbidata);
+		bktr_store_address(unit, BKTR_MEM_VBIBUFFER,    bktr->vbibuffer);
+		bktr_store_address(unit, BKTR_MEM_BUF,          buf);
+	}
 #endif
 
 


>Release-Note:
>Audit-Trail:
>Unformatted: