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

Marius Strobl marius at alchemy.franken.de
Mon Dec 6 22:07:38 UTC 2010


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 
> >>>>><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
> >
> >  
> 
> 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 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 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



More information about the svn-src-all mailing list