[PATCH RFC v2 01/10] block: factor out a helper bio_submit_split_bioset()

Yu Kuai posted 10 patches 1 month ago
There is a newer version of this series
[PATCH RFC v2 01/10] block: factor out a helper bio_submit_split_bioset()
Posted by Yu Kuai 1 month ago
From: Yu Kuai <yukuai3@huawei.com>

No functional changes are intended, some drivers like mdraid will split
bio by internal processing, prepare to unify bio split codes.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 block/blk-merge.c      | 63 ++++++++++++++++++++++++++++--------------
 include/linux/blkdev.h |  2 ++
 2 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 70d704615be5..3d6dc9cc4f61 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -104,34 +104,55 @@ static unsigned int bio_allowed_max_sectors(const struct queue_limits *lim)
 	return round_down(UINT_MAX, lim->logical_block_size) >> SECTOR_SHIFT;
 }
 
+/**
+ * bio_submit_split_bioset - Submit a bio, splitting it at a designated sector
+ * @bio:		the original bio to be submitted and split
+ * @split_sectors:	the sector count at which to split
+ * @bs:			the bio set used for allocating the new split bio
+ *
+ * The original bio is modified to contain the remaining sectors and submitted.
+ * The caller is responsible for submitting the returned bio.
+ *
+ * If succeed, the newly allocated bio representing the initial part will be
+ * returned, on failure NULL will be returned and original bio will fail.
+ */
+struct bio *bio_submit_split_bioset(struct bio *bio, int split_sectors,
+				    struct bio_set *bs)
+{
+	struct bio *split = bio_split(bio, split_sectors, GFP_NOIO, bs);
+
+	if (IS_ERR(split)) {
+		bio->bi_status = errno_to_blk_status(PTR_ERR(split));
+		bio_endio(bio);
+		return NULL;
+	}
+
+	blkcg_bio_issue_init(split);
+	bio_chain(split, bio);
+	trace_block_split(split, bio->bi_iter.bi_sector);
+	WARN_ON_ONCE(bio_zone_write_plugging(bio));
+	submit_bio_noacct(bio);
+
+	return split;
+}
+EXPORT_SYMBOL_GPL(bio_submit_split_bioset);
+
 static struct bio *bio_submit_split(struct bio *bio, int split_sectors)
 {
-	if (unlikely(split_sectors < 0))
-		goto error;
+	if (unlikely(split_sectors < 0)) {
+		bio->bi_status = errno_to_blk_status(split_sectors);
+		bio_endio(bio);
+		return NULL;
+	}
 
 	if (split_sectors) {
-		struct bio *split;
-
-		split = bio_split(bio, split_sectors, GFP_NOIO,
-				&bio->bi_bdev->bd_disk->bio_split);
-		if (IS_ERR(split)) {
-			split_sectors = PTR_ERR(split);
-			goto error;
-		}
-		split->bi_opf |= REQ_NOMERGE;
-		blkcg_bio_issue_init(split);
-		bio_chain(split, bio);
-		trace_block_split(split, bio->bi_iter.bi_sector);
-		WARN_ON_ONCE(bio_zone_write_plugging(bio));
-		submit_bio_noacct(bio);
-		return split;
+		bio = bio_submit_split_bioset(bio, split_sectors,
+					 &bio->bi_bdev->bd_disk->bio_split);
+		if (bio)
+			bio->bi_opf |= REQ_NOMERGE;
 	}
 
 	return bio;
-error:
-	bio->bi_status = errno_to_blk_status(split_sectors);
-	bio_endio(bio);
-	return NULL;
 }
 
 struct bio *bio_split_discard(struct bio *bio, const struct queue_limits *lim,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index fe1797bbec42..be4b3adf3989 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -999,6 +999,8 @@ extern int blk_register_queue(struct gendisk *disk);
 extern void blk_unregister_queue(struct gendisk *disk);
 void submit_bio_noacct(struct bio *bio);
 struct bio *bio_split_to_limits(struct bio *bio);
+struct bio *bio_submit_split_bioset(struct bio *bio, int split_sectors,
+				    struct bio_set *bs);
 
 extern int blk_lld_busy(struct request_queue *q);
 extern int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags);
-- 
2.39.2
Re: [PATCH RFC v2 01/10] block: factor out a helper bio_submit_split_bioset()
Posted by Damien Le Moal 1 month ago
On 8/28/25 15:57, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> No functional changes are intended, some drivers like mdraid will split
> bio by internal processing, prepare to unify bio split codes.
> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>

Looks good to me. A few nits below.

> ---
>  block/blk-merge.c      | 63 ++++++++++++++++++++++++++++--------------
>  include/linux/blkdev.h |  2 ++
>  2 files changed, 44 insertions(+), 21 deletions(-)
> 
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 70d704615be5..3d6dc9cc4f61 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -104,34 +104,55 @@ static unsigned int bio_allowed_max_sectors(const struct queue_limits *lim)
>  	return round_down(UINT_MAX, lim->logical_block_size) >> SECTOR_SHIFT;
>  }
>  
> +/**
> + * bio_submit_split_bioset - Submit a bio, splitting it at a designated sector
> + * @bio:		the original bio to be submitted and split
> + * @split_sectors:	the sector count at which to split
> + * @bs:			the bio set used for allocating the new split bio
> + *
> + * The original bio is modified to contain the remaining sectors and submitted.
> + * The caller is responsible for submitting the returned bio.
> + *
> + * If succeed, the newly allocated bio representing the initial part will be
> + * returned, on failure NULL will be returned and original bio will fail.
> + */
> +struct bio *bio_submit_split_bioset(struct bio *bio, int split_sectors,
> +				    struct bio_set *bs)

While at it, it would be nice to have split_sectors be unsigned. That would
avoid the check in bio_submit_split().

> +{
> +	struct bio *split = bio_split(bio, split_sectors, GFP_NOIO, bs);
> +
> +	if (IS_ERR(split)) {
> +		bio->bi_status = errno_to_blk_status(PTR_ERR(split));
> +		bio_endio(bio);
> +		return NULL;
> +	}
> +
> +	blkcg_bio_issue_init(split);
> +	bio_chain(split, bio);
> +	trace_block_split(split, bio->bi_iter.bi_sector);
> +	WARN_ON_ONCE(bio_zone_write_plugging(bio));
> +	submit_bio_noacct(bio);
> +
> +	return split;
> +}
> +EXPORT_SYMBOL_GPL(bio_submit_split_bioset);
> +
>  static struct bio *bio_submit_split(struct bio *bio, int split_sectors)
>  {
> -	if (unlikely(split_sectors < 0))
> -		goto error;
> +	if (unlikely(split_sectors < 0)) {
> +		bio->bi_status = errno_to_blk_status(split_sectors);
> +		bio_endio(bio);
> +		return NULL;
> +	}

See above.

>  
>  	if (split_sectors) {
> -		struct bio *split;
> -
> -		split = bio_split(bio, split_sectors, GFP_NOIO,
> -				&bio->bi_bdev->bd_disk->bio_split);
> -		if (IS_ERR(split)) {
> -			split_sectors = PTR_ERR(split);
> -			goto error;
> -		}
> -		split->bi_opf |= REQ_NOMERGE;
> -		blkcg_bio_issue_init(split);
> -		bio_chain(split, bio);
> -		trace_block_split(split, bio->bi_iter.bi_sector);
> -		WARN_ON_ONCE(bio_zone_write_plugging(bio));
> -		submit_bio_noacct(bio);
> -		return split;
> +		bio = bio_submit_split_bioset(bio, split_sectors,
> +					 &bio->bi_bdev->bd_disk->bio_split);
> +		if (bio)
> +			bio->bi_opf |= REQ_NOMERGE;

I think that setting REQ_NOMERGE should be done in bio_submit_split_bioset().

>  	}
>  
>  	return bio;
> -error:
> -	bio->bi_status = errno_to_blk_status(split_sectors);
> -	bio_endio(bio);
> -	return NULL;
>  }
>  
>  struct bio *bio_split_discard(struct bio *bio, const struct queue_limits *lim,
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index fe1797bbec42..be4b3adf3989 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -999,6 +999,8 @@ extern int blk_register_queue(struct gendisk *disk);
>  extern void blk_unregister_queue(struct gendisk *disk);
>  void submit_bio_noacct(struct bio *bio);
>  struct bio *bio_split_to_limits(struct bio *bio);
> +struct bio *bio_submit_split_bioset(struct bio *bio, int split_sectors,
> +				    struct bio_set *bs);
>  
>  extern int blk_lld_busy(struct request_queue *q);
>  extern int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags);


-- 
Damien Le Moal
Western Digital Research
Re: [PATCH RFC v2 01/10] block: factor out a helper bio_submit_split_bioset()
Posted by Yu Kuai 1 month ago
Hi,

在 2025/8/30 8:37, Damien Le Moal 写道:
> On 8/28/25 15:57, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> No functional changes are intended, some drivers like mdraid will split
>> bio by internal processing, prepare to unify bio split codes.
>>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> Looks good to me. A few nits below.
>
>> ---
>>   block/blk-merge.c      | 63 ++++++++++++++++++++++++++++--------------
>>   include/linux/blkdev.h |  2 ++
>>   2 files changed, 44 insertions(+), 21 deletions(-)
>>
>> diff --git a/block/blk-merge.c b/block/blk-merge.c
>> index 70d704615be5..3d6dc9cc4f61 100644
>> --- a/block/blk-merge.c
>> +++ b/block/blk-merge.c
>> @@ -104,34 +104,55 @@ static unsigned int bio_allowed_max_sectors(const struct queue_limits *lim)
>>   	return round_down(UINT_MAX, lim->logical_block_size) >> SECTOR_SHIFT;
>>   }
>>   
>> +/**
>> + * bio_submit_split_bioset - Submit a bio, splitting it at a designated sector
>> + * @bio:		the original bio to be submitted and split
>> + * @split_sectors:	the sector count at which to split
>> + * @bs:			the bio set used for allocating the new split bio
>> + *
>> + * The original bio is modified to contain the remaining sectors and submitted.
>> + * The caller is responsible for submitting the returned bio.
>> + *
>> + * If succeed, the newly allocated bio representing the initial part will be
>> + * returned, on failure NULL will be returned and original bio will fail.
>> + */
>> +struct bio *bio_submit_split_bioset(struct bio *bio, int split_sectors,
>> +				    struct bio_set *bs)
> While at it, it would be nice to have split_sectors be unsigned. That would
> avoid the check in bio_submit_split().

Sure, I can change this to unsigned. However, there are caller return error
code and bio_submit_split() will handler the error code, if we want to avoid
the check here, we'll have to hande error before bio_submit_split() as well,
is this what you mean?

>
>> +{
>> +	struct bio *split = bio_split(bio, split_sectors, GFP_NOIO, bs);
>> +
>> +	if (IS_ERR(split)) {
>> +		bio->bi_status = errno_to_blk_status(PTR_ERR(split));
>> +		bio_endio(bio);
>> +		return NULL;
>> +	}
>> +
>> +	blkcg_bio_issue_init(split);
>> +	bio_chain(split, bio);
>> +	trace_block_split(split, bio->bi_iter.bi_sector);
>> +	WARN_ON_ONCE(bio_zone_write_plugging(bio));
>> +	submit_bio_noacct(bio);
>> +
>> +	return split;
>> +}
>> +EXPORT_SYMBOL_GPL(bio_submit_split_bioset);
>> +
>>   static struct bio *bio_submit_split(struct bio *bio, int split_sectors)
>>   {
>> -	if (unlikely(split_sectors < 0))
>> -		goto error;
>> +	if (unlikely(split_sectors < 0)) {
>> +		bio->bi_status = errno_to_blk_status(split_sectors);
>> +		bio_endio(bio);
>> +		return NULL;
>> +	}
> See above.
>
>>   
>>   	if (split_sectors) {
>> -		struct bio *split;
>> -
>> -		split = bio_split(bio, split_sectors, GFP_NOIO,
>> -				&bio->bi_bdev->bd_disk->bio_split);
>> -		if (IS_ERR(split)) {
>> -			split_sectors = PTR_ERR(split);
>> -			goto error;
>> -		}
>> -		split->bi_opf |= REQ_NOMERGE;
>> -		blkcg_bio_issue_init(split);
>> -		bio_chain(split, bio);
>> -		trace_block_split(split, bio->bi_iter.bi_sector);
>> -		WARN_ON_ONCE(bio_zone_write_plugging(bio));
>> -		submit_bio_noacct(bio);
>> -		return split;
>> +		bio = bio_submit_split_bioset(bio, split_sectors,
>> +					 &bio->bi_bdev->bd_disk->bio_split);
>> +		if (bio)
>> +			bio->bi_opf |= REQ_NOMERGE;
> I think that setting REQ_NOMERGE should be done in bio_submit_split_bioset().

In mdraid, we'll expect bio still can be merged in underlying disks after split
by chunksize, and setting it and then clear it in mdraid is not necessary, I think
this is better.

Thanks,
Kuai

>>   	}
>>   
>>   	return bio;
>> -error:
>> -	bio->bi_status = errno_to_blk_status(split_sectors);
>> -	bio_endio(bio);
>> -	return NULL;
>>   }
>>   
>>   struct bio *bio_split_discard(struct bio *bio, const struct queue_limits *lim,
>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>> index fe1797bbec42..be4b3adf3989 100644
>> --- a/include/linux/blkdev.h
>> +++ b/include/linux/blkdev.h
>> @@ -999,6 +999,8 @@ extern int blk_register_queue(struct gendisk *disk);
>>   extern void blk_unregister_queue(struct gendisk *disk);
>>   void submit_bio_noacct(struct bio *bio);
>>   struct bio *bio_split_to_limits(struct bio *bio);
>> +struct bio *bio_submit_split_bioset(struct bio *bio, int split_sectors,
>> +				    struct bio_set *bs);
>>   
>>   extern int blk_lld_busy(struct request_queue *q);
>>   extern int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags);
>