From owner-svn-src-all@FreeBSD.ORG Wed Dec 1 18:19:20 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5EA201065694; Wed, 1 Dec 2010 18:19:20 +0000 (UTC) (envelope-from alc@rice.edu) Received: from mh5.mail.rice.edu (mh5.mail.rice.edu [128.42.199.32]) by mx1.freebsd.org (Postfix) with ESMTP id F0A378FC24; Wed, 1 Dec 2010 18:19:19 +0000 (UTC) Received: from mh5.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh5.mail.rice.edu (Postfix) with ESMTP id 3806D28F768; Wed, 1 Dec 2010 12:19:19 -0600 (CST) X-Virus-Scanned: by amavis-2.6.4 at mh5.mail.rice.edu, auth channel Received: from mh5.mail.rice.edu ([127.0.0.1]) by mh5.mail.rice.edu (mh5.mail.rice.edu [127.0.0.1]) (amavis, port 10026) with ESMTP id XjDwn5xtIUlb; Wed, 1 Dec 2010 12:19:19 -0600 (CST) Received: from adsl-216-63-78-18.dsl.hstntx.swbell.net (adsl-216-63-78-18.dsl.hstntx.swbell.net [216.63.78.18]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (No client certificate requested) (Authenticated sender: alc) by mh5.mail.rice.edu (Postfix) with ESMTPSA id 2E13928F778; Wed, 1 Dec 2010 12:19:18 -0600 (CST) Message-ID: <4CF691A5.8070608@rice.edu> Date: Wed, 01 Dec 2010 12:19:17 -0600 From: Alan Cox User-Agent: Thunderbird 2.0.0.24 (X11/20100725) MIME-Version: 1.0 To: Marius Strobl References: <201011281926.oASJQKiE040689@svn.freebsd.org> <20101128194542.GF9966@alchemy.franken.de> <20101129192308.GX80343@alchemy.franken.de> <20101129192417.GA18893@alchemy.franken.de> In-Reply-To: <20101129192417.GA18893@alchemy.franken.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, alc@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Max Khon Subject: Re: svn commit: r216016 - head/sys/sparc64/include X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Dec 2010 18:19:20 -0000 Marius Strobl wrote: > On Mon, Nov 29, 2010 at 08:23:08PM +0100, Marius Strobl wrote: > >> On Tue, Nov 30, 2010 at 12:31:31AM +0600, Max Khon wrote: >> >>> Marius, >>> >>> On Mon, Nov 29, 2010 at 1:45 AM, Marius Strobl wrote: >>> >>> On Sun, Nov 28, 2010 at 07:26:20PM +0000, Max Khon wrote: >>> >>>>> Author: fjoe >>>>> Date: Sun Nov 28 19:26:20 2010 >>>>> New Revision: 216016 >>>>> URL: http://svn.freebsd.org/changeset/base/216016 >>>>> >>>>> Log: >>>>> Define VM_KMEM_SIZE_MAX on sparc64. Otherwise kernel built with >>>>> DEBUG_MEMGUARD panics early in kmeminit() with the message >>>>> "kmem_suballoc: bad status return of 1" because of zero "size" argument >>>>> passed to kmem_suballoc() due to "vm_kmem_size_max" being zero. >>>>> >>>>> The problem also exists on ia64. >>>>> >>>>> Modified: >>>>> head/sys/sparc64/include/vmparam.h >>>>> >>>>> Modified: head/sys/sparc64/include/vmparam.h >>>>> >>>>> >>>> ============================================================================== >>>> >>>>> --- head/sys/sparc64/include/vmparam.h Sun Nov 28 18:59:52 2010 >>>>> >>>> (r216015) >>>> >>>>> +++ head/sys/sparc64/include/vmparam.h Sun Nov 28 19:26:20 2010 >>>>> >>>> (r216016) >>>> >>>>> @@ -237,6 +237,14 @@ >>>>> #endif >>>>> >>>>> /* >>>>> + * Ceiling on amount of kmem_map kva space. >>>>> + */ >>>>> +#ifndef VM_KMEM_SIZE_MAX >>>>> +#define VM_KMEM_SIZE_MAX ((VM_MAX_KERNEL_ADDRESS - \ >>>>> + VM_MIN_KERNEL_ADDRESS + 1) * 3 / 5) >>>>> +#endif >>>>> + >>>>> +/* >>>>> * Initial pagein size of beginning of executable file. >>>>> */ >>>>> #ifndef VM_INITIAL_PAGEIN >>>>> >>>> How was that value determined? >>>> >>>> >>> I've just copied it from amd64 to be non-zero for now. Do you have a better >>> idea of what it should look like? >>> >>> >> Well, on sparc64 VM_MAX_KERNEL_ADDRESS already is dynamically adjusted >> to the maximum appropriate for the specific CPU during the early cycles >> of the kernel so I'd think one could just use VM_MAX_KERNEL_ADDRESS - >> VM_MIN_KERNEL_ADDRESS for VM_KMEM_SIZE_MAX there, I'm not sure what >> the intention of the ceiling provided by that macro actually is though >> In any case, the commit message of r180210 which changed the amd64 >> version to the current one talks about limiting the kmem map to 3.6GB >> and while it also fails to explain where that value comes from it >> looks rather amd64 specific and the formula used by the macro will >> result in a different ceiling on sparc64 and thus inappropriate. I've >> CC'ed alc@ who hopefully can shed some light on this. >> Apart from this the actual bug here seems to be that memguard_fudge() >> can't deal with a km_max being zero or that zero is passed to it as >> kmeminit() allows for VM_KMEM_SIZE_MAX not being defined. >> >> > > Oops, forgot to actually CC alc@. > There's nothing particularly amd64-specific about the definition. In general, if you allow the kmem_map, which is basically the kernel's heap, to consume the entire kernel address space as you propose, then you're leaving no room for the buffer cache, thread stacks, pipes, and a few other things. Since the cap on the kmem_map size as defined by r180210 is a fraction of the overall kernel address space size, it scales automatically with the kernel address space size and should be a reasonable cap definition for most architectures. In the specific case of sparc64, I think it's fair to say that the kernel virtual address is sufficiently large and the amount of physical memory in any of the supported machines is small enough in comparison that it hasn't mattered that a kmem_map cap doesn't exist, because most of the aforementioned structures are scaled based on the amount of physical memory. In fact, it probably won't matter any time soon. All of that said, I would suggest fixing memguard_fudge() and reverting r216016 and the follow up change. All I think that is required to fix memguard_fudge() is Index: vm/memguard.c =================================================================== --- vm/memguard.c (revision 216070) +++ vm/memguard.c (working copy) @@ -186,7 +186,7 @@ memguard_fudge(unsigned long km_size, unsigned lon memguard_mapsize = round_page(memguard_mapsize); if (memguard_mapsize / (2 * PAGE_SIZE) > mem_pgs) memguard_mapsize = mem_pgs * 2 * PAGE_SIZE; - if (km_size + memguard_mapsize > km_max) + if (km_max > 0 && km_size + memguard_mapsize > km_max) return (km_max); return (km_size + memguard_mapsize); } Regards, Alan