[PATCH 0/2] block: Add done_split_bio() for chained bio post-processing

[PATCH 0/2] block: Add done_split_bio() for chained bio post-processing

[Li kunyu]

[Problems encountered]

The split-out chained bio path has a QoS accounting issue. In a complete
chained bio composed of 10 bio headers and 1 sub-bio (produced by
repeatedly splitting a large block), only the first bio header among the
10 bio headers, the one that satisfies atomic_dec_and_test(&bio->
__bi_remaining) and the single sub-bio can execute rq_qos_done_bio(bio)
to update the QoS-related accounting.

This prevents other bio headers from updating QoS-related information
such as inflight counts. The inflight counts added in
rq_qos_throttle(rq->q, bio) are not subtracted, so the inflight count in
the bio's QoS area gradually increases. As a result, it can no longer be
used as a reference value.

Reproducible scenarios are detailed below(blk-iolatency):
1. fio is used to issue at least 1024 consecutive reads or writes, each 1MB
or larger, thereby exceeding the maximum single I/O size supported by the
hardware device.

2. The 'inflight' count described in the text is observed by printing
inflight = atomic_dec_return(&rqw->inflight); in blkcg_iolatency_done_bio.


[Implementation]

patch 1:  Introduce a new abstract function 'done_split_bio' and call it
from bio_remaining_done to resolve this issue.

patch 2: Implement the blkcg_iolatency_done_split_bio callback in
blk-iolatency to correctly decrement the inflight count when the other
bio headers, which have not called rq_qos_done_bio(bio), complete.


Li kunyu (2):
  block: Add done_split_bio() for chained bio post-processing
  block/blk-iolatency: Fix inflight decrement for chained bio sub-bios

 block/bio.c               |  2 ++
 block/blk-iolatency.c     | 42 +++++++++++++++++++++++++++++++++++++++
 block/blk-merge.c         |  4 +++-
 block/blk-rq-qos.c        |  9 +++++++++
 block/blk-rq-qos.h        | 23 +++++++++++++++++++++
 include/linux/blk_types.h |  6 ++++++
 6 files changed, 85 insertions(+), 1 deletion(-)

-- 
2.47.3

[PATCH 1/2] block: Add done_split_bio() for chained bio post-processing

[Li kunyu]

The split-out chained bio path has a QoS accounting issue. In a complete
chained bio composed of 10 bio headers and 1 sub-bio (produced by
repeatedly splitting a large block), only the first bio header among the
10 bio headers, the one that satisfies atomic_dec_and_test(&bio->
__bi_remaining) and the single sub-bio can execute rq_qos_done_bio(bio)
to update the QoS-related accounting. To address this, a new QoS callback,
done_split_bio(), is added so that QoS-related data can also be updated
for the bio headers that never reach rq_qos_done_bio().

Signed-off-by: Li kunyu <likunyu@open-hieco.net>
---
 block/bio.c               |  2 ++
 block/blk-merge.c         |  4 +++-
 block/blk-rq-qos.c        |  9 +++++++++
 block/blk-rq-qos.h        | 23 +++++++++++++++++++++++
 include/linux/blk_types.h |  6 ++++++
 5 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/block/bio.c b/block/bio.c
index f95b63c0604a..00ba928e7481 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1865,6 +1865,8 @@ static inline bool bio_remaining_done(struct bio *bio)
 		return true;
 	}
 
+	rq_qos_done_split_bio(bio);
+
 	return false;
 }
 
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 258a726071d1..e2781802168d 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -149,8 +149,10 @@ static struct bio *bio_submit_split(struct bio *bio, int split_sectors)
 	if (split_sectors) {
 		bio = bio_submit_split_bioset(bio, split_sectors,
 				&bio->bi_bdev->bd_disk->bio_split);
-		if (bio)
+		if (bio) {
 			bio->bi_opf |= REQ_NOMERGE;
+			bio_set_flag(bio, BIO_QOS_CHAIN_CHILD);
+		}
 	}
 
 	return bio;
diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
index 85cf74402a09..41ee04defca7 100644
--- a/block/blk-rq-qos.c
+++ b/block/blk-rq-qos.c
@@ -95,6 +95,15 @@ void __rq_qos_done_bio(struct rq_qos *rqos, struct bio *bio)
 	} while (rqos);
 }
 
+void __rq_qos_done_split_bio(struct rq_qos *rqos, struct bio *bio)
+{
+	do {
+		if (rqos->ops->done_split_bio)
+			rqos->ops->done_split_bio(rqos, bio);
+		rqos = rqos->next;
+	} while (rqos);
+}
+
 void __rq_qos_queue_depth_changed(struct rq_qos *rqos)
 {
 	do {
diff --git a/block/blk-rq-qos.h b/block/blk-rq-qos.h
index a747a504fe42..6c80ae02025a 100644
--- a/block/blk-rq-qos.h
+++ b/block/blk-rq-qos.h
@@ -45,6 +45,7 @@ struct rq_qos_ops {
 	void (*cleanup)(struct rq_qos *, struct bio *);
 	void (*queue_depth_changed)(struct rq_qos *);
 	void (*exit)(struct rq_qos *);
+	void (*done_split_bio)(struct rq_qos *, struct bio *);
 	const struct blk_mq_debugfs_attr *debugfs_attrs;
 };
 
@@ -108,6 +109,7 @@ void __rq_qos_throttle(struct rq_qos *rqos, struct bio *bio);
 void __rq_qos_track(struct rq_qos *rqos, struct request *rq, struct bio *bio);
 void __rq_qos_merge(struct rq_qos *rqos, struct request *rq, struct bio *bio);
 void __rq_qos_done_bio(struct rq_qos *rqos, struct bio *bio);
+void __rq_qos_done_split_bio(struct rq_qos *rqos, struct bio *bio);
 void __rq_qos_queue_depth_changed(struct rq_qos *rqos);
 
 static inline void rq_qos_cleanup(struct request_queue *q, struct bio *bio)
@@ -157,6 +159,27 @@ static inline void rq_qos_done_bio(struct bio *bio)
 		__rq_qos_done_bio(q->rq_qos, bio);
 }
 
+static inline void rq_qos_done_split_bio(struct bio *bio)
+{
+	struct request_queue *q;
+
+	if (!bio->bi_bdev || (!bio_flagged(bio, BIO_QOS_THROTTLED) &&
+			      !bio_flagged(bio, BIO_QOS_MERGED)) ||
+	    !bio_flagged(bio, BIO_QOS_CHAIN_CHILD))
+		return;
+
+	q = bdev_get_queue(bio->bi_bdev);
+
+	if (test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags) && q->rq_qos)
+		__rq_qos_done_split_bio(q->rq_qos, bio);
+
+	/*
+	 * Clear the flag to avoid multiple invocations of the function
+	 * for a single bio.
+	 */
+	bio_clear_flag(bio, BIO_QOS_CHAIN_CHILD);
+}
+
 static inline void rq_qos_throttle(struct request_queue *q, struct bio *bio)
 {
 	if (test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags) && q->rq_qos) {
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 98e21b4cbf32..bf1afbf8eaab 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -323,6 +323,12 @@ enum {
 	BIO_ZONE_WRITE_PLUGGING, /* bio handled through zone write plugging */
 	BIO_EMULATES_ZONE_APPEND, /* bio emulates a zone append operation */
 	BIO_COMPLETE_IN_TASK, /* complete bi_end_io() in task context */
+	/*
+	 * Marking the bio in a chained bio, except for the first bio header,
+	 * is mainly used to update the QoS statistics of the bio headers
+	 * after the first one.
+	 */
+	BIO_QOS_CHAIN_CHILD,
 	BIO_FLAG_LAST
 };
 
-- 
2.47.3

[PATCH 2/2] block/blk-iolatency: Fix inflight decrement for chained bio sub-bios

[Li kunyu]

Implement the blkcg_iolatency_done_split_bio callback in blk-iolatency to
correctly decrement the inflight count when the remaining bio headers
complete, excluding the one for which atomic_dec_and_test(&bio->
__bi_remaining) is true.

Signed-off-by: Li kunyu <likunyu@open-hieco.net>
---
 block/blk-iolatency.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
index 2caa79a008ad..e1ef3bb10572 100644
--- a/block/blk-iolatency.c
+++ b/block/blk-iolatency.c
@@ -632,6 +632,47 @@ static void blkcg_iolatency_done_bio(struct rq_qos *rqos, struct bio *bio)
 	}
 }
 
+static void blkcg_iolatency_done_split_bio(struct rq_qos *rqos, struct bio *bio)
+{
+	struct blkcg_gq *blkg;
+	struct rq_wait *rqw;
+	struct iolatency_grp *iolat;
+	int inflight = 0;
+
+	blkg = bio->bi_blkg;
+	if (!blkg || !bio_flagged(bio, BIO_QOS_THROTTLED) ||
+	    !bio_flagged(bio, BIO_QOS_CHAIN_CHILD))
+		return;
+
+	iolat = blkg_to_lat(bio->bi_blkg);
+	if (!iolat)
+		return;
+
+	if (!iolat->blkiolat->enabled)
+		return;
+
+	while (blkg && blkg->parent) {
+		iolat = blkg_to_lat(blkg);
+		if (!iolat) {
+			blkg = blkg->parent;
+			continue;
+		}
+		rqw = &iolat->rq_wait;
+
+		inflight = atomic_dec_return(&rqw->inflight);
+		WARN_ON_ONCE(inflight < 0);
+
+		/*
+		 * iolat->min_lat_nsec and other window-time equivalents
+		 * are not calculated here; they are calculated only when
+		 * blkcg_iolatency_done_bio() is called at the end of the
+		 * bio header.
+		 */
+		wake_up(&rqw->wait);
+		blkg = blkg->parent;
+	}
+}
+
 static void blkcg_iolatency_exit(struct rq_qos *rqos)
 {
 	struct blk_iolatency *blkiolat = BLKIOLATENCY(rqos);
@@ -645,6 +686,7 @@ static void blkcg_iolatency_exit(struct rq_qos *rqos)
 static const struct rq_qos_ops blkcg_iolatency_ops = {
 	.throttle = blkcg_iolatency_throttle,
 	.done_bio = blkcg_iolatency_done_bio,
+	.done_split_bio = blkcg_iolatency_done_split_bio,
 	.exit = blkcg_iolatency_exit,
 };
 
-- 
2.47.3

Re: [PATCH 0/2] block: Add done_split_bio() for chained bio post-processing

[Christoph Hellwig]

On Fri, Oct 30, 2026 at 02:59:43PM +0800, Li kunyu wrote:
> 
> patch 1:  Introduce a new abstract function 'done_split_bio' and call it
> from bio_remaining_done to resolve this issue.
> 
> patch 2: Implement the blkcg_iolatency_done_split_bio callback in
> blk-iolatency to correctly decrement the inflight count when the other
> bio headers, which have not called rq_qos_done_bio(bio), complete.

Why are you not simplify calling rq_qos_done_bio before
bio_remaining_done?  __bi_remaining is only incremented for clones of
various types, and the qos processing should probably happen before
calling back into the submitter anyway.

As far as I can tell all the code here is duplicating the existing
done code without much of an explanation of why we need another copy.

While we're at, the existing code in rq_qos_done_bio added in commit
aa1b46dcdc7b ("block: fix rq-qos breakage from skipping rq_qos_done_bio()")
is extremly wrong and dangerous.  Once a bio completes, bi_bdev
is invalid, the driver could have even used it for something that is not
a block device like md raid5 does, but it certainly does not have any
use for the qos/iocost code.  So it really needs to derive anything
it needs to I/O processing from the bio itself.