[RFC 0/2] optimise split bio

[RFC 0/2] optimise split bio

[Pavel Begunkov]

With the no-copy bvec patches and nullb eats blk_bio_segment_split()
eats >7%. It adds yet another fast path for it.

        8K              16K             32K             64K
before: 932             904             868             788
after:  934             919             902             862

Would appreciate if anyone knows off the bat typical queue_max_segments, etc.
numbers for NVMe.

Pavel Begunkov (2):
  block: add a function for *segment_split fast path
  block: add a fast path for seg split of large bio

 block/blk-merge.c | 107 +++++++++++++++++++++++++++++-----------------
 1 file changed, 68 insertions(+), 39 deletions(-)

-- 
2.24.0

[RFC 1/2] block: add a function for *segment_split fast path

[Pavel Begunkov]

Don't keep blk_bio_segment_split()'s fast path hand coded in
__blk_queue_split(), extract it into a function. It's inlined perfectly
well.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 block/blk-merge.c | 84 ++++++++++++++++++++++++++---------------------
 1 file changed, 46 insertions(+), 38 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 808768f6b174..84b9635b5d57 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -223,29 +223,10 @@ static bool bvec_split_segs(const struct request_queue *q,
 	return len > 0 || bv->bv_len > max_len;
 }
 
-/**
- * blk_bio_segment_split - split a bio in two bios
- * @q:    [in] request queue pointer
- * @bio:  [in] bio to be split
- * @bs:	  [in] bio set to allocate the clone from
- * @segs: [out] number of segments in the bio with the first half of the sectors
- *
- * Clone @bio, update the bi_iter of the clone to represent the first sectors
- * of @bio and update @bio->bi_iter to represent the remaining sectors. The
- * following is guaranteed for the cloned bio:
- * - That it has at most get_max_io_size(@q, @bio) sectors.
- * - That it has at most queue_max_segments(@q) segments.
- *
- * Except for discard requests the cloned bio will point at the bi_io_vec of
- * the original bio. It is the responsibility of the caller to ensure that the
- * original bio is not freed before the cloned bio. The caller is also
- * responsible for ensuring that @bs is only destroyed after processing of the
- * split bio has finished.
- */
-static struct bio *blk_bio_segment_split(struct request_queue *q,
-					 struct bio *bio,
-					 struct bio_set *bs,
-					 unsigned *segs)
+static struct bio *__blk_bio_segment_split(struct request_queue *q,
+					   struct bio *bio,
+					   struct bio_set *bs,
+					   unsigned *segs)
 {
 	struct bio_vec bv, bvprv, *bvprvp = NULL;
 	struct bvec_iter iter;
@@ -290,6 +271,48 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
 	return bio_split(bio, sectors, GFP_NOIO, bs);
 }
 
+/**
+ * blk_bio_segment_split - split a bio in two bios
+ * @q:    [in] request queue pointer
+ * @bio:  [in] bio to be split
+ * @bs:	  [in] bio set to allocate the clone from
+ * @segs: [out] number of segments in the bio with the first half of the sectors
+ *
+ * Clone @bio, update the bi_iter of the clone to represent the first sectors
+ * of @bio and update @bio->bi_iter to represent the remaining sectors. The
+ * following is guaranteed for the cloned bio:
+ * - That it has at most get_max_io_size(@q, @bio) sectors.
+ * - That it has at most queue_max_segments(@q) segments.
+ *
+ * Except for discard requests the cloned bio will point at the bi_io_vec of
+ * the original bio. It is the responsibility of the caller to ensure that the
+ * original bio is not freed before the cloned bio. The caller is also
+ * responsible for ensuring that @bs is only destroyed after processing of the
+ * split bio has finished.
+ */
+static inline struct bio *blk_bio_segment_split(struct request_queue *q,
+						struct bio *bio,
+						struct bio_set *bs,
+						unsigned *nr_segs)
+{
+	/*
+	 * All drivers must accept single-segments bios that are <=
+	 * PAGE_SIZE.  This is a quick and dirty check that relies on
+	 * the fact that bi_io_vec[0] is always valid if a bio has data.
+	 * The check might lead to occasional false negatives when bios
+	 * are cloned, but compared to the performance impact of cloned
+	 * bios themselves the loop below doesn't matter anyway.
+	 */
+	if (!q->limits.chunk_sectors && bio->bi_vcnt == 1 &&
+	    (bio->bi_io_vec[0].bv_len +
+	     bio->bi_io_vec[0].bv_offset) <= PAGE_SIZE) {
+		*nr_segs = 1;
+		return NULL;
+	}
+
+	return __blk_bio_segment_split(q, bio, bs, nr_segs);
+}
+
 /**
  * __blk_queue_split - split a bio and submit the second half
  * @bio:     [in, out] bio to be split
@@ -322,21 +345,6 @@ void __blk_queue_split(struct bio **bio, unsigned int *nr_segs)
 				nr_segs);
 		break;
 	default:
-		/*
-		 * All drivers must accept single-segments bios that are <=
-		 * PAGE_SIZE.  This is a quick and dirty check that relies on
-		 * the fact that bi_io_vec[0] is always valid if a bio has data.
-		 * The check might lead to occasional false negatives when bios
-		 * are cloned, but compared to the performance impact of cloned
-		 * bios themselves the loop below doesn't matter anyway.
-		 */
-		if (!q->limits.chunk_sectors &&
-		    (*bio)->bi_vcnt == 1 &&
-		    ((*bio)->bi_io_vec[0].bv_len +
-		     (*bio)->bi_io_vec[0].bv_offset) <= PAGE_SIZE) {
-			*nr_segs = 1;
-			break;
-		}
 		split = blk_bio_segment_split(q, *bio, &q->bio_split, nr_segs);
 		break;
 	}
-- 
2.24.0

[RFC 2/2] block: add a fast path for seg split of large bio

[Pavel Begunkov]

blk_bio_segment_split() is very heavy, but the current fast path covers
only one-segment under PAGE_SIZE bios. Add another one by estimating an
upper bound of sectors a bio can contain.

One restricting factor here is queue_max_segment_size(), which it
compare against full iter size to not dig into bvecs. By default it's
64KB, and so for requests under 64KB, but for those falling under the
conditions it's much faster.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 block/blk-merge.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 84b9635b5d57..15d75f3ffc30 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -226,12 +226,12 @@ static bool bvec_split_segs(const struct request_queue *q,
 static struct bio *__blk_bio_segment_split(struct request_queue *q,
 					   struct bio *bio,
 					   struct bio_set *bs,
-					   unsigned *segs)
+					   unsigned *segs,
+					   const unsigned max_sectors)
 {
 	struct bio_vec bv, bvprv, *bvprvp = NULL;
 	struct bvec_iter iter;
 	unsigned nsegs = 0, sectors = 0;
-	const unsigned max_sectors = get_max_io_size(q, bio);
 	const unsigned max_segs = queue_max_segments(q);
 
 	bio_for_each_bvec(bv, bio, iter) {
@@ -295,6 +295,9 @@ static inline struct bio *blk_bio_segment_split(struct request_queue *q,
 						struct bio_set *bs,
 						unsigned *nr_segs)
 {
+	unsigned int max_sectors, q_max_sectors;
+	unsigned int bio_segs = bio->bi_vcnt;
+
 	/*
 	 * All drivers must accept single-segments bios that are <=
 	 * PAGE_SIZE.  This is a quick and dirty check that relies on
@@ -303,14 +306,32 @@ static inline struct bio *blk_bio_segment_split(struct request_queue *q,
 	 * are cloned, but compared to the performance impact of cloned
 	 * bios themselves the loop below doesn't matter anyway.
 	 */
-	if (!q->limits.chunk_sectors && bio->bi_vcnt == 1 &&
+	if (!q->limits.chunk_sectors && bio_segs == 1 &&
 	    (bio->bi_io_vec[0].bv_len +
 	     bio->bi_io_vec[0].bv_offset) <= PAGE_SIZE) {
 		*nr_segs = 1;
 		return NULL;
 	}
 
-	return __blk_bio_segment_split(q, bio, bs, nr_segs);
+	q_max_sectors = get_max_io_size(q, bio);
+	if (!queue_virt_boundary(q) && bio_segs < queue_max_segments(q) &&
+	    bio->bi_iter.bi_size <= queue_max_segment_size(q)) {
+		/*
+		 * Segments are contiguous, so only their ends may be not full.
+		 * An upper bound for them would to assume that each takes 1B
+		 * but adds a sector, and all left are just full sectors.
+		 * Note: it's ok to round size down because all not full
+		 * sectors are accounted by the first term.
+		 */
+		max_sectors = bio_segs * 2;
+		max_sectors += bio->bi_iter.bi_size >> 9;
+
+		if (max_sectors < q_max_sectors) {
+			*nr_segs = bio_segs;
+			return NULL;
+		}
+	}
+	return __blk_bio_segment_split(q, bio, bs, nr_segs, q_max_sectors);
 }
 
 /**
-- 
2.24.0

Re: [RFC 2/2] block: add a fast path for seg split of large bio

[Pavel Begunkov]

On 05/01/2021 19:43, Pavel Begunkov wrote:
> blk_bio_segment_split() is very heavy, but the current fast path covers
> only one-segment under PAGE_SIZE bios. Add another one by estimating an
> upper bound of sectors a bio can contain.
> 
> One restricting factor here is queue_max_segment_size(), which it
> compare against full iter size to not dig into bvecs. By default it's
> 64KB, and so for requests under 64KB, but for those falling under the
> conditions it's much faster.
> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  block/blk-merge.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
[...]
>  
> -	return __blk_bio_segment_split(q, bio, bs, nr_segs);
> +	q_max_sectors = get_max_io_size(q, bio);
> +	if (!queue_virt_boundary(q) && bio_segs < queue_max_segments(q) &&
> +	    bio->bi_iter.bi_size <= queue_max_segment_size(q)) {

I think I miss a seg_boundary_mask check here. Any insights how to skip it?
Or why it's 2^31-1 by default, but not say ((unsigned long)-1)?


-- 
Pavel Begunkov

Re: [RFC 2/2] block: add a fast path for seg split of large bio

[Christoph Hellwig]

On Tue, Jan 05, 2021 at 07:43:38PM +0000, Pavel Begunkov wrote:
> blk_bio_segment_split() is very heavy, but the current fast path covers
> only one-segment under PAGE_SIZE bios. Add another one by estimating an
> upper bound of sectors a bio can contain.
> 
> One restricting factor here is queue_max_segment_size(), which it
> compare against full iter size to not dig into bvecs. By default it's
> 64KB, and so for requests under 64KB, but for those falling under the
> conditions it's much faster.

I think this works, but it is a pretty gross heuristic, which also
doesn't help us with NVMe, which is the I/O fast path of choice for
most people.  What is your use/test case?

> +		/*
> +		 * Segments are contiguous, so only their ends may be not full.
> +		 * An upper bound for them would to assume that each takes 1B
> +		 * but adds a sector, and all left are just full sectors.
> +		 * Note: it's ok to round size down because all not full
> +		 * sectors are accounted by the first term.
> +		 */
> +		max_sectors = bio_segs * 2;
> +		max_sectors += bio->bi_iter.bi_size >> 9;
> +
> +		if (max_sectors < q_max_sectors) {

I don't think we need the max_sectors variable here.

Re: [RFC 2/2] block: add a fast path for seg split of large bio

[Pavel Begunkov]

On 27/01/2021 17:16, Christoph Hellwig wrote:
> On Tue, Jan 05, 2021 at 07:43:38PM +0000, Pavel Begunkov wrote:
>> blk_bio_segment_split() is very heavy, but the current fast path covers
>> only one-segment under PAGE_SIZE bios. Add another one by estimating an
>> upper bound of sectors a bio can contain.
>>
>> One restricting factor here is queue_max_segment_size(), which it
>> compare against full iter size to not dig into bvecs. By default it's
>> 64KB, and so for requests under 64KB, but for those falling under the
>> conditions it's much faster.
> 
> I think this works, but it is a pretty gross heuristic, which also

bio->bi_iter.bi_size <= queue_max_segment_size(q)

Do you mean this, right? I wouldn't say it's gross, but _very_ loose.

> doesn't help us with NVMe, which is the I/O fast path of choice for
> most people.  What is your use/test case?

Yeah, the idea to make it work for NVMe. Don't remember it restricting
segment size or whatever, maybe only atomicity. Which condition do you
see problematic? I can't recall without opening the spec.

> 
>> +		/*
>> +		 * Segments are contiguous, so only their ends may be not full.
>> +		 * An upper bound for them would to assume that each takes 1B
>> +		 * but adds a sector, and all left are just full sectors.
>> +		 * Note: it's ok to round size down because all not full
>> +		 * sectors are accounted by the first term.
>> +		 */
>> +		max_sectors = bio_segs * 2;
>> +		max_sectors += bio->bi_iter.bi_size >> 9;
>> +
>> +		if (max_sectors < q_max_sectors) {
> 
> I don't think we need the max_sectors variable here.

Even more, considering that it's already sector aligned we can kill off
all this section and just use (bi_iter.bi_size >> 9).

-- 
Pavel Begunkov

Re: [RFC 2/2] block: add a fast path for seg split of large bio

[Ming Lei]

On Tue, Jan 05, 2021 at 07:43:38PM +0000, Pavel Begunkov wrote:
> blk_bio_segment_split() is very heavy, but the current fast path covers
> only one-segment under PAGE_SIZE bios. Add another one by estimating an
> upper bound of sectors a bio can contain.
> 
> One restricting factor here is queue_max_segment_size(), which it
> compare against full iter size to not dig into bvecs. By default it's
> 64KB, and so for requests under 64KB, but for those falling under the
> conditions it's much faster.
> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  block/blk-merge.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 84b9635b5d57..15d75f3ffc30 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -226,12 +226,12 @@ static bool bvec_split_segs(const struct request_queue *q,
>  static struct bio *__blk_bio_segment_split(struct request_queue *q,
>  					   struct bio *bio,
>  					   struct bio_set *bs,
> -					   unsigned *segs)
> +					   unsigned *segs,
> +					   const unsigned max_sectors)
>  {
>  	struct bio_vec bv, bvprv, *bvprvp = NULL;
>  	struct bvec_iter iter;
>  	unsigned nsegs = 0, sectors = 0;
> -	const unsigned max_sectors = get_max_io_size(q, bio);
>  	const unsigned max_segs = queue_max_segments(q);
>  
>  	bio_for_each_bvec(bv, bio, iter) {
> @@ -295,6 +295,9 @@ static inline struct bio *blk_bio_segment_split(struct request_queue *q,
>  						struct bio_set *bs,
>  						unsigned *nr_segs)
>  {
> +	unsigned int max_sectors, q_max_sectors;
> +	unsigned int bio_segs = bio->bi_vcnt;
> +
>  	/*
>  	 * All drivers must accept single-segments bios that are <=
>  	 * PAGE_SIZE.  This is a quick and dirty check that relies on
> @@ -303,14 +306,32 @@ static inline struct bio *blk_bio_segment_split(struct request_queue *q,
>  	 * are cloned, but compared to the performance impact of cloned
>  	 * bios themselves the loop below doesn't matter anyway.
>  	 */
> -	if (!q->limits.chunk_sectors && bio->bi_vcnt == 1 &&
> +	if (!q->limits.chunk_sectors && bio_segs == 1 &&
>  	    (bio->bi_io_vec[0].bv_len +
>  	     bio->bi_io_vec[0].bv_offset) <= PAGE_SIZE) {
>  		*nr_segs = 1;
>  		return NULL;
>  	}
>  
> -	return __blk_bio_segment_split(q, bio, bs, nr_segs);
> +	q_max_sectors = get_max_io_size(q, bio);
> +	if (!queue_virt_boundary(q) && bio_segs < queue_max_segments(q) &&
> +	    bio->bi_iter.bi_size <= queue_max_segment_size(q)) {

.bi_vcnt is 0 for fast cloned bio, so the above check may become true
when real nr_segment is > queue_max_segments(), especially in case that
max segments limit is small and segment size is big.


-- 
Ming

Re: [RFC 2/2] block: add a fast path for seg split of large bio

[Pavel Begunkov]

On 28/01/2021 12:10, Ming Lei wrote:
> On Tue, Jan 05, 2021 at 07:43:38PM +0000, Pavel Begunkov wrote:
>> blk_bio_segment_split() is very heavy, but the current fast path covers
>> only one-segment under PAGE_SIZE bios. Add another one by estimating an
>> upper bound of sectors a bio can contain.
>>
>> One restricting factor here is queue_max_segment_size(), which it
>> compare against full iter size to not dig into bvecs. By default it's
>> 64KB, and so for requests under 64KB, but for those falling under the
>> conditions it's much faster.
>>
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>  block/blk-merge.c | 29 +++++++++++++++++++++++++----
>>  1 file changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/blk-merge.c b/block/blk-merge.c
>> index 84b9635b5d57..15d75f3ffc30 100644
>> --- a/block/blk-merge.c
>> +++ b/block/blk-merge.c
>> @@ -226,12 +226,12 @@ static bool bvec_split_segs(const struct request_queue *q,
>>  static struct bio *__blk_bio_segment_split(struct request_queue *q,
>>  					   struct bio *bio,
>>  					   struct bio_set *bs,
>> -					   unsigned *segs)
>> +					   unsigned *segs,
>> +					   const unsigned max_sectors)
>>  {
>>  	struct bio_vec bv, bvprv, *bvprvp = NULL;
>>  	struct bvec_iter iter;
>>  	unsigned nsegs = 0, sectors = 0;
>> -	const unsigned max_sectors = get_max_io_size(q, bio);
>>  	const unsigned max_segs = queue_max_segments(q);
>>  
>>  	bio_for_each_bvec(bv, bio, iter) {
>> @@ -295,6 +295,9 @@ static inline struct bio *blk_bio_segment_split(struct request_queue *q,
>>  						struct bio_set *bs,
>>  						unsigned *nr_segs)
>>  {
>> +	unsigned int max_sectors, q_max_sectors;
>> +	unsigned int bio_segs = bio->bi_vcnt;
>> +
>>  	/*
>>  	 * All drivers must accept single-segments bios that are <=
>>  	 * PAGE_SIZE.  This is a quick and dirty check that relies on
>> @@ -303,14 +306,32 @@ static inline struct bio *blk_bio_segment_split(struct request_queue *q,
>>  	 * are cloned, but compared to the performance impact of cloned
>>  	 * bios themselves the loop below doesn't matter anyway.
>>  	 */
>> -	if (!q->limits.chunk_sectors && bio->bi_vcnt == 1 &&
>> +	if (!q->limits.chunk_sectors && bio_segs == 1 &&
>>  	    (bio->bi_io_vec[0].bv_len +
>>  	     bio->bi_io_vec[0].bv_offset) <= PAGE_SIZE) {
>>  		*nr_segs = 1;
>>  		return NULL;
>>  	}
>>  
>> -	return __blk_bio_segment_split(q, bio, bs, nr_segs);
>> +	q_max_sectors = get_max_io_size(q, bio);
>> +	if (!queue_virt_boundary(q) && bio_segs < queue_max_segments(q) &&
>> +	    bio->bi_iter.bi_size <= queue_max_segment_size(q)) {
> 
> .bi_vcnt is 0 for fast cloned bio, so the above check may become true
> when real nr_segment is > queue_max_segments(), especially in case that
> max segments limit is small and segment size is big.

I guess we can skip the fast path for them (i.e. bi_vcnt == 0)

I'm curious, why it's 0 but not the real number?

-- 
Pavel Begunkov

Re: [RFC 2/2] block: add a fast path for seg split of large bio

[Ming Lei]

On Thu, Jan 28, 2021 at 12:27:39PM +0000, Pavel Begunkov wrote:
> On 28/01/2021 12:10, Ming Lei wrote:
> > On Tue, Jan 05, 2021 at 07:43:38PM +0000, Pavel Begunkov wrote:
> >> blk_bio_segment_split() is very heavy, but the current fast path covers
> >> only one-segment under PAGE_SIZE bios. Add another one by estimating an
> >> upper bound of sectors a bio can contain.
> >>
> >> One restricting factor here is queue_max_segment_size(), which it
> >> compare against full iter size to not dig into bvecs. By default it's
> >> 64KB, and so for requests under 64KB, but for those falling under the
> >> conditions it's much faster.
> >>
> >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> >> ---
> >>  block/blk-merge.c | 29 +++++++++++++++++++++++++----
> >>  1 file changed, 25 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/block/blk-merge.c b/block/blk-merge.c
> >> index 84b9635b5d57..15d75f3ffc30 100644
> >> --- a/block/blk-merge.c
> >> +++ b/block/blk-merge.c
> >> @@ -226,12 +226,12 @@ static bool bvec_split_segs(const struct request_queue *q,
> >>  static struct bio *__blk_bio_segment_split(struct request_queue *q,
> >>  					   struct bio *bio,
> >>  					   struct bio_set *bs,
> >> -					   unsigned *segs)
> >> +					   unsigned *segs,
> >> +					   const unsigned max_sectors)
> >>  {
> >>  	struct bio_vec bv, bvprv, *bvprvp = NULL;
> >>  	struct bvec_iter iter;
> >>  	unsigned nsegs = 0, sectors = 0;
> >> -	const unsigned max_sectors = get_max_io_size(q, bio);
> >>  	const unsigned max_segs = queue_max_segments(q);
> >>  
> >>  	bio_for_each_bvec(bv, bio, iter) {
> >> @@ -295,6 +295,9 @@ static inline struct bio *blk_bio_segment_split(struct request_queue *q,
> >>  						struct bio_set *bs,
> >>  						unsigned *nr_segs)
> >>  {
> >> +	unsigned int max_sectors, q_max_sectors;
> >> +	unsigned int bio_segs = bio->bi_vcnt;
> >> +
> >>  	/*
> >>  	 * All drivers must accept single-segments bios that are <=
> >>  	 * PAGE_SIZE.  This is a quick and dirty check that relies on
> >> @@ -303,14 +306,32 @@ static inline struct bio *blk_bio_segment_split(struct request_queue *q,
> >>  	 * are cloned, but compared to the performance impact of cloned
> >>  	 * bios themselves the loop below doesn't matter anyway.
> >>  	 */
> >> -	if (!q->limits.chunk_sectors && bio->bi_vcnt == 1 &&
> >> +	if (!q->limits.chunk_sectors && bio_segs == 1 &&
> >>  	    (bio->bi_io_vec[0].bv_len +
> >>  	     bio->bi_io_vec[0].bv_offset) <= PAGE_SIZE) {
> >>  		*nr_segs = 1;
> >>  		return NULL;
> >>  	}
> >>  
> >> -	return __blk_bio_segment_split(q, bio, bs, nr_segs);
> >> +	q_max_sectors = get_max_io_size(q, bio);
> >> +	if (!queue_virt_boundary(q) && bio_segs < queue_max_segments(q) &&
> >> +	    bio->bi_iter.bi_size <= queue_max_segment_size(q)) {
> > 
> > .bi_vcnt is 0 for fast cloned bio, so the above check may become true
> > when real nr_segment is > queue_max_segments(), especially in case that
> > max segments limit is small and segment size is big.
> 
> I guess we can skip the fast path for them (i.e. bi_vcnt == 0)

But bi_vcnt can't represent real segment number, which can be bigger or
less than .bi_vcnt.

> I'm curious, why it's 0 but not the real number?

fast-cloned bio shares bvec table of original bio, so it doesn't have
.bi_vcnt.


-- 
Ming

Re: [RFC 2/2] block: add a fast path for seg split of large bio

[Pavel Begunkov]

On 29/01/2021 02:00, Ming Lei wrote:
> On Thu, Jan 28, 2021 at 12:27:39PM +0000, Pavel Begunkov wrote:
>> On 28/01/2021 12:10, Ming Lei wrote:
>>> On Tue, Jan 05, 2021 at 07:43:38PM +0000, Pavel Begunkov wrote:
>>> .bi_vcnt is 0 for fast cloned bio, so the above check may become true
>>> when real nr_segment is > queue_max_segments(), especially in case that
>>> max segments limit is small and segment size is big.
>>
>> I guess we can skip the fast path for them (i.e. bi_vcnt == 0)
> 
> But bi_vcnt can't represent real segment number, which can be bigger or
> less than .bi_vcnt.

Sounds like "go slow path on bi_vcnt==0" won't work. Ok, I need to
go look what is the real purpose of .bi_vcnt

> 
>> I'm curious, why it's 0 but not the real number?
> 
> fast-cloned bio shares bvec table of original bio, so it doesn't have
> .bi_vcnt.

Got it, thanks


-- 
Pavel Begunkov