Bug #1698

iostat patch: Add dillon's systat changes to iostat

Added by lentferj over 4 years ago. Updated about 4 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:-
Target version:-

Description

Please review the attached patch.

Jan

begin 644 iostat.diff
M9&EF9B`M+6=I="!A+W5S<BYS8FEN+VEO<W1A="]I;W-T870N8R!B+W5S<BYS
M8FEN+VEO<W1A="]I;W-T870N8PII;F1E>"`U,SDX8V4Q+BYA9C`Q-F(W(#$P
M,#8T-`HM+2T@82]U<W(N<V)I;B]I;W-T870O:6]S=&%T+F,**RLK(&(O=7-R
M+G-B:6XO:6]S=&%T+VEO<W1A="YC"D!`("TT-C<L-R`K-#8W+#<@0$`@;6%I
M;BAI;G0@87)G8RP@8VAA<B`J*F%R9W8I"B`)"0EC<%]T:6UE7W1O=&%L(#T@
M,2XP.PH@"B`)"6EF("@H9&9L86<@/3T@,"D@?'P@*%1F;&%G(#X@,"DI"BT)
M"0EP<FEN=&8H(B4T+C!F)34N,&8B+"!D:69F7W1K7VYI;B`O(&-P7W1I;65?
M=&]T86P@*B`Q938L(`HK"0D)<')I;G1F*"(E-"XP9B4U+C!F("(L(&1I9F9?
M=&M?;FEN("\@8W!?=&EM95]T;W1A;"`J(#%E-BP@"B`)"0D)9&EF9E]T:U]N
M;W5T("\@8W!?=&EM95]T;W1A;"`J(#%E-BD["B`)"61E=G-T871S*&AF;&%G
M*3L*(`D):68@*"AD9FQA9R`]/2`P*2!\?"`H0V9L86<@/B`P*2D*0$`@+30Y
M,2PW("LT.3$L-R!`0"!P:&1R*%]?=6YU<V5D(&EN="!S:6=N;RD*(`EI;G0@
M<')I;G1E9#L*(`H@"6EF("@H9&9L86<@/3T@,"D@?'P@*%1F;&%G(#X@,"DI
M"BT)"7!R:6YT9B@B("`@("`@='1Y(BD["BL)"7!R:6YT9B@B("`@('1T>2(I
M.PH@"69O<B`H:2`](#`L('!R:6YT960],#LH:2`\(&YU;5]D979I8V5S*2`F
M)B`H<')I;G1E9"`\(&UA>'-H;W=D979S*3MI*RLI>PH@"0EI;G0@9&D["B`)
M"6EF("@H9&5V7W-E;&5C=%MI72YS96QE8W1E9"`A/2`P*0I`0"`M-3`R+#$T
M("LU,#(L,30@0$`@<&AD<BA?7W5N=7-E9"!I;G0@<VEG;F\I"B`)"0D)"2`@
M("!C=7(N9&EN9F\M/F1E=FEC97-;9&E=+F1E=FEC95]N86UE+`H@"0D)"0D@
M("`@8W5R+F1I;F9O+3YD979I8V5S6V1I72YU;FET7VYU;6)E<BD["B`)"0EE
M;'-E"BT)"0D)<')I;G1F*"(E,34N-G,E9"`B+`HK"0D)"7!R:6YT9B@B)3$Y
M+C9S)60@("`@("`@("`@("`B+`H@"0D)"0D@("`@8W5R+F1I;F9O+3YD979I
M8V5S6V1I72YD979I8V5?;F%M92P*(`D)"0D)("`@(&-U<BYD:6YF;RT^9&5V
M:6-E<UMD:5TN=6YI=%]N=6UB97(I.PH@"0D)<')I;G1E9"LK.PH@"0E]"B`)
M?0H@"6EF("@H9&9L86<@/3T@,"D@?'P@*$-F;&%G(#X@,"DI"BT)"7!R:6YT
M9B@B("`@("`@("`@("`@8W!U7&XB*3L**PD)<')I;G1F*"(@("`@("`@("!C
M<'5<;B(I.PH@"65L<V4*(`D)<')I;G1F*")<;B(I.PH@"D!`("TU,C8L-R`K
M-3(V+#<@0$`@<&AD<BA?7W5N=7-E9"!I;G0@<VEG;F\I"B`)"0D)"7!R:6YT
M9B@B(&)L:R!X9G(@;7-P<R`B*3L*(`D)"7T@96QS92!["B`)"0D):68@*$EF
M;&%G(#T](#`I"BT)"0D)"7!R:6YT9B@B("!+0B]T('1P<R`@($U"+W,@(BD[
M"BL)"0D)"7!R:6YT9B@B("`@2T(O="!R='!S("!-0G(O<R!W='!S("!-0G<O
M<R`B*3L*(`D)"0EE;'-E"B`)"0D)"7!R:6YT9B@B("!+0B]T('AF<G,@("!-
M0B`B*3L*(`D)"7T*0$`@+34S-"PW("LU,S0L-R!`0"!P:&1R*%]?=6YU<V5D
M(&EN="!S:6=N;RD*(`D)?0H@"7T*(`EI9B`H*&1F;&%G(#T](#`I('Q\("A#
M9FQA9R`^(#`I*0HM"0EP<FEN=&8H(B!U<R!N:2!S>2!I;B!I9%QN(BD["BL)
M"7!R:6YT9B@B("!U<R!N:2!S>2!I;B!I9%QN(BD["B`)96QS90H@"0EP<FEN
M=&8H(EQN(BD["B`*0$`@+34T-"PX("LU-#0L,3$@0$`@<W1A=&EC('9O:60*
M(&1E=G-T871S*&EN="!P97)F7W-E;&5C="D*('L*(`EI;G0@9&X["BL);&]N
M9R!D;W5B;&4@:V)?<&5R7W1R86YS9F5R.PH@"6QO;F<@9&]U8FQE('1R86YS
M9F5R<U]P97)?<V5C;VYD.PHM"6QO;F<@9&]U8FQE(&MB7W!E<E]T<F%N<V9E
M<BP@;6)?<&5R7W-E8V]N9#L**PEL;VYG(&1O=6)L92!T<F%N<V9E<G-?<&5R
M7W-E8V]N9'(L('1R86YS9F5R<U]P97)?<V5C;VYD=SL**PEL;VYG(&1O=6)L
M92!M8E]P97)?<V5C;VYD.PHK"6QO;F<@9&]U8FQE(&UB7W!E<E]S96-O;F1R
M+"!M8E]P97)?<V5C;VYD=SL*(`EU7VEN=#8T7W0@=&]T86Q?8GET97,L('1O
M=&%L7W1R86YS9F5R<RP@=&]T86Q?8FQO8VMS.PH@"6QO;F<@9&]U8FQE(&)U
M<WE?<V5C;VYD<SL*(`EL;VYG(&1O=6)L92!T;W1A;%]M8CL*0$`@+34W,"PY
M("LU-S,L,C,@0$`@9&5V<W1A=',H:6YT('!E<F9?<V5L96-T*0H@"0D)"2`@
M)FQA<W0N9&EN9F\M/F1E=FEC97-;9&E=+"!B=7-Y7W-E8V]N9',L"B`)"0D)
M("`F=&]T86Q?8GET97,L("9T;W1A;%]T<F%N<V9E<G,L"B`)"0D)("`F=&]T
M86Q?8FQO8VMS+"`F:V)?<&5R7W1R86YS9F5R+`HM"0D)"2`@)G1R86YS9F5R
M<U]P97)?<V5C;VYD+"`F;6)?<&5R7W-E8V]N9"P@"BL)"0D)("`F=')A;G-F
M97)S7W!E<E]S96-O;F0L("9M8E]P97)?<V5C;VYD+`H@"0D)"2`@)F)L;V-K
M<U]P97)?<V5C;VYD+"`F;7-?<&5R7W1R86YS86-T:6]N*2$](#`I"B`)"0EE
M<G)X*#$L("(E<R(L(&1E=G-T871?97)R8G5F*3L**PD):68@*&-O;7!U=&5?
M<W1A='-?<F5A9"@F8W5R+F1I;F9O+3YD979I8V5S6V1I72P**PD)"0DF;&%S
M="YD:6YF;RT^9&5V:6-E<UMD:5TL(&)U<WE?<V5C;VYD<RP**PD)"0E.54Q,
M+"!.54Q,+`HK"0D)"4Y53$PL($Y53$PL"BL)"0D))G1R86YS9F5R<U]P97)?
M<V5C;VYD<BP@)FUB7W!E<E]S96-O;F1R+`HK"0D)"4Y53$PL($Y53$PI(3T@
M,"D**PD)"65R<G@H,2P@(B5S(BP@9&5V<W1A=%]E<G)B=68I.PHK"0EI9B`H
M8V]M<'5T95]S=&%T<U]W<FET92@F8W5R+F1I;F9O+3YD979I8V5S6V1I72P*
M*PD)"0DF;&%S="YD:6YF;RT^9&5V:6-E<UMD:5TL(&)U<WE?<V5C;VYD<RP*
M*PD)"0E.54Q,+"!.54Q,+`HK"0D)"4Y53$PL($Y53$PL"BL)"0D))G1R86YS
M9F5R<U]P97)?<V5C;VYD=RP@)FUB7W!E<E]S96-O;F1W+`HK"0D)"4Y53$PL
M($Y53$PI(3T@,"D**PD)"65R<G@H,2P@(B5S(BP@9&5V<W1A=%]E<G)B=68I
M.PH@"B`)"6EF("AP97)F7W-E;&5C="`A/2`P*2!["B`)"0ED979?<V5L96-T
M6V1N72YB>71E<R`]('1O=&%L7V)Y=&5S.PI`0"`M-C`T+#$P("LV,C$L,3(@
M0$`@9&5V<W1A=',H:6YT('!E<F9?<V5L96-T*0H@"0D)"2`@("`@("!M<U]P
M97)?=')A;G-A8W1I;VXI.PH@"0E](&5L<V4@>PH@"0D):68@*$EF;&%G(#T]
M(#`I"BT)"0D)<')I;G1F*"(@)34N,DQF("4T+C!,9B`E-2XR3&8@(BP**PD)
M"0EP<FEN=&8H(B`E-2XR3&8@)30N,$QF("4V+C),9B`E-"XP3&8@)38N,DQF
M("`B+`H@"0D)"2`@("`@("!K8E]P97)?=')A;G-F97(L"BT)"0D)("`@("`@
M('1R86YS9F5R<U]P97)?<V5C;VYD+`HM"0D)"2`@("`@("!M8E]P97)?<V5C
M;VYD*3L**PD)"0D@("`@("`@=')A;G-F97)S7W!E<E]S96-O;F1R+`HK"0D)
M"2`@("`@("!M8E]P97)?<V5C;VYD<BP**PD)"0D@("`@("`@=')A;G-F97)S
M7W!E<E]S96-O;F1W+`HK"0D)"2`@("`@("!M8E]P97)?<V5C;VYD=RD["B`)
M"0EE;'-E('L*(`D)"0ET;W1A;%]M8B`]('1O=&%L7V)Y=&5S.PH@"0D)"71O
7=&%L7VUB("\](#$P,C0@*B`Q,#(T.PH`
`
end

History

#1 Updated by lentferj over 4 years ago

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;

#2 Updated by dillon over 4 years ago

: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

#3 Updated by lentferj over 4 years ago

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

#4 Updated by c.turner over 4 years ago

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>

#5 Updated by lentferj about 4 years ago

Committed May 5th, 2010

Also available in: Atom PDF