From owner-svn-src-all@FreeBSD.ORG Mon Dec 6 22:07:38 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 638F21065679; Mon, 6 Dec 2010 22:07:38 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id CD6AE8FC32; Mon, 6 Dec 2010 22:07:35 +0000 (UTC) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.4/8.14.4/ALCHEMY.FRANKEN.DE) with ESMTP id oB6M7XeE075943; Mon, 6 Dec 2010 23:07:34 +0100 (CET) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.4/8.14.4/Submit) id oB6M7XEV075942; Mon, 6 Dec 2010 23:07:33 +0100 (CET) (envelope-from marius) Date: Mon, 6 Dec 2010 23:07:33 +0100 From: Marius Strobl To: Alan Cox Message-ID: <20101206220733.GG38282@alchemy.franken.de> 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> <4CF7D711.9040505@rice.edu> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4CF7D711.9040505@rice.edu> User-Agent: Mutt/1.4.2.3i 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: Mon, 06 Dec 2010 22:07:38 -0000 On Thu, Dec 02, 2010 at 11:27:45AM -0600, Alan Cox wrote: > 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); > } With that one the kernel now survies memguard_init() but then panics right afterwards when kmeminit() calls kmem_suballoc(): 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 #18 r215249:216120M: Mon Dec 6 13:27:57 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 3 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 0xc054d2fc (null)() at 0xc0359a50 (null)() at 0xc031e930 (null)() at 0xc0070028 Note that the status is different now though. Marius