DragonFlyBSD

for easier reviewing, inline:

diff --git a/usr.sbin/iostat/iostat.c b/usr.sbin/iostat/iostat.c
index 5398ce1..af016b7 100644
--- a/usr.sbin/iostat/iostat.c
+++ b/usr.sbin/iostat/iostat.c
@ -467,7 +467,7 @ main(int argc, char **argv)
cp_time_total = 1.0;

if ((dflag  0) || (Tflag > 0))
-            printf("%4.0f%5.0f", diff_tk_nin / cp_time_total * 1e6, 
+            printf("%4.0f%5.0f ", diff_tk_nin / cp_time_total * 1e6, 
 diff_tk_nout / cp_time_total * 1e6);
 devstats(hflag);
 if ((dflag  0) || (Cflag > 0))
@ -491,7 +491,7 @ phdr(__unused int signo)
 int printed;

if ((dflag == 0) || (Tflag > 0))
-        printf("      tty");
+        printf("    tty");
 for (i = 0, printed=0;(i < num_devices) && (printed < maxshowdevs);i++){
 int di;
 if ((dev_select[i].selected != 0)
@ -502,14 +502,14 @ phdr(__unused int signo)
 cur.dinfo->devices[di].device_name,
 cur.dinfo->devices[di].unit_number);
 else
-                printf("%15.6s%d ",
+                printf("%19.6s%d            ",
 cur.dinfo->devices[di].device_name,
 cur.dinfo->devices[di].unit_number);
 printed++;
 }
 }
 if ((dflag  0) || (Cflag > 0))
-        printf("            cpu\n");
+        printf("         cpu\n");
 else
 printf("\n");

@ -526,7 +526,7 @ phdr(_unused int signo)
printf(" blk xfr msps ");
} else {
if (Iflag 0)
- printf(" KB/t tps MB/s ");
+ printf(" KB/t rtps MBr/s wtps MBw/s ");
else
printf(" KB/t xfrs MB ");
}
@ -534,7 +534,7 @ phdr(_unused int signo)
}
}
if ((dflag == 0) || (Cflag > 0))
- printf(" us ni sy in id\n");
+ printf(" us ni sy in id\n");
else
printf("\n");

@ -544,8 +544,11 @ static void
devstats(int perf_select) {
int dn;
+ long double kb_per_transfer;
long double transfers_per_second;
- long double kb_per_transfer, mb_per_second;
+ long double transfers_per_secondr, transfers_per_secondw;
+ long double mb_per_second;
+ long double mb_per_secondr, mb_per_secondw;
u_int64_t total_bytes, total_transfers, total_blocks;
long double busy_seconds;
long double total_mb;
@ -570,9 +573,23 @ devstats(int perf_select)
&last.dinfo->devices[di], busy_seconds,
&total_bytes, &total_transfers,
&total_blocks, &kb_per_transfer,
- &transfers_per_second, &mb_per_second,
+ &transfers_per_second, &mb_per_second,
&blocks_per_second, &ms_per_transaction)!= 0)
errx(1, "%s", devstat_errbuf);
+ if (compute_stats_read(&cur.dinfo->devices[di],
+ &last.dinfo->devices[di], busy_seconds,
+ NULL, NULL,
+ NULL, NULL,
+ &transfers_per_secondr, &mb_per_secondr,
+ NULL, NULL)!= 0)
+ errx(1, "%s", devstat_errbuf);
+ if (compute_stats_write(&cur.dinfo->devices[di],
+ &last.dinfo->devices[di], busy_seconds,
+ NULL, NULL,
+ NULL, NULL,
+ &transfers_per_secondw, &mb_per_secondw,
+ NULL, NULL)!= 0)
+ errx(1, "%s", devstat_errbuf);

if (perf_select != 0) {
 dev_select[dn].bytes = total_bytes;
@ -604,10 +621,12 @ devstats(int perf_select)
 ms_per_transaction);
 } else {
 if (Iflag == 0)
-                printf(" %5.2Lf %4.0Lf %5.2Lf ",
+                printf(" %5.2Lf %4.0Lf %6.2Lf %4.0Lf %6.2Lf  ",
 kb_per_transfer,
-                       transfers_per_second,
-                       mb_per_second);
+                       transfers_per_secondr,
+                       mb_per_secondr,
+                       transfers_per_secondw,
+                       mb_per_secondw);
 else {
 total_mb = total_bytes;
 total_mb /= 1024 * 1024;

:for easier reviewing, inline:
:
:diff --git a/usr.sbin/iostat/iostat.c b/usr.sbin/iostat/iostat.c

For some reason the patch command on that diff file fails on every chunk,
    not sure why.

In anycase, I see where it is headed.  I think we might have an issue
    with too much information reducing the number of devices which can be
    displayed, so I'd like to suggest some other changes:

• Provide options for various modes, with the default being the old
  (KB/t, tps, MB/s) tuple, so the output can remain compact.
  I recommend one that does (tps, MBr/s, MBw/s).

• Maybe we can remove the tty statistics and add aggregate or
  primary interface network statistics. Nobody cares about tty
  statistics any more but it would be kinda cool to see network
  stats there mixed with the disk stats.

I am considering removing the tps/r, tps/w from systat -vm 1 and
    just having tps, but keeping MBs/r and MBs/w.  I think people will
    get more out of an aggregate tps stat.  Then for systat -vm 1
    we would have:  (KB/t, tps, MBs/r, MBs/w) (I should rename those to
    MBr/s and MBw/s I guess too).

-Matt

Matthew Dillon schrieb:

• Provide options for various modes, with the default being the old
  (KB/t, tps, MB/s) tuple, so the output can remain compact.
  I recommend one that does (tps, MBr/s, MBw/s).

Yeah, thought about that, too. Will look into it.

• Maybe we can remove the tty statistics and add aggregate or
  primary interface network statistics. Nobody cares about tty
  statistics any more but it would be kinda cool to see network
  stats there mixed with the disk stats.

Can I get that from devstat also? Well, could just look at systat
-ifstat, right? How to determine the "primary" interface?

If you don't want to see tty (which I fully understand :-) ) you could
just use iostat -dC.

I am considering removing the tps/r, tps/w from systat -vm 1 and
just having tps, but keeping MBs/r and MBs/w. I think people will
get more out of an aggregate tps stat. Then for systat -vm 1
we would have: (KB/t, tps, MBs/r, MBs/w) (I should rename those to
MBr/s and MBw/s I guess too).

Yep, Megabyte-seconds divided by writes (and reads) doesn't make a
horrible lot of sense :-)

Jan

Jan Lentfer wrote:

Matthew Dillon schrieb:

I am considering removing the tps/r, tps/w from systat -vm 1 and
just having tps, but keeping MBs/r and MBs/w. I think people will
get more out of an aggregate tps stat. Then for systat -vm 1
we would have: (KB/t, tps, MBs/r, MBs/w) (I should rename those to
MBr/s and MBw/s I guess too).

Just popped in my head - maybe some kind of format-string type of thing
might make sense here .. e.g:

VMSTAT_IOFMT="%k %t %R %W" -> KB/t tps MBs/r, MBs/w

with the default being set to whatever, and some sort of
'convention' r.e. what upper case vs lower case means
to make it intuitive..

no patch in attachments - sorry guys :)

</2c>

Committed May 5th, 2010