Bug 25445

On Wed, 28 Feb 2001, Peter Philipp wrote:

> Whoops I messed that up somewhat.  Let's try again this time I won't write
> the patches because they are erroneous (and actually go back and break
> alpha while leaving i386 ok, it's a tug-of-war see :P go intel).  

I think you need to reply to freebsd-gnats-submit, not gnats-admin,
so that the reply gets put in the database instead of in an overworked
admin's mailbox.  I quoted it all so that it will be in the database
via this reply.

> 
> I've also read up on the history why we are seeing this bug.  Revision 1.46 
> in sys/malloc.h had a member type changed from long to int64_t.  As far as I 
> know a long for the alpha is 64 bits anyhow but a long for i386 is 32 bits.  
> So it didn't do anything for the alpha in fact but increased the size of 
> ks_calls to 64 bits for the i386.  This created a type size problem for 
> vmstat -m where the type for printing totreq was long which is 32 bits and 
> not 64 bits.  It creates this in a vmstat -m:
> 
> Memory Totals:  In Use    Free    Requests
>                 94474K   1759K    -2126966145
> 
> Perhaps the total requests should be printed as a quad_t which is independent
> of platform and always 64 bits it seems.

I think that is sufficient.  Someone should turn on -Wformat globally to
inhibit new printf format errors.  I hope we fixed most of the old ones.

> 
> Anyhow this seems to open a few cans of worms (shudder) and also goes back to
> size differences in structures such as struct malloc_type where on alpha
> a set of stats collected as type long can hold a lot more than on i386 which
> isn't fair and isn't consistent, which is unfortunately sucky.

This is a fairly large problem.  `long' is the wrong type for all counters,
but `u_long' is not much better (it has only twice the range).  At least
using `long' and printing correctly it gives obviously bogus counts when
counters overflows.  Fixed-size types are not the correct type for
counters unless the range of the counter is known.  They may be too
inefficient.  Atomic updates of 64-bit counters are especially inefficient
on i386's.

> 
> If I'm totally wrong here, I'd love a good explaining, cuz I'll need it.
> 
> -- 
> -                                                                              -
> Peter Philipp <pjp@daemonium.com>
> Daemonium

Ok well since this is still an open pr here is the patch to make usr.bin/vmstat
compile cleanly against -Wformat (yes -Wformat should still be turned on 
globally as Bruce Evans suggests) but would most likely need more fixing than 
this particular PR calls for.  If someone who has an alpha can add temporarily 
-Wformat into the usr.bin/vmstat/Makefile near the CFLAGS+= line and make if
there is no errrors with the formatting then dandy commit/close PR.

--- malloc.h.orig       Tue Feb 27 22:49:28 2001
+++ malloc.h    Sun Apr  1 03:47:41 2001
@@ -56,7 +56,7 @@
        long    ks_limit;       /* most that are allowed to exist */
        long    ks_size;        /* sizes of this thing that are allocated */
        long    ks_inuse;       /* # of packets of this type currently in use */
-       int64_t ks_calls;       /* total packets of this type ever allocated */
+       long    ks_calls;       /* total packets of this type ever allocated */
        long    ks_maxused;     /* maximum number ever used */
        u_long  ks_magic;       /* if it's not magic, don't touch it */
        const char *ks_shortdesc;       /* short description */
@@ -102,7 +102,7 @@
 struct kmembuckets {
        caddr_t kb_next;        /* list of free blocks */
        caddr_t kb_last;        /* last free block */
-       int64_t kb_calls;       /* total calls to allocate this size */
+       long    kb_calls;       /* total calls to allocate this size */
        long    kb_total;       /* total number of blocks allocated */
        long    kb_elmpercl;    /* # of elements in this sized allocation */
        long    kb_totalfree;   /* # of free elements in this bucket */



-- 
-                                                                              -
Peter Philipp <pjp@daemonium.com>
Daemonium

Comment 3 Ed Maste 2013-04-08 14:53:06 UTC

State Changed
From-To: open->closed

Fixed quite some time ago (some time in 2002). 
In any case this functionality has been rewritten since.