[PATCH 0/3] block: fix bi_vcnt misuse for cloned bio

[PATCH 0/3] block: fix bi_vcnt misuse for cloned bio

[Ming Lei]

Hello Guys,

The 1st patch removes .bi_vcnt from bio_may_need_split() because the
incoming bio may be cloned.

The 2nd patch doesn't initialize cloned bio's bi_vcnt in
bio_iov_bvec_set().

The 3rd patch removes iov iter nr_segs re-calculation for io_import_kbuf().


Ming Lei (3):
  block: fix bio_may_need_split() by using bvec iterator way
  block: don't initialize bi_vcnt for cloned bio in bio_iov_bvec_set()
  io_uring: don't re-calculate iov_iter nr_segs in io_import_kbuf()

 block/bio.c     |  2 +-
 block/blk.h     | 13 ++++++++++---
 io_uring/rsrc.c | 10 ----------
 3 files changed, 11 insertions(+), 14 deletions(-)

-- 
2.47.0

[PATCH 1/3] block: fix bio_may_need_split() by using bvec iterator way

[Ming Lei]

->bi_vcnt doesn't make sense for cloned bio, which is perfectly fine
passed to bio_may_need_split().

So fix bio_may_need_split() by not taking ->bi_vcnt directly, instead
checking with help from bio size and bvec->len.

Meantime retrieving the 1st bvec via __bvec_iter_bvec().

Fixes: abd45c159df5 ("block: handle fast path of bio splitting inline")
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk.h | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/block/blk.h b/block/blk.h
index e4c433f62dfc..a0b9cecba8fa 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -371,12 +371,19 @@ struct bio *bio_split_zone_append(struct bio *bio,
 static inline bool bio_may_need_split(struct bio *bio,
 		const struct queue_limits *lim)
 {
+	const struct bio_vec *bv;
+
 	if (lim->chunk_sectors)
 		return true;
-	if (bio->bi_vcnt != 1)
+
+	if ((bio_op(bio) != REQ_OP_READ && bio_op(bio) != REQ_OP_WRITE) ||
+			!bio->bi_io_vec)
+		return true;
+
+	bv = __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
+	if (bio->bi_iter.bi_size > bv->bv_len)
 		return true;
-	return bio->bi_io_vec->bv_len + bio->bi_io_vec->bv_offset >
-		lim->max_fast_segment_size;
+	return bv->bv_len + bv->bv_offset > lim->max_fast_segment_size;
 }
 
 /**
-- 
2.47.0

[PATCH 2/3] block: don't initialize bi_vcnt for cloned bio in bio_iov_bvec_set()

[Ming Lei]

For a cloned bio, bi_vcnt should not be initialized since the bio_vec
array is shared and owned by the original bio. Instead, initialize
bi_iter.bi_idx to 0 to properly start iteration from the beginning
of the shared bio_vec array.

This also avoids to touch iov_iter.nr_segs, which belongs to iov_iter
implementation detail.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/bio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/bio.c b/block/bio.c
index e726c0e280a8..79d1fef8ad0f 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1162,8 +1162,8 @@ void bio_iov_bvec_set(struct bio *bio, const struct iov_iter *iter)
 {
 	WARN_ON_ONCE(bio->bi_max_vecs);
 
-	bio->bi_vcnt = iter->nr_segs;
 	bio->bi_io_vec = (struct bio_vec *)iter->bvec;
+	bio->bi_iter.bi_idx = 0;
 	bio->bi_iter.bi_bvec_done = iter->iov_offset;
 	bio->bi_iter.bi_size = iov_iter_count(iter);
 	bio_set_flag(bio, BIO_CLONED);
-- 
2.47.0

[PATCH 3/3] io_uring: don't re-calculate iov_iter nr_segs in io_import_kbuf()

[Ming Lei]

We have provide correct byte counts to iov_iter, and it is enough to
cap the iteration, not necessary to re-calculate exact nr_segs.

Especially the previous two patches avoid to use bio->bi_vcnt as
split hint, and don't use iov_iter->nr_segs to initialize bio->bi_vcnt.

The iov_iter nr_segs re-calculation[1] is added for avoiding unnecessary
bio split, which is fixed now by the previous two patches.

[1] https://lkml.org/lkml/2025/4/16/351

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 io_uring/rsrc.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index a63474b331bf..ee6283676ba7 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -1055,16 +1055,6 @@ static int io_import_kbuf(int ddir, struct iov_iter *iter,
 
 	iov_iter_bvec(iter, ddir, imu->bvec, imu->nr_bvecs, count);
 	iov_iter_advance(iter, offset);
-
-	if (count < imu->len) {
-		const struct bio_vec *bvec = iter->bvec;
-
-		while (len > bvec->bv_len) {
-			len -= bvec->bv_len;
-			bvec++;
-		}
-		iter->nr_segs = 1 + bvec - iter->bvec;
-	}
 	return 0;
 }
 
-- 
2.47.0

Re: [PATCH 1/3] block: fix bio_may_need_split() by using bvec iterator way

[Christoph Hellwig]

On Thu, Dec 18, 2025 at 05:31:42PM +0800, Ming Lei wrote:
> ->bi_vcnt doesn't make sense for cloned bio, which is perfectly fine
> passed to bio_may_need_split().
> 
> So fix bio_may_need_split() by not taking ->bi_vcnt directly, instead
> checking with help from bio size and bvec->len.
> 
> Meantime retrieving the 1st bvec via __bvec_iter_bvec().

That totally misses the point.  The ->bi_vcnt is a fast and lose
check to see if we need the fairly expensive iterators to do the
real check.

Re: [PATCH 2/3] block: don't initialize bi_vcnt for cloned bio in bio_iov_bvec_set()

[Christoph Hellwig]

On Thu, Dec 18, 2025 at 05:31:43PM +0800, Ming Lei wrote:
> For a cloned bio, bi_vcnt should not be initialized since the bio_vec
> array is shared and owned by the original bio.

Maybe, maybe not.  What is the rational for that "should" ?

Re: [PATCH 1/3] block: fix bio_may_need_split() by using bvec iterator way

[Ming Lei]

On Thu, Dec 18, 2025 at 01:37:37AM -0800, Christoph Hellwig wrote:
> On Thu, Dec 18, 2025 at 05:31:42PM +0800, Ming Lei wrote:
> > ->bi_vcnt doesn't make sense for cloned bio, which is perfectly fine
> > passed to bio_may_need_split().
> > 
> > So fix bio_may_need_split() by not taking ->bi_vcnt directly, instead
> > checking with help from bio size and bvec->len.
> > 
> > Meantime retrieving the 1st bvec via __bvec_iter_bvec().
> 
> That totally misses the point.  The ->bi_vcnt is a fast and lose
> check to see if we need the fairly expensive iterators to do the
> real check.

It is just __bvec_iter_bvec(), whatever it should be in cache sooner or
later.


Thanks,
Ming

Re: [PATCH 2/3] block: don't initialize bi_vcnt for cloned bio in bio_iov_bvec_set()

[Ming Lei]

On Thu, Dec 18, 2025 at 01:38:55AM -0800, Christoph Hellwig wrote:
> On Thu, Dec 18, 2025 at 05:31:43PM +0800, Ming Lei wrote:
> > For a cloned bio, bi_vcnt should not be initialized since the bio_vec
> > array is shared and owned by the original bio.
> 
> Maybe, maybe not.  What is the rational for that "should" ?
 
->bi_vcnt is never set for bio allocated from bio_alloc_clone().

Thanks, 
Ming

Re: [PATCH 1/3] block: fix bio_may_need_split() by using bvec iterator way

[Nitesh Shetty]

[-- Attachment #1: Type: text/plain, Size: 3726 bytes --]

On 18/12/25 05:45PM, Ming Lei wrote:
>On Thu, Dec 18, 2025 at 01:37:37AM -0800, Christoph Hellwig wrote:
>> On Thu, Dec 18, 2025 at 05:31:42PM +0800, Ming Lei wrote:
>> > ->bi_vcnt doesn't make sense for cloned bio, which is perfectly fine
>> > passed to bio_may_need_split().
>> >
>> > So fix bio_may_need_split() by not taking ->bi_vcnt directly, instead
>> > checking with help from bio size and bvec->len.
>> >
>> > Meantime retrieving the 1st bvec via __bvec_iter_bvec().
>>
>> That totally misses the point.  The ->bi_vcnt is a fast and lose
>> check to see if we need the fairly expensive iterators to do the
>> real check.
>
>It is just __bvec_iter_bvec(), whatever it should be in cache sooner or
>later.
>
>
Functionality wise overall patch looks fine to me, but as Christoph
stated there is slight performance(IOPS) penalty.
Here is my benchmarking numbers[1], I suspect Jens setup might show
more regression.

Regards,
Nitesh


[1]
===============================
a. two optane nvme device setup:
----------
base case:
----------
sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
-d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nvme0n1 /dev/nvme1n1
submitter=0, tid=206586, file=/dev/nvme0n1, nfiles=1, node=-1
submitter=1, tid=206587, file=/dev/nvme1n1, nfiles=1, node=-1
polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
Engine=io_uring, sq_ring=128, cq_ring=128
polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
Engine=io_uring, sq_ring=128, cq_ring=128
IOPS=6.45M, BW=3.15GiB/s, IOS/call=32/31
IOPS=6.47M, BW=3.16GiB/s, IOS/call=32/32
IOPS=6.47M, BW=3.16GiB/s, IOS/call=32/32
Exiting on timeout
Maximum IOPS=6.47M

----------------
with this patch:
----------------
sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
-d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nvme0n1 /dev/nvme1n1
submitter=0, tid=6352, file=/dev/nvme0n1, nfiles=1, node=-1
polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
Engine=io_uring, sq_ring=128, cq_ring=128
submitter=1, tid=6353, file=/dev/nvme1n1, nfiles=1, node=-1
IOPS=6.30M, BW=3.08GiB/s, IOS/call=32/31
IOPS=6.35M, BW=3.10GiB/s, IOS/call=32/31
IOPS=6.37M, BW=3.11GiB/s, IOS/call=32/32
Exiting on timeout
Maximum IOPS=6.37M

=============================
b. two null-blk device setup:
------------------
null device setup:
------------------
sudo modprobe null_blk queue_mode=2 gb=10 bs=512 nr_devices=2 irqmode=2 \
completion_nsec=1000000 hw_queue_depth=256 memory_backed=0 discard=0 \
use_per_node_hctx=1

----------
base case:
----------
sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
-d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nullb0 /dev/nullb1
submitter=0, tid=6743, file=/dev/nullb0, nfiles=1, node=-1
submitter=1, tid=6744, file=/dev/nullb1, nfiles=1, node=-1
polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
Engine=io_uring, sq_ring=128, cq_ring=128
IOPS=7.89M, BW=3.85GiB/s, IOS/call=32/31
IOPS=7.96M, BW=3.89GiB/s, IOS/call=32/32
IOPS=7.99M, BW=3.90GiB/s, IOS/call=32/32
Exiting on timeout
Maximum IOPS=7.99M

-------------------
with this patchset:
-------------------
sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
-d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nullb0 /dev/nullb1
submitter=0, tid=35633, file=/dev/nullb0, nfiles=1, node=-1
submitter=1, tid=35634, file=/dev/nullb1, nfiles=1, node=-1
polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
Engine=io_uring, sq_ring=128, cq_ring=128
polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
Engine=io_uring, sq_ring=128, cq_ring=128
IOPS=7.79M, BW=3.80GiB/s, IOS/call=32/31
IOPS=7.86M, BW=3.84GiB/s, IOS/call=32/32
IOPS=7.89M, BW=3.85GiB/s, IOS/call=32/32
Exiting on timeout
Maximum IOPS=7.89M

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/3] block: fix bio_may_need_split() by using bvec iterator way

[Ming Lei]

On Thu, Dec 18, 2025 at 08:46:47PM +0530, Nitesh Shetty wrote:
> On 18/12/25 05:45PM, Ming Lei wrote:
> > On Thu, Dec 18, 2025 at 01:37:37AM -0800, Christoph Hellwig wrote:
> > > On Thu, Dec 18, 2025 at 05:31:42PM +0800, Ming Lei wrote:
> > > > ->bi_vcnt doesn't make sense for cloned bio, which is perfectly fine
> > > > passed to bio_may_need_split().
> > > >
> > > > So fix bio_may_need_split() by not taking ->bi_vcnt directly, instead
> > > > checking with help from bio size and bvec->len.
> > > >
> > > > Meantime retrieving the 1st bvec via __bvec_iter_bvec().
> > > 
> > > That totally misses the point.  The ->bi_vcnt is a fast and lose
> > > check to see if we need the fairly expensive iterators to do the
> > > real check.
> > 
> > It is just __bvec_iter_bvec(), whatever it should be in cache sooner or
> > later.
> > 
> > 
> Functionality wise overall patch looks fine to me, but as Christoph
> stated there is slight performance(IOPS) penalty.
> Here is my benchmarking numbers[1], I suspect Jens setup might show
> more regression.
> 
> Regards,
> Nitesh
> 
> 
> [1]
> ===============================
> a. two optane nvme device setup:
> ----------
> base case:
> ----------
> sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
> -d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nvme0n1 /dev/nvme1n1
> submitter=0, tid=206586, file=/dev/nvme0n1, nfiles=1, node=-1
> submitter=1, tid=206587, file=/dev/nvme1n1, nfiles=1, node=-1
> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
> Engine=io_uring, sq_ring=128, cq_ring=128
> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
> Engine=io_uring, sq_ring=128, cq_ring=128
> IOPS=6.45M, BW=3.15GiB/s, IOS/call=32/31
> IOPS=6.47M, BW=3.16GiB/s, IOS/call=32/32
> IOPS=6.47M, BW=3.16GiB/s, IOS/call=32/32
> Exiting on timeout
> Maximum IOPS=6.47M
> 
> ----------------
> with this patch:
> ----------------
> sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
> -d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nvme0n1 /dev/nvme1n1
> submitter=0, tid=6352, file=/dev/nvme0n1, nfiles=1, node=-1
> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
> Engine=io_uring, sq_ring=128, cq_ring=128
> submitter=1, tid=6353, file=/dev/nvme1n1, nfiles=1, node=-1
> IOPS=6.30M, BW=3.08GiB/s, IOS/call=32/31
> IOPS=6.35M, BW=3.10GiB/s, IOS/call=32/31
> IOPS=6.37M, BW=3.11GiB/s, IOS/call=32/32
> Exiting on timeout
> Maximum IOPS=6.37M
> 
> =============================
> b. two null-blk device setup:
> ------------------
> null device setup:
> ------------------
> sudo modprobe null_blk queue_mode=2 gb=10 bs=512 nr_devices=2 irqmode=2 \
> completion_nsec=1000000 hw_queue_depth=256 memory_backed=0 discard=0 \
> use_per_node_hctx=1
> 
> ----------
> base case:
> ----------
> sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
> -d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nullb0 /dev/nullb1
> submitter=0, tid=6743, file=/dev/nullb0, nfiles=1, node=-1
> submitter=1, tid=6744, file=/dev/nullb1, nfiles=1, node=-1
> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
> Engine=io_uring, sq_ring=128, cq_ring=128
> IOPS=7.89M, BW=3.85GiB/s, IOS/call=32/31
> IOPS=7.96M, BW=3.89GiB/s, IOS/call=32/32
> IOPS=7.99M, BW=3.90GiB/s, IOS/call=32/32
> Exiting on timeout
> Maximum IOPS=7.99M
> 
> -------------------
> with this patchset:
> -------------------
> sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
> -d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nullb0 /dev/nullb1
> submitter=0, tid=35633, file=/dev/nullb0, nfiles=1, node=-1
> submitter=1, tid=35634, file=/dev/nullb1, nfiles=1, node=-1
> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
> Engine=io_uring, sq_ring=128, cq_ring=128
> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
> Engine=io_uring, sq_ring=128, cq_ring=128
> IOPS=7.79M, BW=3.80GiB/s, IOS/call=32/31
> IOPS=7.86M, BW=3.84GiB/s, IOS/call=32/32
> IOPS=7.89M, BW=3.85GiB/s, IOS/call=32/32
> Exiting on timeout
> Maximum IOPS=7.89M

Thanks for the perf test!

This patch only adds bio->bi_iter memory footprint, which is supposed
to hit from L1, maybe because `bi_io_vec` is in the 2nd cacheline, can
you see any difference with the following change?


diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 5dc061d318a4..1c4570b37436 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -240,6 +240,7 @@ struct bio {
                /* for plugged zoned writes only: */
                unsigned int            __bi_nr_segments;
        };
+       struct bio_vec          *bi_io_vec;     /* the actual vec list */
        bio_end_io_t            *bi_end_io;
        void                    *bi_private;
 #ifdef CONFIG_BLK_CGROUP
@@ -275,8 +276,6 @@ struct bio {

        atomic_t                __bi_cnt;       /* pin count */

-       struct bio_vec          *bi_io_vec;     /* the actual vec list */
-
        struct bio_set          *bi_pool;
 };



Thanks,
Ming

Re: [PATCH 1/3] block: fix bio_may_need_split() by using bvec iterator way

[Nitesh Shetty]

[-- Attachment #1: Type: text/plain, Size: 5205 bytes --]

On 19/12/25 12:08AM, Ming Lei wrote:
>On Thu, Dec 18, 2025 at 08:46:47PM +0530, Nitesh Shetty wrote:
>> On 18/12/25 05:45PM, Ming Lei wrote:
>> > On Thu, Dec 18, 2025 at 01:37:37AM -0800, Christoph Hellwig wrote:
>> > > On Thu, Dec 18, 2025 at 05:31:42PM +0800, Ming Lei wrote:
>> > > > ->bi_vcnt doesn't make sense for cloned bio, which is perfectly fine
>> > > > passed to bio_may_need_split().
>> > > >
>> > > > So fix bio_may_need_split() by not taking ->bi_vcnt directly, instead
>> > > > checking with help from bio size and bvec->len.
>> > > >
>> > > > Meantime retrieving the 1st bvec via __bvec_iter_bvec().
>> > >
>> > > That totally misses the point.  The ->bi_vcnt is a fast and lose
>> > > check to see if we need the fairly expensive iterators to do the
>> > > real check.
>> >
>> > It is just __bvec_iter_bvec(), whatever it should be in cache sooner or
>> > later.
>> >
>> >
>> Functionality wise overall patch looks fine to me, but as Christoph
>> stated there is slight performance(IOPS) penalty.
>> Here is my benchmarking numbers[1], I suspect Jens setup might show
>> more regression.
>>
>> Regards,
>> Nitesh
>>
>>
>> [1]
>> ===============================
>> a. two optane nvme device setup:
>> ----------
>> base case:
>> ----------
>> sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
>> -d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nvme0n1 /dev/nvme1n1
>> submitter=0, tid=206586, file=/dev/nvme0n1, nfiles=1, node=-1
>> submitter=1, tid=206587, file=/dev/nvme1n1, nfiles=1, node=-1
>> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
>> Engine=io_uring, sq_ring=128, cq_ring=128
>> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
>> Engine=io_uring, sq_ring=128, cq_ring=128
>> IOPS=6.45M, BW=3.15GiB/s, IOS/call=32/31
>> IOPS=6.47M, BW=3.16GiB/s, IOS/call=32/32
>> IOPS=6.47M, BW=3.16GiB/s, IOS/call=32/32
>> Exiting on timeout
>> Maximum IOPS=6.47M
>>
>> ----------------
>> with this patch:
>> ----------------
>> sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
>> -d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nvme0n1 /dev/nvme1n1
>> submitter=0, tid=6352, file=/dev/nvme0n1, nfiles=1, node=-1
>> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
>> Engine=io_uring, sq_ring=128, cq_ring=128
>> submitter=1, tid=6353, file=/dev/nvme1n1, nfiles=1, node=-1
>> IOPS=6.30M, BW=3.08GiB/s, IOS/call=32/31
>> IOPS=6.35M, BW=3.10GiB/s, IOS/call=32/31
>> IOPS=6.37M, BW=3.11GiB/s, IOS/call=32/32
>> Exiting on timeout
>> Maximum IOPS=6.37M
>>
>> =============================
>> b. two null-blk device setup:
>> ------------------
>> null device setup:
>> ------------------
>> sudo modprobe null_blk queue_mode=2 gb=10 bs=512 nr_devices=2 irqmode=2 \
>> completion_nsec=1000000 hw_queue_depth=256 memory_backed=0 discard=0 \
>> use_per_node_hctx=1
>>
>> ----------
>> base case:
>> ----------
>> sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
>> -d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nullb0 /dev/nullb1
>> submitter=0, tid=6743, file=/dev/nullb0, nfiles=1, node=-1
>> submitter=1, tid=6744, file=/dev/nullb1, nfiles=1, node=-1
>> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
>> Engine=io_uring, sq_ring=128, cq_ring=128
>> IOPS=7.89M, BW=3.85GiB/s, IOS/call=32/31
>> IOPS=7.96M, BW=3.89GiB/s, IOS/call=32/32
>> IOPS=7.99M, BW=3.90GiB/s, IOS/call=32/32
>> Exiting on timeout
>> Maximum IOPS=7.99M
>>
>> -------------------
>> with this patchset:
>> -------------------
>> sudo taskset -c 0,1 /home/nitesh/src/private/fio/t/io_uring -b512 \
>> -d128 -c32 -s32 -p1 -F1 -B1 -n2 -r4 /dev/nullb0 /dev/nullb1
>> submitter=0, tid=35633, file=/dev/nullb0, nfiles=1, node=-1
>> submitter=1, tid=35634, file=/dev/nullb1, nfiles=1, node=-1
>> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
>> Engine=io_uring, sq_ring=128, cq_ring=128
>> polled=1, fixedbufs=1, register_files=1, buffered=0, QD=128
>> Engine=io_uring, sq_ring=128, cq_ring=128
>> IOPS=7.79M, BW=3.80GiB/s, IOS/call=32/31
>> IOPS=7.86M, BW=3.84GiB/s, IOS/call=32/32
>> IOPS=7.89M, BW=3.85GiB/s, IOS/call=32/32
>> Exiting on timeout
>> Maximum IOPS=7.89M
>
>Thanks for the perf test!
>
>This patch only adds bio->bi_iter memory footprint, which is supposed
>to hit from L1, maybe because `bi_io_vec` is in the 2nd cacheline, can
>you see any difference with the following change?
>
>
>diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
>index 5dc061d318a4..1c4570b37436 100644
>--- a/include/linux/blk_types.h
>+++ b/include/linux/blk_types.h
>@@ -240,6 +240,7 @@ struct bio {
>                /* for plugged zoned writes only: */
>                unsigned int            __bi_nr_segments;
>        };
>+       struct bio_vec          *bi_io_vec;     /* the actual vec list */
>        bio_end_io_t            *bi_end_io;
>        void                    *bi_private;
> #ifdef CONFIG_BLK_CGROUP
>@@ -275,8 +276,6 @@ struct bio {
>
>        atomic_t                __bi_cnt;       /* pin count */
>
>-       struct bio_vec          *bi_io_vec;     /* the actual vec list */
>-
>        struct bio_set          *bi_pool;
> };
>
With above patch perf numbers match the base case.

Thanks,
Nitesh Shetty

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/3] block: fix bio_may_need_split() by using bvec iterator way

[Nitesh Shetty]

[-- Attachment #1: Type: text/plain, Size: 1146 bytes --]

On 18/12/25 05:31PM, Ming Lei wrote:
>->bi_vcnt doesn't make sense for cloned bio, which is perfectly fine
>passed to bio_may_need_split().
>
>So fix bio_may_need_split() by not taking ->bi_vcnt directly, instead
>checking with help from bio size and bvec->len.
>
>Meantime retrieving the 1st bvec via __bvec_iter_bvec().
>
>Fixes: abd45c159df5 ("block: handle fast path of bio splitting inline")
>Signed-off-by: Ming Lei <ming.lei@redhat.com>
>---
> block/blk.h | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
>diff --git a/block/blk.h b/block/blk.h
>index e4c433f62dfc..a0b9cecba8fa 100644
>--- a/block/blk.h
>+++ b/block/blk.h
>@@ -371,12 +371,19 @@ struct bio *bio_split_zone_append(struct bio *bio,
> static inline bool bio_may_need_split(struct bio *bio,
> 		const struct queue_limits *lim)
> {
>+	const struct bio_vec *bv;
>+
> 	if (lim->chunk_sectors)
> 		return true;
>-	if (bio->bi_vcnt != 1)
>+
>+	if ((bio_op(bio) != REQ_OP_READ && bio_op(bio) != REQ_OP_WRITE) ||
>+			!bio->bi_io_vec)
REQ_OP_READ, REQ_OP_WRITE check is not necessary, since bio_may_need_split
is always called for READ/WRITE.

Thanks,
Nitesh

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]