stat: add comments describing the quirky behavior of clat prio samples
authorNiklas Cassel <niklas.cassel@wdc.com>
Thu, 25 Nov 2021 13:20:30 +0000 (13:20 +0000)
committerJens Axboe <axboe@kernel.dk>
Thu, 25 Nov 2021 16:03:10 +0000 (09:03 -0700)
Commit 56440e63ac17 ("fio: report percentiles for slat, clat, lat")
together with commit 38ec5c514104 ("stat: make priority summary statistics
consistent with percentiles") changed so that per prio stats track either
completion latency (clat) or total latency (lat), depending on the option
lat_percentiles.

It is not obvious why add_clat_sample() shouldn't add a high/low clat prio
sample when option lat_percentiles is set, especially considering that
option lat_percentiles is usually used for controlling if total latency
percentiles should be displayed or not.

Add comments to describe why add_clat_sample() has to care about option
lat_percentiles.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Link: https://lore.kernel.org/r/20211125132020.109955-3-Niklas.Cassel@wdc.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
stat.c

diff --git a/stat.c b/stat.c
index e0dc99b6eb4b9413aaf590af958572569f20af62..8bb49e4be9181918870752bdd40eb5591b21e63e 100644 (file)
--- a/stat.c
+++ b/stat.c
@@ -3089,6 +3089,15 @@ void add_clat_sample(struct thread_data *td, enum fio_ddir ddir,
 
        add_stat_sample(&ts->clat_stat[ddir], nsec);
 
+       /*
+        * When lat_percentiles=1 (default 0), the reported high/low priority
+        * percentiles and stats are used for describing total latency values,
+        * even though the variable names themselves start with clat_.
+        *
+        * Because of the above definition, add a prio stat sample only when
+        * lat_percentiles=0. add_lat_sample() will add the prio stat sample
+        * when lat_percentiles=1.
+        */
        if (!ts->lat_percentiles) {
                if (high_prio)
                        add_stat_sample(&ts->clat_high_prio_stat[ddir], nsec);
@@ -3101,6 +3110,11 @@ void add_clat_sample(struct thread_data *td, enum fio_ddir ddir,
                               offset, ioprio);
 
        if (ts->clat_percentiles) {
+               /*
+                * Because of the above definition, add a prio lat percentile
+                * sample only when lat_percentiles=0. add_lat_sample() will add
+                * the prio lat percentile sample when lat_percentiles=1.
+                */
                if (ts->lat_percentiles)
                        add_lat_percentile_sample_noprio(ts, nsec, ddir, FIO_CLAT);
                else
@@ -3194,6 +3208,16 @@ void add_lat_sample(struct thread_data *td, enum fio_ddir ddir,
                add_log_sample(td, td->lat_log, sample_val(nsec), ddir, bs,
                               offset, ioprio);
 
+       /*
+        * When lat_percentiles=1 (default 0), the reported high/low priority
+        * percentiles and stats are used for describing total latency values,
+        * even though the variable names themselves start with clat_.
+        *
+        * Because of the above definition, add a prio stat and prio lat
+        * percentile sample only when lat_percentiles=1. add_clat_sample() will
+        * add the prio stat and prio lat percentile sample when
+        * lat_percentiles=0.
+        */
        if (ts->lat_percentiles) {
                add_lat_percentile_sample(ts, nsec, ddir, high_prio, FIO_LAT);
                if (high_prio)