Re: Extend "struct malloc_type" for extended KMEMSTATS?

[christos%astron.com@localhost (Christos Zoulas)]

In article <20100331.151226.59481996.he%uninett.no@localhost>,
Havard Eidnes  <he%NetBSD.org@localhost> wrote:
>Comments?

From my reading of the code ku_indx ranges from MINBUCKET (5 or 4
depending) on _LP64 to MINBUCKET + 15. I would #define MAXBUCKET
to be 15 in sys/param.h use MAXBUCKET instead of 15 in the ?: code
to find BUCKETINDX(), and then declare ks_active as u_short
ks_active[MAXBUCKET + 1] (do we ever see more than 65K active
allocations?); and change the code to ksp->ks_active[kup->ku_indx
- MINBUCKET]{--,++}. This would save ~100 bytes per struct malloc_type.

Also perhaps add checks for underflow:
#if DIAGNOSTIC
        if (ksp->ks_active[kup->ku_indx - MINBUCKET] == 0)
                panic("...");
#endif
        ksp->ks_active[kup->ku_indx - MINBUCKET]--;

and overflow when DIAGNOSTIC.

christos

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