Re: Extend "struct malloc_type" for extended KMEMSTATS?

[Havard Eidnes <he%NetBSD.org@localhost>]

Hi,

it's time to pick up an old issue I discussed here earlier.

As described earlier, I'd like to extend vmstat to show the
number of allocations done per type and size, like so:

Memory statistics by type                                Type  Kern
           Type InUse  MemUse HighUse   Limit   Requests Limit Limit Size(s)
   kernfs mount     1      1K      1K  78644K          1     0     0 16:1
    ptyfs mount     1      1K      1K  78644K          1     0     0 16:1
     prop array     1      1K      1K  78644K          1     0     0 64:1
prop dictionary   145     19K     19K  78644K        145     0     0 128:145
    prop string   143      3K      3K  78644K        143     0     0 16:143
           acpi    30      1K      1K  78644K         31     0     0 
16:7,32:19,64:4
            USB    46      5K      5K  78644K         52     0     0 
16:6,32:6,64:19,128:9,256:6,1024:0
     USB device    18     19K     19K  78644K         18     0     0 
16:6,128:3,2048:9

These stats are only available under KMEMSTATS, but since struct
malloc_type is part of what vmstat uses, it would not be friendly
to require a new vmstat just because you turned on KMEMSTATS in
the kernel.  This means that this change will currently consume
around 5-6KB extra kernel memory.  All the counting in the kernel
is however done under KMEMSTATS ifdefs.

While it is apparently true that malloc(9) over time is being
replaced by kmem(9), there remains a large number of uses of
malloc(9) in the kernel, and they are unlikely to go away anytime
soon.

This change is the first stage of a two-stage set of changes
which would make it easier to spot and trace kernel memory leaks
in use of malloc(9), and was of much help finding the kernel
memory leak documented in PRs 39145 and 42661.

The new diff is attached below.  This time I've refrained from
the re-typing of the preexisting fields in "struct malloc_type",
to keep the change as small as possible.

It would appear that in addition to this change, a kernel
revision bump would be required.

Comments?

Regards,

- Håvard

Index: sys/sys/mallocvar.h
===================================================================
RCS file: /cvsroot/src/sys/sys/mallocvar.h,v
retrieving revision 1.7
diff -u -p -r1.7 mallocvar.h
--- sys/sys/mallocvar.h 7 Nov 2007 16:12:25 -0000       1.7
+++ sys/sys/mallocvar.h 31 Mar 2010 12:52:13 -0000
@@ -56,7 +56,7 @@ struct malloc_type {
        u_long  ks_maxused;     /* maximum number ever used */
        u_long  ks_limit;       /* most that are allowed to exist */
        u_long  ks_size;        /* sizes of this thing that are allocated */
-       u_long  ks_spare;
+       u_int   ks_active[32];  /* number of active allocations per size */
 };
 
 #ifdef _KERNEL
Index: sys/kern/kern_malloc.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_malloc.c,v
retrieving revision 1.128
diff -u -p -r1.128 kern_malloc.c
--- sys/kern/kern_malloc.c      22 Jan 2010 08:32:05 -0000      1.128
+++ sys/kern/kern_malloc.c      31 Mar 2010 12:52:13 -0000
@@ -371,6 +371,7 @@ kern_malloc(unsigned long size, struct m
                        &malloc_lock);
        }
        ksp->ks_size |= 1 << indx;
+       ksp->ks_active[indx]++;
 #endif
 #ifdef DIAGNOSTIC
        copysize = 1 << indx < MAX_COPY ? 1 << indx : MAX_COPY;
@@ -604,6 +605,7 @@ kern_free(void *addr, struct malloc_type
 #ifdef KMEMSTATS
                size = kup->ku_pagecnt << PGSHIFT;
                ksp->ks_memuse -= size;
+               ksp->ks_active[kup->ku_indx]--;
                kup->ku_indx = 0;
                kup->ku_pagecnt = 0;
                if (ksp->ks_memuse + size >= ksp->ks_limit &&
@@ -660,6 +662,7 @@ kern_free(void *addr, struct malloc_type
        }
        kbp->kb_totalfree++;
        ksp->ks_memuse -= size;
+       ksp->ks_active[kup->ku_indx]--;
        if (ksp->ks_memuse + size >= ksp->ks_limit &&
            ksp->ks_memuse < ksp->ks_limit)
                wakeup((void *)ksp);
Index: usr.bin/vmstat/vmstat.c
===================================================================
RCS file: /cvsroot/src/usr.bin/vmstat/vmstat.c,v
retrieving revision 1.166
diff -u -p -r1.166 vmstat.c
--- usr.bin/vmstat/vmstat.c     21 Oct 2009 21:12:07 -0000      1.166
+++ usr.bin/vmstat/vmstat.c     31 Mar 2010 12:52:13 -0000
@@ -1172,7 +1172,10 @@ domem(void)
                    howmany(ks.ks_limit, KILO), ks.ks_calls,
                    ks.ks_limblocks, ks.ks_mapblocks);
                first = 1;
-               for (j =  1 << MINBUCKET; j < 1 << (MINBUCKET + 16); j <<= 1) {
+               for (j = 1 << MINBUCKET, i = MINBUCKET;
+                    j < 1 << (MINBUCKET + 16);
+                    j <<= 1, i++)
+               {
                        if ((ks.ks_size & j) == 0)
                                continue;
                        if (first)
@@ -1180,6 +1183,7 @@ domem(void)
                        else
                                (void)printf(",%d", j);
                        first = 0;
+                       (void)printf(":%u", ks.ks_active[i]);
                }
                (void)printf("\n");
                totuse += ks.ks_memuse;