kern/162741: [PATCH] vm_kmem_size miscalculated due to int type overflow sometimes

[Adam McDougall]

>Number:         162741
>Category:       kern
>Synopsis:       [PATCH] vm_kmem_size miscalculated due to int type overflow sometimes
>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:   Mon Nov 21 22:30:10 UTC 2011
>Closed-Date:
>Last-Modified:
>Originator:     Adam McDougall
>Release:        8 and 9
>Organization:
>Environment:
FreeBSD ike2.egr.msu.edu 9.0-RC1 FreeBSD 9.0-RC1 #3: Mon Nov 21 12:29:54 EST 2011     root at ike2:/usr/obj/usr/src/sys/AMD64-9  amd64

>Description:
Advancements in filesystems in the last few years in FreeBSD have greatly increased the need to have a large vm.kmem_size.  I've been raising mine to improve stability and performance and I've been using a rule of thumb of setting the kmem_size to be twice the amount of physical ram, or a little above 2x.  As equipment has improved over the years, and FreeBSD has allowed a vm.kmem_size to exceed 2G, I've run into issues with increasing vm.kmem_size because the number I provided in loader.conf would sometimes get overridden with a much smaller number.  This problem became woefully evident on computers with considerably more than 4G of ram.  I had a local hack in place for years but finally got some help tracking down the true problem which was the following check in sys/kern/kern_malloc.c:

         * to something sane. Be careful to not overflow the 32bit
         * ints while doing the check.
         */
        if (((vm_kmem_size / 2) / PAGE_SIZE) > cnt.v_page_count)
                vm_kmem_size = 2 * cnt.v_page_count * PAGE_SIZE;

The problem was cnt.v_page_count being defined as an u_int while vm_kmem_size is a u_long and the math would overflow if the vm.kmem_size I provided in loader.conf was over 2x as large as physmem.  The warning in the comment applies to the adjustment and not just the check.  Solutions include substituting cnt.v_page_count with mem_size or ((u_long) cnt.v_page_count).  mem_size is defined earlier in the file as a u_long with the same value as cnt.v_page_count.  The same fix is needed in 8.x (and probably earlier) but the line numbers are different.  I would really appreciate it if this or an equivalent fix can be committed so I don't need to remember to apply any patches at all to my source trees before building.
>How-To-Repeat:
Set vm.kmem_size="17G" on a computer with 8G of ram and check the sysctl after boot to see that it has reduced it to 3.5G or something much smaller than 2x physmem as the check claims to do.  Setting vm.kmem_size to anything larger than 2x physmem should reproduce this issue I believe.

>Fix:


Patch attached with submission follows:

--- sys/kern/kern_malloc.c.orig	2011-11-21 12:19:25.712591472 -0500
+++ sys/kern/kern_malloc.c	2011-11-21 17:25:11.831042640 -0500
@@ -704,10 +704,10 @@
 	 * Limit kmem virtual size to twice the physical memory.
 	 * This allows for kmem map sparseness, but limits the size
 	 * to something sane. Be careful to not overflow the 32bit
-	 * ints while doing the check.
+	 * ints while doing the check or the adjustment.
 	 */
 	if (((vm_kmem_size / 2) / PAGE_SIZE) > cnt.v_page_count)
-		vm_kmem_size = 2 * cnt.v_page_count * PAGE_SIZE;
+		vm_kmem_size = 2 * mem_size * PAGE_SIZE;
 
 #ifdef DEBUG_MEMGUARD
 	tmp = memguard_fudge(vm_kmem_size, vm_kmem_size_max);


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