From owner-svn-src-head@FreeBSD.ORG Thu Dec 2 17:27:48 2010 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 79BF7106564A; Thu, 2 Dec 2010 17:27:48 +0000 (UTC) (envelope-from alc@rice.edu) Received: from mh3.mail.rice.edu (mh3.mail.rice.edu [128.42.199.10]) by mx1.freebsd.org (Postfix) with ESMTP id 3D67C8FC15; Thu, 2 Dec 2010 17:27:47 +0000 (UTC) Received: from mh3.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh3.mail.rice.edu (Postfix) with ESMTP id 514E128F6FE; Thu, 2 Dec 2010 11:27:47 -0600 (CST) X-Virus-Scanned: by amavis-2.6.4 at mh3.mail.rice.edu, auth channel Received: from mh3.mail.rice.edu ([127.0.0.1]) by mh3.mail.rice.edu (mh3.mail.rice.edu [127.0.0.1]) (amavis, port 10026) with ESMTP id zg76JzpJDaBT; Thu, 2 Dec 2010 11:27:47 -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 mh3.mail.rice.edu (Postfix) with ESMTPSA id 7BD4F28F6FA; Thu, 2 Dec 2010 11:27:46 -0600 (CST) Message-ID: <4CF7D711.9040505@rice.edu> Date: Thu, 02 Dec 2010 11:27:45 -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> <4CF691A5.8070608@rice.edu> <20101202164727.GB38282@alchemy.franken.de> In-Reply-To: <20101202164727.GB38282@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-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Dec 2010 17:27:48 -0000 Marius Strobl wrote: > On Wed, Dec 01, 2010 at 12:19:17PM -0600, Alan Cox wrote: > >> 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); >> } >> >> > > Thanks but unfortunately this variant then still panics in kmem_suballoc() > when called by memguard_init(): > KDB: debugger backends: ddb > KDB: current backend: ddb > Copyright (c) 1992-2010 The FreeBSD Project. > Copyright (c) 1979, 1980, 1983, 1986, 1988, 1989, 1991, 1992, 1993, 1994 > The Regents of the University of California. All rights reserved. > FreeBSD is a registered trademark of The FreeBSD Foundation. > FreeBSD 9.0-CURRENT #17 r215249:216120M: Thu Dec 2 15:17:35 CET 2010 > marius@v20z.zeist.de:/home/marius/co/build/head2/sparc64.sparc64/usr/home/m4 > WARNING: WITNESS option enabled, expect reduced performance. > panic: kmem_suballoc: bad status return of 1 > cpuid = 0 > KDB: enter: panic > [ thread pid 0 tid 0 ] > Stopped at 0xc03b04c0: ta %xcc, 1 > db> bt > Tracing pid 0 tid 0 td 0xc089ca10 > (null)() at 0xc0371bac > (null)() at 0xc054d2dc > (null)() at 0xc0547dc8 > (null)() at 0xc0359a78 > (null)() at 0xc031e930 > (null)() at 0xc0070028 > > Sorry, I overlooked another use of km_max in memguard_fudge(). Please try this instead: Index: vm/memguard.c =================================================================== --- vm/memguard.c (revision 216070) +++ vm/memguard.c (working copy) @@ -184,9 +184,10 @@ memguard_fudge(unsigned long km_size, unsigned lon memguard_mapsize = km_max / vm_memguard_divisor; /* size must be multiple of PAGE_SIZE */ memguard_mapsize = round_page(memguard_mapsize); - if (memguard_mapsize / (2 * PAGE_SIZE) > mem_pgs) + if (memguard_mapsize == 0 || + 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); }