Hello tech@, I'd appreciate it if someone could review both this patch and my analysis.
PROBLEM ------- When running systat, the vmstat page shows inaccurate numbers for disk i/o speed (likely only on multiprocessor systems) when compared to both the iostat page, as well as the statistics reported by dd. To test this, open two xterms. Run "systat" in one, and "systat iostat" in the other. Then run something like, "dd if=/dev/zero of=zero.deleteme bs=64k" (or anything that will generate a lot of disk i/o). Observe that "systat" shows a different number for the reported i/o speed et. al., whereas "systat iostat" and pressing ^T while dd is running both return numbers in agreement with each other. On my 4-core system, the numbers returned by "systat" for "seeks", "xfers", and "speed" all appear to be exactly 4-times lower than are indicated in "systat iostat" as well as the transfer rate reported by dd. TENTATIVE ANALYSIS __________________ (I am not an expert on OpenBSD's internals, and did not dig all the way into the depths of the kernel, so some of this analysis is based on conjecture). In /usr/src/usr.bin/systat/vmstat.c on line 349, (via some printf() debugging) the variable "etime" appears to be set to the sum total of time elapsed on ALL CPUs in the system. etime = 0; for (i = 0; i < CPUSTATES; i++) { X(cpustats.cs_time); etime += s.cpustats.cs_time[i]; } This causes an error in calculations later on when it is used to calculate disk i/o statistics in vmstat.c (but not in iostat). vmstat.c: /* # of K transferred */ words = (cur.dk_rbytes[dn] + cur.dk_wbytes[dn]) / 1024.0; putint((int)((float)cur.dk_seek[dn]/etime+0.5), DISKROW + 1, c, 5); putint((int)((float)(cur.dk_rxfer[dn] + cur.dk_wxfer[dn])/etime+0.5), DISKROW + 2, c, 5); putintmk((int)(words/etime + 0.5), DISKROW + 3, c, 5); putfloat(atime/etime, DISKROW + 4, c, 5, 1, 1); Compare the above to iostat.c, where etime = naptime; is set on line 143 before showtotal() prints the values. for (dn = 0; dn < cur.dk_ndrive; dn++) { rsum += cur.dk_rbytes[dn] / etime; wsum += cur.dk_wbytes[dn] / etime; rtsum += cur.dk_rxfer[dn] / etime; wtsum += cur.dk_wxfer[dn] / etime; mssum += ATIME(cur.dk_time, dn) / etime; } print_fld_str(FLD_IO_DEVICE, "Totals"); print_fld_size(FLD_IO_READ, rsum); print_fld_size(FLD_IO_WRITE, wsum); print_fld_size(FLD_IO_RTPS, rtsum); print_fld_size(FLD_IO_WTPS, wtsum); print_fld_float(FLD_IO_SEC, mssum, 1); SOLUTION ________ Set "etime = naptime;" in vmstat.c as well, as indicated in the attached patch. ADDENDUM ________ I'm curious why iostat.c uses cur.dk_*[dn] directly whereas vmstat.c adds 0.5 to it. rsum += cur.dk_rbytes[dn] / etime; vs. words = (cur.dk_rbytes[dn] + cur.dk_wbytes[dn]) / 1024.0; putintmk((int)(words/etime + 0.5), DISKROW + 3, c, 5); ^^^^^^ Also, there are several casts in lines like putint((int)((float)cur.dk_seek[dn]/etime+0.5), DISKROW + 1, c, 5) I understand why the value would need to be cast to an (int), since putint() is expecting an int, but since etime is already a (double), wouldn't the C language's automatic type-promotion rules automatically promote everything to a (double) anyway? Casting to a (float) would only seem to reduce precision, and then since it's immediately being cast to an (int) anyway, is the (float) cast really necessary? Removing it seems to cause no immediately noticeable ill effects on my system. I would appreciate any advice or explanation anyone can give regarding the above. Thank you. -- Bryan
Index: vmstat.c =================================================================== RCS file: /cvs/src/usr.bin/systat/vmstat.c,v retrieving revision 1.89 diff -u -r1.89 vmstat.c --- vmstat.c 17 Nov 2018 23:10:08 -0000 1.89 +++ vmstat.c 19 Jan 2019 11:37:33 -0000 @@ -678,6 +678,8 @@ { double words, atime; + etime = naptime; + c += DISKCOL; /* time busy in disk activity */