Report I/O stats from the CAM_IOSCHED_DYNAMIC extension

[imp]

Summary

Report, on a periodic basis, the I/O latencies the CAM I/O scheduler
computes. These times are only for the hardware portion of the I/O as
measured from the time the operation is scheduled with the SIM using
xpt_action() until the SIM reports it has completed with xpt_dine(). Any
time the I/O operation spends in a software queue is no included.

The P50 (median), P90, P99 and P99.9 statitics about the latency of each
of the read, write and trim operations that completed during the polling
interval are reported. If there are fewer than 2, 10, 100 or 1000
operations during the polling interval, no statistic is reported and a
single dash '-' is displayed.

The read, write and trim commands (either on the command line or at run
time) toggle display of these operations. The color command toggles
color (it defaults to on, like gstat). When color is enabled, unknown
statitics are reported in blue, high latency for a statitics is reported
in red, medium in magenta and low in green (as with gstat). The med= and
hi= commands can set these latency thresholds.

Limitations: The entire sysctl space for all the devices is walked for
each polling period. This should be optimized to remember the OIDs and
only do such polling with the xpt generation changes. There is also no
way to filter devices displayed. This command only works on physical
devies that are connected to SCSI, ATA or NVME sims as those are the
only ones that are instrumented in the CAM I/O scheduler (the
CAM_IOSCHED_DYNAMIC option must be in the kernel, and the dynamic
scheduler can't be disabled).

Relnotes: yes
Sponsored by: Netflix

[debdrup]

Please run igor and mandoc -Tlint on the manual page - because among other things that they check for, I see that .Dd isn't bumped.

[bcr]

usr.bin/systat/systat.1

256

Suggestion: Avoid the use of "we" here by writing:

... are the percentiles that are computed based on ...

[pauamma_gundo.com]

"When color is enabled, unknown
statitics are reported in blue, high latency for a statitics is reported
in red, medium in magenta and low in green (as with gstat)."
How are those latencies reported when color is disabled? If they're not, I recommend a color-blindness and contrast check, perhaps using kontrast (if using it isn't an option for you eg because it doesn't work outside KDE, let me know and I'll look for another).

If the summary becomes the commit message (doesn't matter otherwise):

• s/statitic/statistic/g

usr.bin/systat/systat.1

259

269–273

I'd make this a bulleted list with "When color is enabled" factored out of the first.

I would also mention how they're displayed without color (see overall comment). Or if they're not, that should be mentioned in the BUGS section.

276

Do you mean "iolat"?

741–742

[imp]

In D34259#775366, @pauamma_gundo.com wrote:

"When color is enabled, unknown
statitics are reported in blue, high latency for a statitics is reported
in red, medium in magenta and low in green (as with gstat)."
How are those latencies reported when color is disabled? If they're not, I recommend a color-blindness and contrast check, perhaps using kontrast (if using it isn't an option for you eg because it doesn't work outside KDE, let me know and I'll look for another).

They are reported with the normal, default FG and BG colors (usually white on black or black on white).

If the summary becomes the commit message (doesn't matter otherwise):

• s/statitic/statistic/g

I'll fix locally. If there's more changes than this, I'll upload a new summary. Otherwise assume I'll make the change (updating this data in phabricator isn't automated).

[imp]

Update man page per review comments

[pauamma_gundo.com]

I would suggest either

• modifying the display to highlight high and low values when colors aren't used (perhaps bold/bright and underlined) as well for accessibility
• or documenting that this wasn't done in the BUGS section.

[imp]

In D34259#775436, @pauamma_gundo.com wrote:

I would suggest either

• modifying the display to highlight high and low values when colors aren't used (perhaps bold/bright and underlined) as well for accessibility
• or documenting that this wasn't done in the BUGS section.

I document that values aren't highlighted at all when colors aren't selected.

As for doing alternatives, curses makes that much harder than necessary to do reliably. Its abstraction layer for bold vs standout vs etc makes it hard to know how an improved highlighting would render. And there's not a natural progression available like there is with colors: is bold higher than underlined? Is bold and standout the same, or somehow different, etc. If I were targeting only one terminal, then it wouldn't be too bad, but the diversity of even 'xterm' compatible terminals makes it hard to get right.

[pauamma_gundo.com]

! In D34259#775504, @imp wrote:

As for doing alternatives, curses makes that much harder than necessary to do reliably. Its abstraction layer for bold vs standout vs etc makes it hard to know how an improved highlighting would render. And there's not a natural progression available like there is with colors: is bold higher than underlined? Is bold and standout the same, or somehow different, etc. If I were targeting only one terminal, then it wouldn't be too bad, but the diversity of even 'xterm' compatible terminals makes it hard to get right.

*swears at curses* Hrmph. OK. Then if documenting it (I'd like that to be in bugs, but your call) is the best we can do, then it's the best we can do. :-(

[imp]

rebase and minor tweaks

[pauamma_gundo.com]

LGTM. (Should I accept as manpages as well?)

[chs]

It would be good to go ahead and resolve the XXX comments that are currently being added by the patch.

usr.bin/systat/iolat.c

207

It would be better to do the op2num() once up walk_sysctl() and pass down the result, rather than doing it down in these lower-level functions. That would also eliminate the odd-looking business here of returning NULL even though setting up the new structure succeeded.

390

is there some reason that "_col" should have the leading underscore? I realize that you just copied it from iostat.c but it looks goofy. same down in showiolat().

413

It would be clearer to split this into two separate statements rather than abusing the comma operator. same down in showiolat().

[imp]

usr.bin/systat/iolat.c

207

Good idea

390

There's a global 'col' that I didn't want to shadow.

[imp]

update per chs' feedback