All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tao Cui <cui.tao@linux.dev>
To: Jens Axboe <axboe@kernel.dk>
Cc: Tejun Heo <tj@kernel.org>, Josef Bacik <josef@toxicpanda.com>,
	Tang Yizhou <tangyeechou@gmail.com>,
	linux-block@vger.kernel.org, cgroups@vger.kernel.org,
	linux-kernel@vger.kernel.org, cui.tao@linux.dev,
	Tao Cui <cuitao@kylinos.cn>
Subject: [PATCH v2] block/blk-iolatency: fix always-zero cur_stat mean on non-SSD
Date: Tue, 21 Jul 2026 11:03:12 +0800	[thread overview]
Message-ID: <20260721030312.85655-1-cui.tao@linux.dev> (raw)

From: Tao Cui <cuitao@kylinos.cn>

iolatency_check_latencies() accumulates the per-window latency stat
into iolat->cur_stat via latency_stat_sum(), which for non-SSD devices
calls blk_rq_stat_sum().  But blk_rq_stat_sum() is contracted on a raw
per-cpu src -- it folds src->batch into the new mean -- and by the time
the per-window stat reaches cur_stat it is already aggregated, with
batch left at 0.  Each window therefore contributes zero latency to
cur_stat->mean, which on non-SSD devices stays at 0; the
latency_sum_ok(&cur_stat) check that gates scaling up is always true,
so the scale-up hysteresis never engages.  SSD devices use the
percentile path and are unaffected.

Add latency_stat_merge(), which merges two already-aggregated stats by
their reconstructed totals (mean * nr_samples) instead of src->batch,
and use it for the cur_stat accumulation.  The earlier per-cpu sum in
iolatency_check_latencies() still uses latency_stat_sum()/blk_rq_stat_sum(),
whose contract is a raw per-cpu src; only the cur_stat accumulation --
where the src is already aggregated -- switches to latency_stat_merge().

Fixes: d70675121546 ("block: introduce blk-iolatency io controller")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>

---
Changes in v2: move the fix from blk-stat.c (blk_rq_stat_sum) into
blk-iolatency.c, per Tang Yizhou's review -- blk_rq_stat_sum() is contracted on
a raw per-cpu src, so the aggregated-stat merge belongs in the iolatency
caller, not the shared helper.

v1: https://lore.kernel.org/all/20260720133813.45150-1-cui.tao@linux.dev/

 block/blk-iolatency.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
index cef02b6c5fa9..24fbd281e4b5 100644
--- a/block/blk-iolatency.c
+++ b/block/blk-iolatency.c
@@ -216,6 +216,32 @@ static inline void latency_stat_sum(struct iolatency_grp *iolat,
 		blk_rq_stat_sum(&sum->rqs, &stat->rqs);
 }
 
+/*
+ * Like latency_stat_sum(), but @stat is itself already aggregated (i.e.
+ * carries a valid mean), not a raw per-cpu stat. blk_rq_stat_sum() is
+ * contracted on a raw src -- it reads src->batch -- so for the non-SSD
+ * case merge by the reconstructed totals (mean * nr_samples) instead.
+ */
+static inline void latency_stat_merge(struct iolatency_grp *iolat,
+				      struct latency_stat *sum,
+				      struct latency_stat *stat)
+{
+	if (iolat->ssd) {
+		sum->ps.total += stat->ps.total;
+		sum->ps.missed += stat->ps.missed;
+	} else {
+		if (sum->rqs.nr_samples + stat->rqs.nr_samples <=
+		    sum->rqs.nr_samples)
+			return;
+		sum->rqs.mean = div_u64(sum->rqs.mean * sum->rqs.nr_samples +
+					stat->rqs.mean * stat->rqs.nr_samples,
+					sum->rqs.nr_samples + stat->rqs.nr_samples);
+		sum->rqs.min = min(sum->rqs.min, stat->rqs.min);
+		sum->rqs.max = max(sum->rqs.max, stat->rqs.max);
+		sum->rqs.nr_samples += stat->rqs.nr_samples;
+	}
+}
+
 static inline void latency_stat_record_time(struct iolatency_grp *iolat,
 					    u64 req_time)
 {
@@ -547,7 +573,7 @@ static void iolatency_check_latencies(struct iolatency_grp *iolat, u64 now)
 	/* Somebody beat us to the punch, just bail. */
 	spin_lock_irqsave(&lat_info->lock, flags);
 
-	latency_stat_sum(iolat, &iolat->cur_stat, &stat);
+	latency_stat_merge(iolat, &iolat->cur_stat, &stat);
 	lat_info->nr_samples -= iolat->nr_samples;
 	lat_info->nr_samples += latency_stat_samples(iolat, &iolat->cur_stat);
 	iolat->nr_samples = latency_stat_samples(iolat, &iolat->cur_stat);
--
2.43.0

                 reply	other threads:[~2026-07-21  3:03 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20260721030312.85655-1-cui.tao@linux.dev \
    --to=cui.tao@linux.dev \
    --cc=axboe@kernel.dk \
    --cc=cgroups@vger.kernel.org \
    --cc=cuitao@kylinos.cn \
    --cc=josef@toxicpanda.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tangyeechou@gmail.com \
    --cc=tj@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.