From 717569b13cabd1485cc14a7e235643aa88974cd4 Mon Sep 17 00:00:00 2001 From: Niklas Cassel Date: Thu, 25 Nov 2021 13:20:30 +0000 Subject: [PATCH] stat: add comments describing the quirky behavior of clat prio samples 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 Reviewed-by: Damien Le Moal Link: https://lore.kernel.org/r/20211125132020.109955-3-Niklas.Cassel@wdc.com Signed-off-by: Jens Axboe --- stat.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/stat.c b/stat.c index e0dc99b6..8bb49e4b 100644 --- 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) -- 2.25.1