All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <Niklas.Cassel@wdc.com>
To: "axboe@kernel.dk" <axboe@kernel.dk>
Cc: "fio@vger.kernel.org" <fio@vger.kernel.org>,
	"damien.lemoal@opensource.wdc.com"
	<damien.lemoal@opensource.wdc.com>,
	Niklas Cassel <Niklas.Cassel@wdc.com>
Subject: [PATCH v3 2/6] stat: add comments describing the quirky behavior of clat prio samples
Date: Thu, 25 Nov 2021 13:20:30 +0000	[thread overview]
Message-ID: <20211125132020.109955-3-Niklas.Cassel@wdc.com> (raw)
In-Reply-To: <20211125132020.109955-1-Niklas.Cassel@wdc.com>

From: Niklas Cassel <niklas.cassel@wdc.com>

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>
---
 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.33.1

  reply	other threads:[~2021-11-25 13:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-25 13:20 [PATCH v3 0/6] cleanup clat prio stat handling Niklas Cassel
2021-11-25 13:20 ` Niklas Cassel [this message]
2021-11-25 13:20 ` [PATCH v3 1/6] docs: document quirky implementation of per priority stats reporting Niklas Cassel
2021-11-25 13:20 ` [PATCH v3 4/6] stat: rename add_lat_percentile_sample_noprio() Niklas Cassel
2021-11-25 13:20 ` [PATCH v3 3/6] stat: rename add_lat_percentile_sample() Niklas Cassel
2021-11-25 13:20 ` [PATCH v3 5/6] stat: simplify add_lat_percentile_prio_sample() Niklas Cassel
2021-11-25 13:20 ` [PATCH v3 6/6] stat: make add lat percentile functions inline Niklas Cassel
2021-11-25 16:03 ` [PATCH v3 0/6] cleanup clat prio stat handling Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211125132020.109955-3-Niklas.Cassel@wdc.com \
    --to=niklas.cassel@wdc.com \
    --cc=axboe@kernel.dk \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=fio@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.