svn commit: r216016 - head/sys/sparc64/include

Thu Dec 2 16:47:30 UTC 2010

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 
> >>><marius at alchemy.franken.de>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 at .
> >  
> 
> 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 at 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

Marius