On Thu, Nov 18, 2010 at 15:20 +0100, Mike Belopuhov wrote:
quoted text
> On Thu, Nov 18, 2010 at 14:42 +0100, Mike Belopuhov wrote:
> > On Thu, Nov 18, 2010 at 01:01 -0500, Ted Unangst wrote:
> > > 2010/11/17 Mike Belopuhov <mkb@crypt.org.ru>:
> > > > I think this time around it's kernel that's wrong.  top(1) calls CPTIME2
> > > > when 'ncpus > 1' to get detailed per-cpu information.  CPTIME is only
> > > > valid on UP systems, but can represent an average value on MP without
> > > > having userland programs figure out whether kernel is lying or not.
> > > 
> > > I think the patch below is reasonable, and probably the best idea.
> > > 
> > > But what really puzzles me is why are iostat and friends using CP_TIME
> > > as a clock?  That strikes me as rather weird.
> > > 
> > 
> > they don't.  they're using hz as clock.  cp_time field description in
> > src/usr.bin/vmstat/dkstats.h is a bit misleading.
> 
> funny enough, their calculations are still a bit off: they devide by hz,
> meaning that this CPTIME values are collected in 1/hz intervals, while in
> practice they're collected by the statclock, which is called from hardclock
> only if stathz is zero.  Otherwise they're called 1/stathz times per second,
> where stathz can be 128Hz :)

that's not true.  i've overlooked that all of them (iostat, systat,
vmstat) call a KERN_CLOCKRATE sysctl and prefer stathz, while kernel
does "clkinfo.stathz = stathz ? stathz : hz;".

so the only thing we need to do is make KERN_CPTIME return an average.
diff tested by me and Luis Useche.  OK to commit?

Index: sys/kern/kern_sysctl.c
===================================================================
RCS file: /home/cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.194
diff -u -p -r1.194 kern_sysctl.c
--- sys/kern/kern_sysctl.c	2 Nov 2010 09:36:09 -0000	1.194
+++ sys/kern/kern_sysctl.c	18 Nov 2010 13:50:48 -0000
@@ -469,6 +469,9 @@ kern_sysctl(int *name, u_int namelen, vo
 				cp_time[i] += ci->ci_schedstate.spc_cp_time[i];
 		}
 
+		for (i = 0; i < CPUSTATES; i++)
+			cp_time[i] /= ncpus;
+
 		return (sysctl_rdstruct(oldp, oldlenp, newp, &cp_time,
 		    sizeof(cp_time)));
 	}
Index: lib/libc/gen/sysctl.3
===================================================================
RCS file: /home/cvs/src/lib/libc/gen/sysctl.3,v
retrieving revision 1.200
diff -u -p -r1.200 sysctl.3
--- lib/libc/gen/sysctl.3	11 Oct 2010 08:26:40 -0000	1.200
+++ lib/libc/gen/sysctl.3	18 Nov 2010 14:04:52 -0000
@@ -511,8 +511,7 @@ The console device.
 An array of longs of size
 .Li CPUSTATES
 is returned, containing statistics about the number of ticks spent by
-the system among all processors in interrupt processing,
-user processes
+the system in interrupt processing, user processes
 .Po
 .Xr nice 1
 or normal
Index: usr.bin/systat/vmstat.c
===================================================================
RCS file: /home/cvs/src/usr.bin/systat/vmstat.c,v
retrieving revision 1.72
diff -u -p -r1.72 vmstat.c
--- usr.bin/systat/vmstat.c	27 Oct 2009 23:59:44 -0000	1.72
+++ usr.bin/systat/vmstat.c	18 Nov 2010 13:53:28 -0000
@@ -159,8 +159,6 @@ field_view views_vm[] = {
 	{NULL, NULL, 0, NULL}
 };
 
-int ncpu = 1;
-
 int
 initvmstat(void)
 {
@@ -168,12 +166,6 @@ initvmstat(void)
 	int mib[4], i;
 	size_t size;
 
-	mib[0] = CTL_HW;
-	mib[1] = HW_NCPU;
-	size = sizeof(ncpu);
-	if (sysctl(mib, 2, &ncpu, &size, NULL, 0) < 0)
-		return (-1);
-
 	hertz = stathz ? stathz : hz;
 	if (!dkinit(1))
 		return(0);
@@ -343,7 +335,6 @@ showkre(void)
 	}
 	failcnt = 0;
 	etime /= hertz;
-	etime /= ncpu;
 	inttotal = 0;
 	for (i = 0; i < nintr; i++) {
 		if (s.intrcnt[i] == 0)
Index: usr.bin/vmstat/vmstat.c
===================================================================
RCS file: /home/cvs/src/usr.bin/vmstat/vmstat.c,v
retrieving revision 1.117
diff -u -p -r1.117 vmstat.c
--- usr.bin/vmstat/vmstat.c	1 Aug 2010 02:51:03 -0000	1.117
+++ usr.bin/vmstat/vmstat.c	18 Nov 2010 13:51:27 -0000
@@ -128,8 +128,6 @@ extern char *__progname;
 int verbose = 0;
 int zflag = 0;
 
-int ncpu;
-
 int
 main(int argc, char *argv[])
 {
@@ -211,11 +209,6 @@ main(int argc, char *argv[])
 		}
 	}
 
-	mib[0] = CTL_HW;
-	mib[1] = HW_NCPU;
-	size = sizeof(ncpu);
-	(void) sysctl(mib, 2, &ncpu, &size, NULL, 0);
-
 	if (todo & VMSTAT) {
 		struct winsize winsize;
 
@@ -649,7 +642,6 @@ dkstats(void)
 	if (etime == 0)
 		etime = 1;
 	etime /= hz;
-	etime /= ncpu;
 	for (dn = 0; dn < dk_ndrive; ++dn) {
 		if (!dk_select[dn])
 			continue;