On 8/28/25 15:57, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
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
