public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
* bio splitting cleanups
@ 2022-07-20 14:24 Christoph Hellwig
  2022-07-20 14:24 ` [PATCH 1/5] block: move ->bio_split to the gendisk Christoph Hellwig
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Christoph Hellwig @ 2022-07-20 14:24 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block

Hi Jens,

this series has two parts:  the first patch moves the ->bio_split
bio_set to the gendisk as it only is used for file system style I/O.

The other patches reshuffle the bio splitting code so that in the future
blk_bio_segment_split can be used to split REQ_OP_ZONE_APPEND bios
under file system / remapping driver control.  I plan to use that in
btrfs in the next merge window.

Diffstat:
 block/bio-integrity.c  |    2 
 block/bio.c            |    2 
 block/blk-core.c       |    9 ---
 block/blk-merge.c      |  139 ++++++++++++++++++++++++-------------------------
 block/blk-mq.c         |    4 -
 block/blk-settings.c   |    2 
 block/blk-sysfs.c      |    2 
 block/blk.h            |   34 ++++-------
 block/genhd.c          |    9 ++-
 drivers/md/dm.c        |    2 
 include/linux/blkdev.h |    3 -
 11 files changed, 100 insertions(+), 108 deletions(-)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/5] block: move ->bio_split to the gendisk
  2022-07-20 14:24 bio splitting cleanups Christoph Hellwig
@ 2022-07-20 14:24 ` Christoph Hellwig
  2022-07-22  5:56   ` Damien Le Moal
  2022-07-20 14:24 ` [PATCH 2/5] block: fix max_zone_append_sectors inheritance in blk_stack_limits Christoph Hellwig
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2022-07-20 14:24 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block

Only non-passthrough requests are split by the block layer and use the
->bio_split bio_set.  Move it from the request_queue to the gendisk.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c       |  9 +--------
 block/blk-merge.c      | 20 ++++++++++----------
 block/blk-sysfs.c      |  2 --
 block/genhd.c          |  9 ++++++++-
 drivers/md/dm.c        |  2 +-
 include/linux/blkdev.h |  3 ++-
 6 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 123468b9d2e43..59f13d011949d 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -377,7 +377,6 @@ static void blk_timeout_work(struct work_struct *work)
 struct request_queue *blk_alloc_queue(int node_id, bool alloc_srcu)
 {
 	struct request_queue *q;
-	int ret;
 
 	q = kmem_cache_alloc_node(blk_get_queue_kmem_cache(alloc_srcu),
 			GFP_KERNEL | __GFP_ZERO, node_id);
@@ -396,13 +395,9 @@ struct request_queue *blk_alloc_queue(int node_id, bool alloc_srcu)
 	if (q->id < 0)
 		goto fail_srcu;
 
-	ret = bioset_init(&q->bio_split, BIO_POOL_SIZE, 0, 0);
-	if (ret)
-		goto fail_id;
-
 	q->stats = blk_alloc_queue_stats();
 	if (!q->stats)
-		goto fail_split;
+		goto fail_id;
 
 	q->node = node_id;
 
@@ -439,8 +434,6 @@ struct request_queue *blk_alloc_queue(int node_id, bool alloc_srcu)
 
 fail_stats:
 	blk_free_queue_stats(q->stats);
-fail_split:
-	bioset_exit(&q->bio_split);
 fail_id:
 	ida_free(&blk_queue_ida, q->id);
 fail_srcu:
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 3c3f785f558af..e657f1dc824cb 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -328,26 +328,26 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
  * Split a bio into two bios, chain the two bios, submit the second half and
  * store a pointer to the first half in *@bio. If the second bio is still too
  * big it will be split by a recursive call to this function. Since this
- * function may allocate a new bio from q->bio_split, it is the responsibility
- * of the caller to ensure that q->bio_split is only released after processing
- * of the split bio has finished.
+ * function may allocate a new bio from disk->bio_split, it is the
+ * responsibility of the caller to ensure that disk->bio_split is only released
+ * after processing of the split bio has finished.
  */
 void __blk_queue_split(struct request_queue *q, struct bio **bio,
 		       unsigned int *nr_segs)
 {
+	struct bio_set *bs = &(*bio)->bi_bdev->bd_disk->bio_split;
 	struct bio *split = NULL;
 
 	switch (bio_op(*bio)) {
 	case REQ_OP_DISCARD:
 	case REQ_OP_SECURE_ERASE:
-		split = blk_bio_discard_split(q, *bio, &q->bio_split, nr_segs);
+		split = blk_bio_discard_split(q, *bio, bs, nr_segs);
 		break;
 	case REQ_OP_WRITE_ZEROES:
-		split = blk_bio_write_zeroes_split(q, *bio, &q->bio_split,
-				nr_segs);
+		split = blk_bio_write_zeroes_split(q, *bio, bs, nr_segs);
 		break;
 	default:
-		split = blk_bio_segment_split(q, *bio, &q->bio_split, nr_segs);
+		split = blk_bio_segment_split(q, *bio, bs, nr_segs);
 		break;
 	}
 
@@ -368,9 +368,9 @@ void __blk_queue_split(struct request_queue *q, struct bio **bio,
  *
  * Split a bio into two bios, chains the two bios, submit the second half and
  * store a pointer to the first half in *@bio. Since this function may allocate
- * a new bio from q->bio_split, it is the responsibility of the caller to ensure
- * that q->bio_split is only released after processing of the split bio has
- * finished.
+ * a new bio from disk->bio_split, it is the responsibility of the caller to
+ * ensure that disk->bio_split is only released after processing of the split
+ * bio has finished.
  */
 void blk_queue_split(struct bio **bio)
 {
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index c0303026752d5..e1f009aba6fd2 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -779,8 +779,6 @@ static void blk_release_queue(struct kobject *kobj)
 	if (queue_is_mq(q))
 		blk_mq_release(q);
 
-	bioset_exit(&q->bio_split);
-
 	if (blk_queue_has_srcu(q))
 		cleanup_srcu_struct(q->srcu);
 
diff --git a/block/genhd.c b/block/genhd.c
index 44dfcf67ed96a..150494e8742b0 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1138,6 +1138,8 @@ static void disk_release(struct device *dev)
 	might_sleep();
 	WARN_ON_ONCE(disk_live(disk));
 
+	bioset_exit(&disk->bio_split);
+
 	blkcg_exit_queue(disk->queue);
 
 	disk_release_events(disk);
@@ -1330,9 +1332,12 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 	if (!disk)
 		goto out_put_queue;
 
+	if (bioset_init(&disk->bio_split, BIO_POOL_SIZE, 0, 0))
+		goto out_free_disk;
+
 	disk->bdi = bdi_alloc(node_id);
 	if (!disk->bdi)
-		goto out_free_disk;
+		goto out_free_bioset;
 
 	/* bdev_alloc() might need the queue, set before the first call */
 	disk->queue = q;
@@ -1370,6 +1375,8 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 	iput(disk->part0->bd_inode);
 out_free_bdi:
 	bdi_put(disk->bdi);
+out_free_bioset:
+	bioset_exit(&disk->bio_split);
 out_free_disk:
 	kfree(disk);
 out_put_queue:
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 54c2a23f4e55c..b163de50f3c6b 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1693,7 +1693,7 @@ static void dm_split_and_process_bio(struct mapped_device *md,
 	 */
 	WARN_ON_ONCE(!dm_io_flagged(io, DM_IO_WAS_SPLIT));
 	io->split_bio = bio_split(bio, io->sectors, GFP_NOIO,
-				  &md->queue->bio_split);
+				  &md->disk->bio_split);
 	bio_chain(io->split_bio, bio);
 	trace_block_split(io->split_bio, bio->bi_iter.bi_sector);
 	submit_bio_noacct(bio);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d04bdf549efa9..97d3ef8f3f416 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -140,6 +140,8 @@ struct gendisk {
 	struct request_queue *queue;
 	void *private_data;
 
+	struct bio_set bio_split;
+
 	int flags;
 	unsigned long state;
 #define GD_NEED_PART_SCAN		0
@@ -531,7 +533,6 @@ struct request_queue {
 
 	struct blk_mq_tag_set	*tag_set;
 	struct list_head	tag_set_list;
-	struct bio_set		bio_split;
 
 	struct dentry		*debugfs_dir;
 	struct dentry		*sched_debugfs_dir;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/5] block: fix max_zone_append_sectors inheritance in blk_stack_limits
  2022-07-20 14:24 bio splitting cleanups Christoph Hellwig
  2022-07-20 14:24 ` [PATCH 1/5] block: move ->bio_split to the gendisk Christoph Hellwig
@ 2022-07-20 14:24 ` Christoph Hellwig
  2022-07-22  5:59   ` Damien Le Moal
  2022-07-20 14:24 ` [PATCH 3/5] block: move the call to get_max_io_size out of blk_bio_segment_split Christoph Hellwig
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2022-07-20 14:24 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block

As we start out with a default of 0, this needs a min_not_zero to
actually work.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-settings.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 8bb9eef5310eb..9f6e271ca67f4 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -554,7 +554,7 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
 	t->max_dev_sectors = min_not_zero(t->max_dev_sectors, b->max_dev_sectors);
 	t->max_write_zeroes_sectors = min(t->max_write_zeroes_sectors,
 					b->max_write_zeroes_sectors);
-	t->max_zone_append_sectors = min(t->max_zone_append_sectors,
+	t->max_zone_append_sectors = min_not_zero(t->max_zone_append_sectors,
 					b->max_zone_append_sectors);
 	t->bounce = max(t->bounce, b->bounce);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/5] block: move the call to get_max_io_size out of blk_bio_segment_split
  2022-07-20 14:24 bio splitting cleanups Christoph Hellwig
  2022-07-20 14:24 ` [PATCH 1/5] block: move ->bio_split to the gendisk Christoph Hellwig
  2022-07-20 14:24 ` [PATCH 2/5] block: fix max_zone_append_sectors inheritance in blk_stack_limits Christoph Hellwig
@ 2022-07-20 14:24 ` Christoph Hellwig
  2022-07-22  6:09   ` Damien Le Moal
  2022-07-20 14:24 ` [PATCH 4/5] block: move bio_allowed_max_sectors to blk-merge.c Christoph Hellwig
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2022-07-20 14:24 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block

Prepare for reusing blk_bio_segment_split for (file system controlled)
splits of REQ_OP_ZONE_APPEND bios by letting the caller control the
maximum size of the bio.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-merge.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index e657f1dc824cb..1676a835b16e7 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -252,11 +252,12 @@ static bool bvec_split_segs(const struct request_queue *q,
  * @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
+ * @max_bytes: [in] maximum number of bytes per bio
  *
  * 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 @max_bytes worth of data
  * - 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
@@ -266,14 +267,12 @@ static bool bvec_split_segs(const struct request_queue *q,
  * split bio has finished.
  */
 static struct bio *blk_bio_segment_split(struct request_queue *q,
-					 struct bio *bio,
-					 struct bio_set *bs,
-					 unsigned *segs)
+		struct bio *bio, struct bio_set *bs, unsigned *segs,
+		unsigned max_bytes)
 {
 	struct bio_vec bv, bvprv, *bvprvp = NULL;
 	struct bvec_iter iter;
 	unsigned nsegs = 0, bytes = 0;
-	const unsigned max_bytes = get_max_io_size(q, bio) << 9;
 	const unsigned max_segs = queue_max_segments(q);
 
 	bio_for_each_bvec(bv, bio, iter) {
@@ -347,7 +346,8 @@ void __blk_queue_split(struct request_queue *q, struct bio **bio,
 		split = blk_bio_write_zeroes_split(q, *bio, bs, nr_segs);
 		break;
 	default:
-		split = blk_bio_segment_split(q, *bio, bs, nr_segs);
+		split = blk_bio_segment_split(q, *bio, bs, nr_segs,
+				get_max_io_size(q, *bio) << SECTOR_SHIFT);
 		break;
 	}
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/5] block: move bio_allowed_max_sectors to blk-merge.c
  2022-07-20 14:24 bio splitting cleanups Christoph Hellwig
                   ` (2 preceding siblings ...)
  2022-07-20 14:24 ` [PATCH 3/5] block: move the call to get_max_io_size out of blk_bio_segment_split Christoph Hellwig
@ 2022-07-20 14:24 ` Christoph Hellwig
  2022-07-21  6:31   ` Johannes Thumshirn
  2022-07-22  6:02   ` Damien Le Moal
  2022-07-20 14:24 ` [PATCH 5/5] block: pass struct queue_limits to the bio splitting helpers Christoph Hellwig
  2022-07-21  6:33 ` bio splitting cleanups Johannes Thumshirn
  5 siblings, 2 replies; 19+ messages in thread
From: Christoph Hellwig @ 2022-07-20 14:24 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block

Move this helper into the only file where it is used.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-merge.c | 10 ++++++++++
 block/blk.h       | 10 ----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 1676a835b16e7..9593a8a617292 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -95,6 +95,16 @@ static inline bool req_gap_front_merge(struct request *req, struct bio *bio)
 	return bio_will_gap(req->q, NULL, bio, req->bio);
 }
 
+/*
+ * The max size one bio can handle is UINT_MAX becasue bvec_iter.bi_size
+ * is defined as 'unsigned int', meantime it has to aligned to with logical
+ * block size which is the minimum accepted unit by hardware.
+ */
+static unsigned int bio_allowed_max_sectors(struct request_queue *q)
+{
+	return round_down(UINT_MAX, queue_logical_block_size(q)) >> 9;
+}
+
 static struct bio *blk_bio_discard_split(struct request_queue *q,
 					 struct bio *bio,
 					 struct bio_set *bs,
diff --git a/block/blk.h b/block/blk.h
index c4b084bfe87c9..3026ba81c85f0 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -349,16 +349,6 @@ static inline void req_set_nomerge(struct request_queue *q, struct request *req)
 		q->last_merge = NULL;
 }
 
-/*
- * The max size one bio can handle is UINT_MAX becasue bvec_iter.bi_size
- * is defined as 'unsigned int', meantime it has to aligned to with logical
- * block size which is the minimum accepted unit by hardware.
- */
-static inline unsigned int bio_allowed_max_sectors(struct request_queue *q)
-{
-	return round_down(UINT_MAX, queue_logical_block_size(q)) >> 9;
-}
-
 /*
  * Internal io_context interface
  */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/5] block: pass struct queue_limits to the bio splitting helpers
  2022-07-20 14:24 bio splitting cleanups Christoph Hellwig
                   ` (3 preceding siblings ...)
  2022-07-20 14:24 ` [PATCH 4/5] block: move bio_allowed_max_sectors to blk-merge.c Christoph Hellwig
@ 2022-07-20 14:24 ` Christoph Hellwig
  2022-07-22  6:08   ` Damien Le Moal
  2022-07-21  6:33 ` bio splitting cleanups Johannes Thumshirn
  5 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2022-07-20 14:24 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block

Allow using the splitting helpers on just a queue_limits instead of
a full request_queue structure.  This will eventuall allow file systems
or remapping drivers to split REQ_OP_ZONE_APPEND bios based on limits
calculated as the minimum common capabilities over multiple devices.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio-integrity.c |   2 +-
 block/bio.c           |   2 +-
 block/blk-merge.c     | 111 +++++++++++++++++++-----------------------
 block/blk-mq.c        |   4 +-
 block/blk.h           |  24 ++++-----
 5 files changed, 68 insertions(+), 75 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 32929c89ba8a6..3f5685c00e360 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -134,7 +134,7 @@ int bio_integrity_add_page(struct bio *bio, struct page *page,
 	iv = bip->bip_vec + bip->bip_vcnt;
 
 	if (bip->bip_vcnt &&
-	    bvec_gap_to_prev(bdev_get_queue(bio->bi_bdev),
+	    bvec_gap_to_prev(&bdev_get_queue(bio->bi_bdev)->limits,
 			     &bip->bip_vec[bip->bip_vcnt - 1], offset))
 		return 0;
 
diff --git a/block/bio.c b/block/bio.c
index 6f9f883f9a65a..0d866d44ce533 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -965,7 +965,7 @@ int bio_add_hw_page(struct request_queue *q, struct bio *bio,
 		 * would create a gap, disallow it.
 		 */
 		bvec = &bio->bi_io_vec[bio->bi_vcnt - 1];
-		if (bvec_gap_to_prev(q, bvec, offset))
+		if (bvec_gap_to_prev(&q->limits, bvec, offset))
 			return 0;
 	}
 
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 9593a8a617292..d8c99cc4ef362 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -82,7 +82,7 @@ static inline bool bio_will_gap(struct request_queue *q,
 	bio_get_first_bvec(next, &nb);
 	if (biovec_phys_mergeable(q, &pb, &nb))
 		return false;
-	return __bvec_gap_to_prev(q, &pb, nb.bv_offset);
+	return __bvec_gap_to_prev(&q->limits, &pb, nb.bv_offset);
 }
 
 static inline bool req_gap_back_merge(struct request *req, struct bio *bio)
@@ -100,28 +100,25 @@ static inline bool req_gap_front_merge(struct request *req, struct bio *bio)
  * is defined as 'unsigned int', meantime it has to aligned to with logical
  * block size which is the minimum accepted unit by hardware.
  */
-static unsigned int bio_allowed_max_sectors(struct request_queue *q)
+static unsigned int bio_allowed_max_sectors(struct queue_limits *lim)
 {
-	return round_down(UINT_MAX, queue_logical_block_size(q)) >> 9;
+	return round_down(UINT_MAX, lim->logical_block_size) >> SECTOR_SHIFT;
 }
 
-static struct bio *blk_bio_discard_split(struct request_queue *q,
-					 struct bio *bio,
-					 struct bio_set *bs,
-					 unsigned *nsegs)
+static struct bio *blk_bio_discard_split(struct queue_limits *lim,
+		struct bio *bio, struct bio_set *bs, unsigned *nsegs)
 {
 	unsigned int max_discard_sectors, granularity;
-	int alignment;
 	sector_t tmp;
 	unsigned split_sectors;
 
 	*nsegs = 1;
 
 	/* Zero-sector (unknown) and one-sector granularities are the same.  */
-	granularity = max(q->limits.discard_granularity >> 9, 1U);
+	granularity = max(lim->discard_granularity >> 9, 1U);
 
-	max_discard_sectors = min(q->limits.max_discard_sectors,
-			bio_allowed_max_sectors(q));
+	max_discard_sectors =
+		min(lim->max_discard_sectors, bio_allowed_max_sectors(lim));
 	max_discard_sectors -= max_discard_sectors % granularity;
 
 	if (unlikely(!max_discard_sectors)) {
@@ -138,9 +135,8 @@ static struct bio *blk_bio_discard_split(struct request_queue *q,
 	 * If the next starting sector would be misaligned, stop the discard at
 	 * the previous aligned sector.
 	 */
-	alignment = (q->limits.discard_alignment >> 9) % granularity;
-
-	tmp = bio->bi_iter.bi_sector + split_sectors - alignment;
+	tmp = bio->bi_iter.bi_sector + split_sectors -
+		((lim->discard_alignment >> 9) % granularity);
 	tmp = sector_div(tmp, granularity);
 
 	if (split_sectors > tmp)
@@ -149,18 +145,15 @@ static struct bio *blk_bio_discard_split(struct request_queue *q,
 	return bio_split(bio, split_sectors, GFP_NOIO, bs);
 }
 
-static struct bio *blk_bio_write_zeroes_split(struct request_queue *q,
+static struct bio *blk_bio_write_zeroes_split(struct queue_limits *lim,
 		struct bio *bio, struct bio_set *bs, unsigned *nsegs)
 {
 	*nsegs = 0;
-
-	if (!q->limits.max_write_zeroes_sectors)
+	if (!lim->max_write_zeroes_sectors)
 		return NULL;
-
-	if (bio_sectors(bio) <= q->limits.max_write_zeroes_sectors)
+	if (bio_sectors(bio) <= lim->max_write_zeroes_sectors)
 		return NULL;
-
-	return bio_split(bio, q->limits.max_write_zeroes_sectors, GFP_NOIO, bs);
+	return bio_split(bio, lim->max_write_zeroes_sectors, GFP_NOIO, bs);
 }
 
 /*
@@ -171,17 +164,17 @@ static struct bio *blk_bio_write_zeroes_split(struct request_queue *q,
  * requests that are submitted to a block device if the start of a bio is not
  * aligned to a physical block boundary.
  */
-static inline unsigned get_max_io_size(struct request_queue *q,
+static inline unsigned get_max_io_size(struct queue_limits *lim,
 				       struct bio *bio)
 {
-	unsigned pbs = queue_physical_block_size(q) >> SECTOR_SHIFT;
-	unsigned lbs = queue_logical_block_size(q) >> SECTOR_SHIFT;
-	unsigned max_sectors = queue_max_sectors(q), start, end;
+	unsigned pbs = lim->physical_block_size >> SECTOR_SHIFT;
+	unsigned lbs = lim->logical_block_size >> SECTOR_SHIFT;
+	unsigned max_sectors = lim->max_sectors, start, end;
 
-	if (q->limits.chunk_sectors) {
+	if (lim->chunk_sectors) {
 		max_sectors = min(max_sectors,
 			blk_chunk_sectors_left(bio->bi_iter.bi_sector,
-					       q->limits.chunk_sectors));
+					       lim->chunk_sectors));
 	}
 
 	start = bio->bi_iter.bi_sector & (pbs - 1);
@@ -191,11 +184,10 @@ static inline unsigned get_max_io_size(struct request_queue *q,
 	return max_sectors & ~(lbs - 1);
 }
 
-static inline unsigned get_max_segment_size(const struct request_queue *q,
-					    struct page *start_page,
-					    unsigned long offset)
+static inline unsigned get_max_segment_size(struct queue_limits *lim,
+		struct page *start_page, unsigned long offset)
 {
-	unsigned long mask = queue_segment_boundary(q);
+	unsigned long mask = lim->seg_boundary_mask;
 
 	offset = mask & (page_to_phys(start_page) + offset);
 
@@ -204,12 +196,12 @@ static inline unsigned get_max_segment_size(const struct request_queue *q,
 	 * on 32bit arch, use queue's max segment size when that happens.
 	 */
 	return min_not_zero(mask - offset + 1,
-			(unsigned long)queue_max_segment_size(q));
+			(unsigned long)lim->max_segment_size);
 }
 
 /**
  * bvec_split_segs - verify whether or not a bvec should be split in the middle
- * @q:        [in] request queue associated with the bio associated with @bv
+ * @lim:      [in] queue limits to split based on
  * @bv:       [in] bvec to examine
  * @nsegs:    [in,out] Number of segments in the bio being built. Incremented
  *            by the number of segments from @bv that may be appended to that
@@ -227,10 +219,9 @@ static inline unsigned get_max_segment_size(const struct request_queue *q,
  * *@nsegs segments and *@sectors sectors would make that bio unacceptable for
  * the block driver.
  */
-static bool bvec_split_segs(const struct request_queue *q,
-			    const struct bio_vec *bv, unsigned *nsegs,
-			    unsigned *bytes, unsigned max_segs,
-			    unsigned max_bytes)
+static bool bvec_split_segs(struct queue_limits *lim, const struct bio_vec *bv,
+		unsigned *nsegs, unsigned *bytes, unsigned max_segs,
+		unsigned max_bytes)
 {
 	unsigned max_len = min(max_bytes, UINT_MAX) - *bytes;
 	unsigned len = min(bv->bv_len, max_len);
@@ -238,7 +229,7 @@ static bool bvec_split_segs(const struct request_queue *q,
 	unsigned seg_size = 0;
 
 	while (len && *nsegs < max_segs) {
-		seg_size = get_max_segment_size(q, bv->bv_page,
+		seg_size = get_max_segment_size(lim, bv->bv_page,
 						bv->bv_offset + total_len);
 		seg_size = min(seg_size, len);
 
@@ -246,7 +237,7 @@ static bool bvec_split_segs(const struct request_queue *q,
 		total_len += seg_size;
 		len -= seg_size;
 
-		if ((bv->bv_offset + total_len) & queue_virt_boundary(q))
+		if ((bv->bv_offset + total_len) & lim->virt_boundary_mask)
 			break;
 	}
 
@@ -258,7 +249,7 @@ static bool bvec_split_segs(const struct request_queue *q,
 
 /**
  * blk_bio_segment_split - split a bio in two bios
- * @q:    [in] request queue pointer
+ * @lim:  [in] queue limits to split based on
  * @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
@@ -276,31 +267,31 @@ static bool bvec_split_segs(const struct request_queue *q,
  * 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,
+static struct bio *blk_bio_segment_split(struct queue_limits *lim,
 		struct bio *bio, struct bio_set *bs, unsigned *segs,
 		unsigned max_bytes)
 {
 	struct bio_vec bv, bvprv, *bvprvp = NULL;
 	struct bvec_iter iter;
 	unsigned nsegs = 0, bytes = 0;
-	const unsigned max_segs = queue_max_segments(q);
 
 	bio_for_each_bvec(bv, bio, iter) {
 		/*
 		 * If the queue doesn't support SG gaps and adding this
 		 * offset would create a gap, disallow it.
 		 */
-		if (bvprvp && bvec_gap_to_prev(q, bvprvp, bv.bv_offset))
+		if (bvprvp && bvec_gap_to_prev(lim, bvprvp, bv.bv_offset))
 			goto split;
 
-		if (nsegs < max_segs &&
+		if (nsegs < lim->max_segments &&
 		    bytes + bv.bv_len <= max_bytes &&
 		    bv.bv_offset + bv.bv_len <= PAGE_SIZE) {
 			nsegs++;
 			bytes += bv.bv_len;
-		} else if (bvec_split_segs(q, &bv, &nsegs, &bytes, max_segs,
-					   max_bytes)) {
-			goto split;
+		} else {
+			if (bvec_split_segs(lim, &bv, &nsegs, &bytes,
+					lim->max_segments, max_bytes))
+				goto split;
 		}
 
 		bvprv = bv;
@@ -317,7 +308,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
 	 * split size so that each bio is properly block size aligned, even if
 	 * we do not use the full hardware limits.
 	 */
-	bytes = ALIGN_DOWN(bytes, queue_logical_block_size(q));
+	bytes = ALIGN_DOWN(bytes, lim->logical_block_size);
 
 	/*
 	 * Bio splitting may cause subtle trouble such as hang when doing sync
@@ -330,7 +321,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
 
 /**
  * __blk_queue_split - split a bio and submit the second half
- * @q:       [in] request_queue new bio is being queued at
+ * @lim:     [in] queue limits to split based on
  * @bio:     [in, out] bio to be split
  * @nr_segs: [out] number of segments in the first bio
  *
@@ -341,7 +332,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
  * responsibility of the caller to ensure that disk->bio_split is only released
  * after processing of the split bio has finished.
  */
-void __blk_queue_split(struct request_queue *q, struct bio **bio,
+void __blk_queue_split(struct queue_limits *lim, struct bio **bio,
 		       unsigned int *nr_segs)
 {
 	struct bio_set *bs = &(*bio)->bi_bdev->bd_disk->bio_split;
@@ -350,14 +341,14 @@ void __blk_queue_split(struct request_queue *q, struct bio **bio,
 	switch (bio_op(*bio)) {
 	case REQ_OP_DISCARD:
 	case REQ_OP_SECURE_ERASE:
-		split = blk_bio_discard_split(q, *bio, bs, nr_segs);
+		split = blk_bio_discard_split(lim, *bio, bs, nr_segs);
 		break;
 	case REQ_OP_WRITE_ZEROES:
-		split = blk_bio_write_zeroes_split(q, *bio, bs, nr_segs);
+		split = blk_bio_write_zeroes_split(lim, *bio, bs, nr_segs);
 		break;
 	default:
-		split = blk_bio_segment_split(q, *bio, bs, nr_segs,
-				get_max_io_size(q, *bio) << SECTOR_SHIFT);
+		split = blk_bio_segment_split(lim, *bio, bs, nr_segs,
+				get_max_io_size(lim, *bio) << SECTOR_SHIFT);
 		break;
 	}
 
@@ -384,11 +375,11 @@ void __blk_queue_split(struct request_queue *q, struct bio **bio,
  */
 void blk_queue_split(struct bio **bio)
 {
-	struct request_queue *q = bdev_get_queue((*bio)->bi_bdev);
+	struct queue_limits *lim = &bdev_get_queue((*bio)->bi_bdev)->limits;
 	unsigned int nr_segs;
 
-	if (blk_may_split(q, *bio))
-		__blk_queue_split(q, bio, &nr_segs);
+	if (blk_may_split(lim, *bio))
+		__blk_queue_split(lim, bio, &nr_segs);
 }
 EXPORT_SYMBOL(blk_queue_split);
 
@@ -420,7 +411,7 @@ unsigned int blk_recalc_rq_segments(struct request *rq)
 	}
 
 	rq_for_each_bvec(bv, rq, iter)
-		bvec_split_segs(rq->q, &bv, &nr_phys_segs, &bytes,
+		bvec_split_segs(&rq->q->limits, &bv, &nr_phys_segs, &bytes,
 				UINT_MAX, UINT_MAX);
 	return nr_phys_segs;
 }
@@ -451,8 +442,8 @@ static unsigned blk_bvec_map_sg(struct request_queue *q,
 
 	while (nbytes > 0) {
 		unsigned offset = bvec->bv_offset + total;
-		unsigned len = min(get_max_segment_size(q, bvec->bv_page,
-					offset), nbytes);
+		unsigned len = min(get_max_segment_size(&q->limits,
+				   bvec->bv_page, offset), nbytes);
 		struct page *page = bvec->bv_page;
 
 		/*
diff --git a/block/blk-mq.c b/block/blk-mq.c
index d716b7f3763f3..bd7a14911d5ac 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2816,8 +2816,8 @@ void blk_mq_submit_bio(struct bio *bio)
 	blk_status_t ret;
 
 	blk_queue_bounce(q, &bio);
-	if (blk_may_split(q, bio))
-		__blk_queue_split(q, &bio, &nr_segs);
+	if (blk_may_split(&q->limits, bio))
+		__blk_queue_split(&q->limits, &bio, &nr_segs);
 
 	if (!bio_integrity_prep(bio))
 		return;
diff --git a/block/blk.h b/block/blk.h
index 3026ba81c85f0..94576404953f8 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -102,23 +102,23 @@ static inline bool biovec_phys_mergeable(struct request_queue *q,
 	return true;
 }
 
-static inline bool __bvec_gap_to_prev(struct request_queue *q,
+static inline bool __bvec_gap_to_prev(struct queue_limits *lim,
 		struct bio_vec *bprv, unsigned int offset)
 {
-	return (offset & queue_virt_boundary(q)) ||
-		((bprv->bv_offset + bprv->bv_len) & queue_virt_boundary(q));
+	return (offset & lim->virt_boundary_mask) ||
+		((bprv->bv_offset + bprv->bv_len) & lim->virt_boundary_mask);
 }
 
 /*
  * Check if adding a bio_vec after bprv with offset would create a gap in
  * the SG list. Most drivers don't care about this, but some do.
  */
-static inline bool bvec_gap_to_prev(struct request_queue *q,
+static inline bool bvec_gap_to_prev(struct queue_limits *lim,
 		struct bio_vec *bprv, unsigned int offset)
 {
-	if (!queue_virt_boundary(q))
+	if (!lim->virt_boundary_mask)
 		return false;
-	return __bvec_gap_to_prev(q, bprv, offset);
+	return __bvec_gap_to_prev(lim, bprv, offset);
 }
 
 static inline bool rq_mergeable(struct request *rq)
@@ -194,7 +194,8 @@ static inline bool integrity_req_gap_back_merge(struct request *req,
 	struct bio_integrity_payload *bip = bio_integrity(req->bio);
 	struct bio_integrity_payload *bip_next = bio_integrity(next);
 
-	return bvec_gap_to_prev(req->q, &bip->bip_vec[bip->bip_vcnt - 1],
+	return bvec_gap_to_prev(&req->q->limits,
+				&bip->bip_vec[bip->bip_vcnt - 1],
 				bip_next->bip_vec[0].bv_offset);
 }
 
@@ -204,7 +205,8 @@ static inline bool integrity_req_gap_front_merge(struct request *req,
 	struct bio_integrity_payload *bip = bio_integrity(bio);
 	struct bio_integrity_payload *bip_next = bio_integrity(req->bio);
 
-	return bvec_gap_to_prev(req->q, &bip->bip_vec[bip->bip_vcnt - 1],
+	return bvec_gap_to_prev(&req->q->limits,
+				&bip->bip_vec[bip->bip_vcnt - 1],
 				bip_next->bip_vec[0].bv_offset);
 }
 
@@ -293,7 +295,7 @@ ssize_t part_timeout_show(struct device *, struct device_attribute *, char *);
 ssize_t part_timeout_store(struct device *, struct device_attribute *,
 				const char *, size_t);
 
-static inline bool blk_may_split(struct request_queue *q, struct bio *bio)
+static inline bool blk_may_split(struct queue_limits *lim, struct bio *bio)
 {
 	switch (bio_op(bio)) {
 	case REQ_OP_DISCARD:
@@ -312,11 +314,11 @@ static inline bool blk_may_split(struct request_queue *q, struct bio *bio)
 	 * to the performance impact of cloned bios themselves the loop below
 	 * doesn't matter anyway.
 	 */
-	return q->limits.chunk_sectors || bio->bi_vcnt != 1 ||
+	return lim->chunk_sectors || bio->bi_vcnt != 1 ||
 		bio->bi_io_vec->bv_len + bio->bi_io_vec->bv_offset > PAGE_SIZE;
 }
 
-void __blk_queue_split(struct request_queue *q, struct bio **bio,
+void __blk_queue_split(struct queue_limits *lim, struct bio **bio,
 			unsigned int *nr_segs);
 int ll_back_merge_fn(struct request *req, struct bio *bio,
 		unsigned int nr_segs);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] block: move bio_allowed_max_sectors to blk-merge.c
  2022-07-20 14:24 ` [PATCH 4/5] block: move bio_allowed_max_sectors to blk-merge.c Christoph Hellwig
@ 2022-07-21  6:31   ` Johannes Thumshirn
  2022-07-21  6:42     ` Christoph Hellwig
  2022-07-22  6:02   ` Damien Le Moal
  1 sibling, 1 reply; 19+ messages in thread
From: Johannes Thumshirn @ 2022-07-21  6:31 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: linux-block@vger.kernel.org

On 20.07.22 16:25, Christoph Hellwig wrote:
> +/*
> + * The max size one bio can handle is UINT_MAX becasue bvec_iter.bi_size
> + * is defined as 'unsigned int', meantime it has to aligned to with logical
> + * block size which is the minimum accepted unit by hardware.
> + */

Not your call, as your only moving the comment but 'to be aligned to'

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: bio splitting cleanups
  2022-07-20 14:24 bio splitting cleanups Christoph Hellwig
                   ` (4 preceding siblings ...)
  2022-07-20 14:24 ` [PATCH 5/5] block: pass struct queue_limits to the bio splitting helpers Christoph Hellwig
@ 2022-07-21  6:33 ` Johannes Thumshirn
  5 siblings, 0 replies; 19+ messages in thread
From: Johannes Thumshirn @ 2022-07-21  6:33 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: linux-block@vger.kernel.org

On 20.07.22 16:25, Christoph Hellwig wrote:
> Hi Jens,
> 
> this series has two parts:  the first patch moves the ->bio_split
> bio_set to the gendisk as it only is used for file system style I/O.
> 
> The other patches reshuffle the bio splitting code so that in the future
> blk_bio_segment_split can be used to split REQ_OP_ZONE_APPEND bios
> under file system / remapping driver control.  I plan to use that in
> btrfs in the next merge window.
> 
> Diffstat:
>  block/bio-integrity.c  |    2 
>  block/bio.c            |    2 
>  block/blk-core.c       |    9 ---
>  block/blk-merge.c      |  139 ++++++++++++++++++++++++-------------------------
>  block/blk-mq.c         |    4 -
>  block/blk-settings.c   |    2 
>  block/blk-sysfs.c      |    2 
>  block/blk.h            |   34 ++++-------
>  block/genhd.c          |    9 ++-
>  drivers/md/dm.c        |    2 
>  include/linux/blkdev.h |    3 -
>  11 files changed, 100 insertions(+), 108 deletions(-)
> 

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] block: move bio_allowed_max_sectors to blk-merge.c
  2022-07-21  6:31   ` Johannes Thumshirn
@ 2022-07-21  6:42     ` Christoph Hellwig
  2022-07-21  6:42       ` Johannes Thumshirn
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2022-07-21  6:42 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Christoph Hellwig, Jens Axboe, linux-block@vger.kernel.org

On Thu, Jul 21, 2022 at 06:31:24AM +0000, Johannes Thumshirn wrote:
> On 20.07.22 16:25, Christoph Hellwig wrote:
> > +/*
> > + * The max size one bio can handle is UINT_MAX becasue bvec_iter.bi_size
> > + * is defined as 'unsigned int', meantime it has to aligned to with logical
> > + * block size which is the minimum accepted unit by hardware.
> > + */
> 
> Not your call, as your only moving the comment but 'to be aligned to'

I'm perfectly fine with fixing it, but a little to lazy to resend just
for that :)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] block: move bio_allowed_max_sectors to blk-merge.c
  2022-07-21  6:42     ` Christoph Hellwig
@ 2022-07-21  6:42       ` Johannes Thumshirn
  0 siblings, 0 replies; 19+ messages in thread
From: Johannes Thumshirn @ 2022-07-21  6:42 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block@vger.kernel.org

On 21.07.22 08:42, Christoph Hellwig wrote:
> On Thu, Jul 21, 2022 at 06:31:24AM +0000, Johannes Thumshirn wrote:
>> On 20.07.22 16:25, Christoph Hellwig wrote:
>>> +/*
>>> + * The max size one bio can handle is UINT_MAX becasue bvec_iter.bi_size
>>> + * is defined as 'unsigned int', meantime it has to aligned to with logical
>>> + * block size which is the minimum accepted unit by hardware.
>>> + */
>>
>> Not your call, as your only moving the comment but 'to be aligned to'
> 
> I'm perfectly fine with fixing it, but a little to lazy to resend just
> for that :)
> 

Sure. I think Jens can fix it up when applying.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/5] block: move ->bio_split to the gendisk
  2022-07-20 14:24 ` [PATCH 1/5] block: move ->bio_split to the gendisk Christoph Hellwig
@ 2022-07-22  5:56   ` Damien Le Moal
  2022-07-22  6:00     ` Christoph Hellwig
  0 siblings, 1 reply; 19+ messages in thread
From: Damien Le Moal @ 2022-07-22  5:56 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: linux-block

On 7/20/22 23:24, Christoph Hellwig wrote:
> Only non-passthrough requests are split by the block layer and use the
> ->bio_split bio_set.  Move it from the request_queue to the gendisk.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/blk-core.c       |  9 +--------
>  block/blk-merge.c      | 20 ++++++++++----------
>  block/blk-sysfs.c      |  2 --
>  block/genhd.c          |  9 ++++++++-
>  drivers/md/dm.c        |  2 +-
>  include/linux/blkdev.h |  3 ++-
>  6 files changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 123468b9d2e43..59f13d011949d 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -377,7 +377,6 @@ static void blk_timeout_work(struct work_struct *work)
>  struct request_queue *blk_alloc_queue(int node_id, bool alloc_srcu)
>  {
>  	struct request_queue *q;
> -	int ret;
>  
>  	q = kmem_cache_alloc_node(blk_get_queue_kmem_cache(alloc_srcu),
>  			GFP_KERNEL | __GFP_ZERO, node_id);
> @@ -396,13 +395,9 @@ struct request_queue *blk_alloc_queue(int node_id, bool alloc_srcu)
>  	if (q->id < 0)
>  		goto fail_srcu;
>  
> -	ret = bioset_init(&q->bio_split, BIO_POOL_SIZE, 0, 0);
> -	if (ret)
> -		goto fail_id;
> -
>  	q->stats = blk_alloc_queue_stats();
>  	if (!q->stats)
> -		goto fail_split;
> +		goto fail_id;
>  
>  	q->node = node_id;
>  
> @@ -439,8 +434,6 @@ struct request_queue *blk_alloc_queue(int node_id, bool alloc_srcu)
>  
>  fail_stats:
>  	blk_free_queue_stats(q->stats);
> -fail_split:
> -	bioset_exit(&q->bio_split);
>  fail_id:
>  	ida_free(&blk_queue_ida, q->id);
>  fail_srcu:
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 3c3f785f558af..e657f1dc824cb 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -328,26 +328,26 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
>   * Split a bio into two bios, chain the two bios, submit the second half and
>   * store a pointer to the first half in *@bio. If the second bio is still too
>   * big it will be split by a recursive call to this function. Since this
> - * function may allocate a new bio from q->bio_split, it is the responsibility
> - * of the caller to ensure that q->bio_split is only released after processing
> - * of the split bio has finished.
> + * function may allocate a new bio from disk->bio_split, it is the
> + * responsibility of the caller to ensure that disk->bio_split is only released
> + * after processing of the split bio has finished.
>   */
>  void __blk_queue_split(struct request_queue *q, struct bio **bio,
>  		       unsigned int *nr_segs)
>  {
> +	struct bio_set *bs = &(*bio)->bi_bdev->bd_disk->bio_split;
>  	struct bio *split = NULL;
>  
>  	switch (bio_op(*bio)) {
>  	case REQ_OP_DISCARD:
>  	case REQ_OP_SECURE_ERASE:
> -		split = blk_bio_discard_split(q, *bio, &q->bio_split, nr_segs);
> +		split = blk_bio_discard_split(q, *bio, bs, nr_segs);
>  		break;
>  	case REQ_OP_WRITE_ZEROES:
> -		split = blk_bio_write_zeroes_split(q, *bio, &q->bio_split,
> -				nr_segs);
> +		split = blk_bio_write_zeroes_split(q, *bio, bs, nr_segs);
>  		break;
>  	default:
> -		split = blk_bio_segment_split(q, *bio, &q->bio_split, nr_segs);
> +		split = blk_bio_segment_split(q, *bio, bs, nr_segs);
>  		break;
>  	}

Suggestion for a follow-up patch: we could save the *bio pointer in a
local variable instead of constantly de-referencing bio.

>  
> @@ -368,9 +368,9 @@ void __blk_queue_split(struct request_queue *q, struct bio **bio,
>   *
>   * Split a bio into two bios, chains the two bios, submit the second half and
>   * store a pointer to the first half in *@bio. Since this function may allocate
> - * a new bio from q->bio_split, it is the responsibility of the caller to ensure
> - * that q->bio_split is only released after processing of the split bio has
> - * finished.
> + * a new bio from disk->bio_split, it is the responsibility of the caller to
> + * ensure that disk->bio_split is only released after processing of the split
> + * bio has finished.
>   */
>  void blk_queue_split(struct bio **bio)
>  {
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index c0303026752d5..e1f009aba6fd2 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -779,8 +779,6 @@ static void blk_release_queue(struct kobject *kobj)
>  	if (queue_is_mq(q))
>  		blk_mq_release(q);
>  
> -	bioset_exit(&q->bio_split);
> -
>  	if (blk_queue_has_srcu(q))
>  		cleanup_srcu_struct(q->srcu);
>  
> diff --git a/block/genhd.c b/block/genhd.c
> index 44dfcf67ed96a..150494e8742b0 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -1138,6 +1138,8 @@ static void disk_release(struct device *dev)
>  	might_sleep();
>  	WARN_ON_ONCE(disk_live(disk));
>  
> +	bioset_exit(&disk->bio_split);
> +
>  	blkcg_exit_queue(disk->queue);
>  
>  	disk_release_events(disk);
> @@ -1330,9 +1332,12 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
>  	if (!disk)
>  		goto out_put_queue;
>  
> +	if (bioset_init(&disk->bio_split, BIO_POOL_SIZE, 0, 0))
> +		goto out_free_disk;
> +
>  	disk->bdi = bdi_alloc(node_id);
>  	if (!disk->bdi)
> -		goto out_free_disk;
> +		goto out_free_bioset;
>  
>  	/* bdev_alloc() might need the queue, set before the first call */
>  	disk->queue = q;
> @@ -1370,6 +1375,8 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
>  	iput(disk->part0->bd_inode);
>  out_free_bdi:
>  	bdi_put(disk->bdi);
> +out_free_bioset:
> +	bioset_exit(&disk->bio_split);
>  out_free_disk:
>  	kfree(disk);
>  out_put_queue:
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 54c2a23f4e55c..b163de50f3c6b 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1693,7 +1693,7 @@ static void dm_split_and_process_bio(struct mapped_device *md,
>  	 */
>  	WARN_ON_ONCE(!dm_io_flagged(io, DM_IO_WAS_SPLIT));
>  	io->split_bio = bio_split(bio, io->sectors, GFP_NOIO,
> -				  &md->queue->bio_split);
> +				  &md->disk->bio_split);
>  	bio_chain(io->split_bio, bio);
>  	trace_block_split(io->split_bio, bio->bi_iter.bi_sector);
>  	submit_bio_noacct(bio);
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index d04bdf549efa9..97d3ef8f3f416 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -140,6 +140,8 @@ struct gendisk {
>  	struct request_queue *queue;
>  	void *private_data;
>  
> +	struct bio_set bio_split;
> +
>  	int flags;
>  	unsigned long state;
>  #define GD_NEED_PART_SCAN		0
> @@ -531,7 +533,6 @@ struct request_queue {
>  
>  	struct blk_mq_tag_set	*tag_set;
>  	struct list_head	tag_set_list;
> -	struct bio_set		bio_split;
>  
>  	struct dentry		*debugfs_dir;
>  	struct dentry		*sched_debugfs_dir;

Looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/5] block: fix max_zone_append_sectors inheritance in blk_stack_limits
  2022-07-20 14:24 ` [PATCH 2/5] block: fix max_zone_append_sectors inheritance in blk_stack_limits Christoph Hellwig
@ 2022-07-22  5:59   ` Damien Le Moal
  2022-07-22  6:03     ` Christoph Hellwig
  0 siblings, 1 reply; 19+ messages in thread
From: Damien Le Moal @ 2022-07-22  5:59 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: linux-block

On 7/20/22 23:24, Christoph Hellwig wrote:
> As we start out with a default of 0, this needs a min_not_zero to
> actually work.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/blk-settings.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 8bb9eef5310eb..9f6e271ca67f4 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -554,7 +554,7 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
>  	t->max_dev_sectors = min_not_zero(t->max_dev_sectors, b->max_dev_sectors);
>  	t->max_write_zeroes_sectors = min(t->max_write_zeroes_sectors,
>  					b->max_write_zeroes_sectors);
> -	t->max_zone_append_sectors = min(t->max_zone_append_sectors,
> +	t->max_zone_append_sectors = min_not_zero(t->max_zone_append_sectors,
>  					b->max_zone_append_sectors);

Hmmm... Given that max_zone_append_sectors should never be zero for any
zoned block device, that is OK. However, DM targets combining zoned and
non-zoned devices to create a non zoned logical drive, e.g. dm-zoned with
a regular ssd for metadata, should not have a non-zero
max_zone_append_sectors. So I am not confident this change leads to
correct limits in all cases.

>  	t->bounce = max(t->bounce, b->bounce);
>  


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/5] block: move ->bio_split to the gendisk
  2022-07-22  5:56   ` Damien Le Moal
@ 2022-07-22  6:00     ` Christoph Hellwig
  0 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2022-07-22  6:00 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: Christoph Hellwig, Jens Axboe, linux-block

On Fri, Jul 22, 2022 at 02:56:13PM +0900, Damien Le Moal wrote:
> Suggestion for a follow-up patch: we could save the *bio pointer in a
> local variable instead of constantly de-referencing bio.

I suspect a better calling convention might be to just return the
new bio.  I'll give that a spin for the follow up and see what it does
to code size.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] block: move bio_allowed_max_sectors to blk-merge.c
  2022-07-20 14:24 ` [PATCH 4/5] block: move bio_allowed_max_sectors to blk-merge.c Christoph Hellwig
  2022-07-21  6:31   ` Johannes Thumshirn
@ 2022-07-22  6:02   ` Damien Le Moal
  1 sibling, 0 replies; 19+ messages in thread
From: Damien Le Moal @ 2022-07-22  6:02 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: linux-block

On 7/20/22 23:24, Christoph Hellwig wrote:
> Move this helper into the only file where it is used.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

> ---
>  block/blk-merge.c | 10 ++++++++++
>  block/blk.h       | 10 ----------
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 1676a835b16e7..9593a8a617292 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -95,6 +95,16 @@ static inline bool req_gap_front_merge(struct request *req, struct bio *bio)
>  	return bio_will_gap(req->q, NULL, bio, req->bio);
>  }
>  
> +/*
> + * The max size one bio can handle is UINT_MAX becasue bvec_iter.bi_size
> + * is defined as 'unsigned int', meantime it has to aligned to with logical
> + * block size which is the minimum accepted unit by hardware.
> + */
> +static unsigned int bio_allowed_max_sectors(struct request_queue *q)
> +{
> +	return round_down(UINT_MAX, queue_logical_block_size(q)) >> 9;
> +}
> +
>  static struct bio *blk_bio_discard_split(struct request_queue *q,
>  					 struct bio *bio,
>  					 struct bio_set *bs,
> diff --git a/block/blk.h b/block/blk.h
> index c4b084bfe87c9..3026ba81c85f0 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -349,16 +349,6 @@ static inline void req_set_nomerge(struct request_queue *q, struct request *req)
>  		q->last_merge = NULL;
>  }
>  
> -/*
> - * The max size one bio can handle is UINT_MAX becasue bvec_iter.bi_size
> - * is defined as 'unsigned int', meantime it has to aligned to with logical
> - * block size which is the minimum accepted unit by hardware.
> - */
> -static inline unsigned int bio_allowed_max_sectors(struct request_queue *q)
> -{
> -	return round_down(UINT_MAX, queue_logical_block_size(q)) >> 9;
> -}
> -
>  /*
>   * Internal io_context interface
>   */


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/5] block: fix max_zone_append_sectors inheritance in blk_stack_limits
  2022-07-22  5:59   ` Damien Le Moal
@ 2022-07-22  6:03     ` Christoph Hellwig
  0 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2022-07-22  6:03 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: Christoph Hellwig, Jens Axboe, linux-block

On Fri, Jul 22, 2022 at 02:59:30PM +0900, Damien Le Moal wrote:
> Hmmm... Given that max_zone_append_sectors should never be zero for any
> zoned block device, that is OK. However, DM targets combining zoned and
> non-zoned devices to create a non zoned logical drive, e.g. dm-zoned with
> a regular ssd for metadata, should not have a non-zero
> max_zone_append_sectors. So I am not confident this change leads to
> correct limits in all cases.

Good point.

I think we can drop this patch, as I just need to call
blk_set_stacking_limits instead of blk_set_default_limits in the
btrfs code, which should sort all this out.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/5] block: pass struct queue_limits to the bio splitting helpers
  2022-07-20 14:24 ` [PATCH 5/5] block: pass struct queue_limits to the bio splitting helpers Christoph Hellwig
@ 2022-07-22  6:08   ` Damien Le Moal
  0 siblings, 0 replies; 19+ messages in thread
From: Damien Le Moal @ 2022-07-22  6:08 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: linux-block

On 7/20/22 23:24, Christoph Hellwig wrote:
> Allow using the splitting helpers on just a queue_limits instead of
> a full request_queue structure.  This will eventuall allow file systems

s/eventuall/eventually

> or remapping drivers to split REQ_OP_ZONE_APPEND bios based on limits
> calculated as the minimum common capabilities over multiple devices.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Other that the message typo, looks good to me.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

> ---
>  block/bio-integrity.c |   2 +-
>  block/bio.c           |   2 +-
>  block/blk-merge.c     | 111 +++++++++++++++++++-----------------------
>  block/blk-mq.c        |   4 +-
>  block/blk.h           |  24 ++++-----
>  5 files changed, 68 insertions(+), 75 deletions(-)
> 
> diff --git a/block/bio-integrity.c b/block/bio-integrity.c
> index 32929c89ba8a6..3f5685c00e360 100644
> --- a/block/bio-integrity.c
> +++ b/block/bio-integrity.c
> @@ -134,7 +134,7 @@ int bio_integrity_add_page(struct bio *bio, struct page *page,
>  	iv = bip->bip_vec + bip->bip_vcnt;
>  
>  	if (bip->bip_vcnt &&
> -	    bvec_gap_to_prev(bdev_get_queue(bio->bi_bdev),
> +	    bvec_gap_to_prev(&bdev_get_queue(bio->bi_bdev)->limits,
>  			     &bip->bip_vec[bip->bip_vcnt - 1], offset))
>  		return 0;
>  
> diff --git a/block/bio.c b/block/bio.c
> index 6f9f883f9a65a..0d866d44ce533 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -965,7 +965,7 @@ int bio_add_hw_page(struct request_queue *q, struct bio *bio,
>  		 * would create a gap, disallow it.
>  		 */
>  		bvec = &bio->bi_io_vec[bio->bi_vcnt - 1];
> -		if (bvec_gap_to_prev(q, bvec, offset))
> +		if (bvec_gap_to_prev(&q->limits, bvec, offset))
>  			return 0;
>  	}
>  
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 9593a8a617292..d8c99cc4ef362 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -82,7 +82,7 @@ static inline bool bio_will_gap(struct request_queue *q,
>  	bio_get_first_bvec(next, &nb);
>  	if (biovec_phys_mergeable(q, &pb, &nb))
>  		return false;
> -	return __bvec_gap_to_prev(q, &pb, nb.bv_offset);
> +	return __bvec_gap_to_prev(&q->limits, &pb, nb.bv_offset);
>  }
>  
>  static inline bool req_gap_back_merge(struct request *req, struct bio *bio)
> @@ -100,28 +100,25 @@ static inline bool req_gap_front_merge(struct request *req, struct bio *bio)
>   * is defined as 'unsigned int', meantime it has to aligned to with logical
>   * block size which is the minimum accepted unit by hardware.
>   */
> -static unsigned int bio_allowed_max_sectors(struct request_queue *q)
> +static unsigned int bio_allowed_max_sectors(struct queue_limits *lim)
>  {
> -	return round_down(UINT_MAX, queue_logical_block_size(q)) >> 9;
> +	return round_down(UINT_MAX, lim->logical_block_size) >> SECTOR_SHIFT;
>  }
>  
> -static struct bio *blk_bio_discard_split(struct request_queue *q,
> -					 struct bio *bio,
> -					 struct bio_set *bs,
> -					 unsigned *nsegs)
> +static struct bio *blk_bio_discard_split(struct queue_limits *lim,
> +		struct bio *bio, struct bio_set *bs, unsigned *nsegs)
>  {
>  	unsigned int max_discard_sectors, granularity;
> -	int alignment;
>  	sector_t tmp;
>  	unsigned split_sectors;
>  
>  	*nsegs = 1;
>  
>  	/* Zero-sector (unknown) and one-sector granularities are the same.  */
> -	granularity = max(q->limits.discard_granularity >> 9, 1U);
> +	granularity = max(lim->discard_granularity >> 9, 1U);
>  
> -	max_discard_sectors = min(q->limits.max_discard_sectors,
> -			bio_allowed_max_sectors(q));
> +	max_discard_sectors =
> +		min(lim->max_discard_sectors, bio_allowed_max_sectors(lim));
>  	max_discard_sectors -= max_discard_sectors % granularity;
>  
>  	if (unlikely(!max_discard_sectors)) {
> @@ -138,9 +135,8 @@ static struct bio *blk_bio_discard_split(struct request_queue *q,
>  	 * If the next starting sector would be misaligned, stop the discard at
>  	 * the previous aligned sector.
>  	 */
> -	alignment = (q->limits.discard_alignment >> 9) % granularity;
> -
> -	tmp = bio->bi_iter.bi_sector + split_sectors - alignment;
> +	tmp = bio->bi_iter.bi_sector + split_sectors -
> +		((lim->discard_alignment >> 9) % granularity);
>  	tmp = sector_div(tmp, granularity);
>  
>  	if (split_sectors > tmp)
> @@ -149,18 +145,15 @@ static struct bio *blk_bio_discard_split(struct request_queue *q,
>  	return bio_split(bio, split_sectors, GFP_NOIO, bs);
>  }
>  
> -static struct bio *blk_bio_write_zeroes_split(struct request_queue *q,
> +static struct bio *blk_bio_write_zeroes_split(struct queue_limits *lim,
>  		struct bio *bio, struct bio_set *bs, unsigned *nsegs)
>  {
>  	*nsegs = 0;
> -
> -	if (!q->limits.max_write_zeroes_sectors)
> +	if (!lim->max_write_zeroes_sectors)
>  		return NULL;
> -
> -	if (bio_sectors(bio) <= q->limits.max_write_zeroes_sectors)
> +	if (bio_sectors(bio) <= lim->max_write_zeroes_sectors)
>  		return NULL;
> -
> -	return bio_split(bio, q->limits.max_write_zeroes_sectors, GFP_NOIO, bs);
> +	return bio_split(bio, lim->max_write_zeroes_sectors, GFP_NOIO, bs);
>  }
>  
>  /*
> @@ -171,17 +164,17 @@ static struct bio *blk_bio_write_zeroes_split(struct request_queue *q,
>   * requests that are submitted to a block device if the start of a bio is not
>   * aligned to a physical block boundary.
>   */
> -static inline unsigned get_max_io_size(struct request_queue *q,
> +static inline unsigned get_max_io_size(struct queue_limits *lim,
>  				       struct bio *bio)
>  {
> -	unsigned pbs = queue_physical_block_size(q) >> SECTOR_SHIFT;
> -	unsigned lbs = queue_logical_block_size(q) >> SECTOR_SHIFT;
> -	unsigned max_sectors = queue_max_sectors(q), start, end;
> +	unsigned pbs = lim->physical_block_size >> SECTOR_SHIFT;
> +	unsigned lbs = lim->logical_block_size >> SECTOR_SHIFT;
> +	unsigned max_sectors = lim->max_sectors, start, end;
>  
> -	if (q->limits.chunk_sectors) {
> +	if (lim->chunk_sectors) {
>  		max_sectors = min(max_sectors,
>  			blk_chunk_sectors_left(bio->bi_iter.bi_sector,
> -					       q->limits.chunk_sectors));
> +					       lim->chunk_sectors));
>  	}
>  
>  	start = bio->bi_iter.bi_sector & (pbs - 1);
> @@ -191,11 +184,10 @@ static inline unsigned get_max_io_size(struct request_queue *q,
>  	return max_sectors & ~(lbs - 1);
>  }
>  
> -static inline unsigned get_max_segment_size(const struct request_queue *q,
> -					    struct page *start_page,
> -					    unsigned long offset)
> +static inline unsigned get_max_segment_size(struct queue_limits *lim,
> +		struct page *start_page, unsigned long offset)
>  {
> -	unsigned long mask = queue_segment_boundary(q);
> +	unsigned long mask = lim->seg_boundary_mask;
>  
>  	offset = mask & (page_to_phys(start_page) + offset);
>  
> @@ -204,12 +196,12 @@ static inline unsigned get_max_segment_size(const struct request_queue *q,
>  	 * on 32bit arch, use queue's max segment size when that happens.
>  	 */
>  	return min_not_zero(mask - offset + 1,
> -			(unsigned long)queue_max_segment_size(q));
> +			(unsigned long)lim->max_segment_size);
>  }
>  
>  /**
>   * bvec_split_segs - verify whether or not a bvec should be split in the middle
> - * @q:        [in] request queue associated with the bio associated with @bv
> + * @lim:      [in] queue limits to split based on
>   * @bv:       [in] bvec to examine
>   * @nsegs:    [in,out] Number of segments in the bio being built. Incremented
>   *            by the number of segments from @bv that may be appended to that
> @@ -227,10 +219,9 @@ static inline unsigned get_max_segment_size(const struct request_queue *q,
>   * *@nsegs segments and *@sectors sectors would make that bio unacceptable for
>   * the block driver.
>   */
> -static bool bvec_split_segs(const struct request_queue *q,
> -			    const struct bio_vec *bv, unsigned *nsegs,
> -			    unsigned *bytes, unsigned max_segs,
> -			    unsigned max_bytes)
> +static bool bvec_split_segs(struct queue_limits *lim, const struct bio_vec *bv,
> +		unsigned *nsegs, unsigned *bytes, unsigned max_segs,
> +		unsigned max_bytes)
>  {
>  	unsigned max_len = min(max_bytes, UINT_MAX) - *bytes;
>  	unsigned len = min(bv->bv_len, max_len);
> @@ -238,7 +229,7 @@ static bool bvec_split_segs(const struct request_queue *q,
>  	unsigned seg_size = 0;
>  
>  	while (len && *nsegs < max_segs) {
> -		seg_size = get_max_segment_size(q, bv->bv_page,
> +		seg_size = get_max_segment_size(lim, bv->bv_page,
>  						bv->bv_offset + total_len);
>  		seg_size = min(seg_size, len);
>  
> @@ -246,7 +237,7 @@ static bool bvec_split_segs(const struct request_queue *q,
>  		total_len += seg_size;
>  		len -= seg_size;
>  
> -		if ((bv->bv_offset + total_len) & queue_virt_boundary(q))
> +		if ((bv->bv_offset + total_len) & lim->virt_boundary_mask)
>  			break;
>  	}
>  
> @@ -258,7 +249,7 @@ static bool bvec_split_segs(const struct request_queue *q,
>  
>  /**
>   * blk_bio_segment_split - split a bio in two bios
> - * @q:    [in] request queue pointer
> + * @lim:  [in] queue limits to split based on
>   * @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
> @@ -276,31 +267,31 @@ static bool bvec_split_segs(const struct request_queue *q,
>   * 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,
> +static struct bio *blk_bio_segment_split(struct queue_limits *lim,
>  		struct bio *bio, struct bio_set *bs, unsigned *segs,
>  		unsigned max_bytes)
>  {
>  	struct bio_vec bv, bvprv, *bvprvp = NULL;
>  	struct bvec_iter iter;
>  	unsigned nsegs = 0, bytes = 0;
> -	const unsigned max_segs = queue_max_segments(q);
>  
>  	bio_for_each_bvec(bv, bio, iter) {
>  		/*
>  		 * If the queue doesn't support SG gaps and adding this
>  		 * offset would create a gap, disallow it.
>  		 */
> -		if (bvprvp && bvec_gap_to_prev(q, bvprvp, bv.bv_offset))
> +		if (bvprvp && bvec_gap_to_prev(lim, bvprvp, bv.bv_offset))
>  			goto split;
>  
> -		if (nsegs < max_segs &&
> +		if (nsegs < lim->max_segments &&
>  		    bytes + bv.bv_len <= max_bytes &&
>  		    bv.bv_offset + bv.bv_len <= PAGE_SIZE) {
>  			nsegs++;
>  			bytes += bv.bv_len;
> -		} else if (bvec_split_segs(q, &bv, &nsegs, &bytes, max_segs,
> -					   max_bytes)) {
> -			goto split;
> +		} else {
> +			if (bvec_split_segs(lim, &bv, &nsegs, &bytes,
> +					lim->max_segments, max_bytes))
> +				goto split;
>  		}
>  
>  		bvprv = bv;
> @@ -317,7 +308,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
>  	 * split size so that each bio is properly block size aligned, even if
>  	 * we do not use the full hardware limits.
>  	 */
> -	bytes = ALIGN_DOWN(bytes, queue_logical_block_size(q));
> +	bytes = ALIGN_DOWN(bytes, lim->logical_block_size);
>  
>  	/*
>  	 * Bio splitting may cause subtle trouble such as hang when doing sync
> @@ -330,7 +321,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
>  
>  /**
>   * __blk_queue_split - split a bio and submit the second half
> - * @q:       [in] request_queue new bio is being queued at
> + * @lim:     [in] queue limits to split based on
>   * @bio:     [in, out] bio to be split
>   * @nr_segs: [out] number of segments in the first bio
>   *
> @@ -341,7 +332,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
>   * responsibility of the caller to ensure that disk->bio_split is only released
>   * after processing of the split bio has finished.
>   */
> -void __blk_queue_split(struct request_queue *q, struct bio **bio,
> +void __blk_queue_split(struct queue_limits *lim, struct bio **bio,
>  		       unsigned int *nr_segs)
>  {
>  	struct bio_set *bs = &(*bio)->bi_bdev->bd_disk->bio_split;
> @@ -350,14 +341,14 @@ void __blk_queue_split(struct request_queue *q, struct bio **bio,
>  	switch (bio_op(*bio)) {
>  	case REQ_OP_DISCARD:
>  	case REQ_OP_SECURE_ERASE:
> -		split = blk_bio_discard_split(q, *bio, bs, nr_segs);
> +		split = blk_bio_discard_split(lim, *bio, bs, nr_segs);
>  		break;
>  	case REQ_OP_WRITE_ZEROES:
> -		split = blk_bio_write_zeroes_split(q, *bio, bs, nr_segs);
> +		split = blk_bio_write_zeroes_split(lim, *bio, bs, nr_segs);
>  		break;
>  	default:
> -		split = blk_bio_segment_split(q, *bio, bs, nr_segs,
> -				get_max_io_size(q, *bio) << SECTOR_SHIFT);
> +		split = blk_bio_segment_split(lim, *bio, bs, nr_segs,
> +				get_max_io_size(lim, *bio) << SECTOR_SHIFT);
>  		break;
>  	}
>  
> @@ -384,11 +375,11 @@ void __blk_queue_split(struct request_queue *q, struct bio **bio,
>   */
>  void blk_queue_split(struct bio **bio)
>  {
> -	struct request_queue *q = bdev_get_queue((*bio)->bi_bdev);
> +	struct queue_limits *lim = &bdev_get_queue((*bio)->bi_bdev)->limits;
>  	unsigned int nr_segs;
>  
> -	if (blk_may_split(q, *bio))
> -		__blk_queue_split(q, bio, &nr_segs);
> +	if (blk_may_split(lim, *bio))
> +		__blk_queue_split(lim, bio, &nr_segs);
>  }
>  EXPORT_SYMBOL(blk_queue_split);
>  
> @@ -420,7 +411,7 @@ unsigned int blk_recalc_rq_segments(struct request *rq)
>  	}
>  
>  	rq_for_each_bvec(bv, rq, iter)
> -		bvec_split_segs(rq->q, &bv, &nr_phys_segs, &bytes,
> +		bvec_split_segs(&rq->q->limits, &bv, &nr_phys_segs, &bytes,
>  				UINT_MAX, UINT_MAX);
>  	return nr_phys_segs;
>  }
> @@ -451,8 +442,8 @@ static unsigned blk_bvec_map_sg(struct request_queue *q,
>  
>  	while (nbytes > 0) {
>  		unsigned offset = bvec->bv_offset + total;
> -		unsigned len = min(get_max_segment_size(q, bvec->bv_page,
> -					offset), nbytes);
> +		unsigned len = min(get_max_segment_size(&q->limits,
> +				   bvec->bv_page, offset), nbytes);
>  		struct page *page = bvec->bv_page;
>  
>  		/*
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index d716b7f3763f3..bd7a14911d5ac 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2816,8 +2816,8 @@ void blk_mq_submit_bio(struct bio *bio)
>  	blk_status_t ret;
>  
>  	blk_queue_bounce(q, &bio);
> -	if (blk_may_split(q, bio))
> -		__blk_queue_split(q, &bio, &nr_segs);
> +	if (blk_may_split(&q->limits, bio))
> +		__blk_queue_split(&q->limits, &bio, &nr_segs);
>  
>  	if (!bio_integrity_prep(bio))
>  		return;
> diff --git a/block/blk.h b/block/blk.h
> index 3026ba81c85f0..94576404953f8 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -102,23 +102,23 @@ static inline bool biovec_phys_mergeable(struct request_queue *q,
>  	return true;
>  }
>  
> -static inline bool __bvec_gap_to_prev(struct request_queue *q,
> +static inline bool __bvec_gap_to_prev(struct queue_limits *lim,
>  		struct bio_vec *bprv, unsigned int offset)
>  {
> -	return (offset & queue_virt_boundary(q)) ||
> -		((bprv->bv_offset + bprv->bv_len) & queue_virt_boundary(q));
> +	return (offset & lim->virt_boundary_mask) ||
> +		((bprv->bv_offset + bprv->bv_len) & lim->virt_boundary_mask);
>  }
>  
>  /*
>   * Check if adding a bio_vec after bprv with offset would create a gap in
>   * the SG list. Most drivers don't care about this, but some do.
>   */
> -static inline bool bvec_gap_to_prev(struct request_queue *q,
> +static inline bool bvec_gap_to_prev(struct queue_limits *lim,
>  		struct bio_vec *bprv, unsigned int offset)
>  {
> -	if (!queue_virt_boundary(q))
> +	if (!lim->virt_boundary_mask)
>  		return false;
> -	return __bvec_gap_to_prev(q, bprv, offset);
> +	return __bvec_gap_to_prev(lim, bprv, offset);
>  }
>  
>  static inline bool rq_mergeable(struct request *rq)
> @@ -194,7 +194,8 @@ static inline bool integrity_req_gap_back_merge(struct request *req,
>  	struct bio_integrity_payload *bip = bio_integrity(req->bio);
>  	struct bio_integrity_payload *bip_next = bio_integrity(next);
>  
> -	return bvec_gap_to_prev(req->q, &bip->bip_vec[bip->bip_vcnt - 1],
> +	return bvec_gap_to_prev(&req->q->limits,
> +				&bip->bip_vec[bip->bip_vcnt - 1],
>  				bip_next->bip_vec[0].bv_offset);
>  }
>  
> @@ -204,7 +205,8 @@ static inline bool integrity_req_gap_front_merge(struct request *req,
>  	struct bio_integrity_payload *bip = bio_integrity(bio);
>  	struct bio_integrity_payload *bip_next = bio_integrity(req->bio);
>  
> -	return bvec_gap_to_prev(req->q, &bip->bip_vec[bip->bip_vcnt - 1],
> +	return bvec_gap_to_prev(&req->q->limits,
> +				&bip->bip_vec[bip->bip_vcnt - 1],
>  				bip_next->bip_vec[0].bv_offset);
>  }
>  
> @@ -293,7 +295,7 @@ ssize_t part_timeout_show(struct device *, struct device_attribute *, char *);
>  ssize_t part_timeout_store(struct device *, struct device_attribute *,
>  				const char *, size_t);
>  
> -static inline bool blk_may_split(struct request_queue *q, struct bio *bio)
> +static inline bool blk_may_split(struct queue_limits *lim, struct bio *bio)
>  {
>  	switch (bio_op(bio)) {
>  	case REQ_OP_DISCARD:
> @@ -312,11 +314,11 @@ static inline bool blk_may_split(struct request_queue *q, struct bio *bio)
>  	 * to the performance impact of cloned bios themselves the loop below
>  	 * doesn't matter anyway.
>  	 */
> -	return q->limits.chunk_sectors || bio->bi_vcnt != 1 ||
> +	return lim->chunk_sectors || bio->bi_vcnt != 1 ||
>  		bio->bi_io_vec->bv_len + bio->bi_io_vec->bv_offset > PAGE_SIZE;
>  }
>  
> -void __blk_queue_split(struct request_queue *q, struct bio **bio,
> +void __blk_queue_split(struct queue_limits *lim, struct bio **bio,
>  			unsigned int *nr_segs);
>  int ll_back_merge_fn(struct request *req, struct bio *bio,
>  		unsigned int nr_segs);


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] block: move the call to get_max_io_size out of blk_bio_segment_split
  2022-07-20 14:24 ` [PATCH 3/5] block: move the call to get_max_io_size out of blk_bio_segment_split Christoph Hellwig
@ 2022-07-22  6:09   ` Damien Le Moal
  2022-07-22  6:11     ` Christoph Hellwig
  0 siblings, 1 reply; 19+ messages in thread
From: Damien Le Moal @ 2022-07-22  6:09 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: linux-block

On 7/20/22 23:24, Christoph Hellwig wrote:
> Prepare for reusing blk_bio_segment_split for (file system controlled)
> splits of REQ_OP_ZONE_APPEND bios by letting the caller control the
> maximum size of the bio.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Though I think this patch could wait for the actual users of
this change.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

> ---
>  block/blk-merge.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index e657f1dc824cb..1676a835b16e7 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -252,11 +252,12 @@ static bool bvec_split_segs(const struct request_queue *q,
>   * @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
> + * @max_bytes: [in] maximum number of bytes per bio
>   *
>   * 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 @max_bytes worth of data
>   * - 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
> @@ -266,14 +267,12 @@ static bool bvec_split_segs(const struct request_queue *q,
>   * split bio has finished.
>   */
>  static struct bio *blk_bio_segment_split(struct request_queue *q,
> -					 struct bio *bio,
> -					 struct bio_set *bs,
> -					 unsigned *segs)
> +		struct bio *bio, struct bio_set *bs, unsigned *segs,
> +		unsigned max_bytes)
>  {
>  	struct bio_vec bv, bvprv, *bvprvp = NULL;
>  	struct bvec_iter iter;
>  	unsigned nsegs = 0, bytes = 0;
> -	const unsigned max_bytes = get_max_io_size(q, bio) << 9;
>  	const unsigned max_segs = queue_max_segments(q);
>  
>  	bio_for_each_bvec(bv, bio, iter) {
> @@ -347,7 +346,8 @@ void __blk_queue_split(struct request_queue *q, struct bio **bio,
>  		split = blk_bio_write_zeroes_split(q, *bio, bs, nr_segs);
>  		break;
>  	default:
> -		split = blk_bio_segment_split(q, *bio, bs, nr_segs);
> +		split = blk_bio_segment_split(q, *bio, bs, nr_segs,
> +				get_max_io_size(q, *bio) << SECTOR_SHIFT);
>  		break;
>  	}
>  


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] block: move the call to get_max_io_size out of blk_bio_segment_split
  2022-07-22  6:09   ` Damien Le Moal
@ 2022-07-22  6:11     ` Christoph Hellwig
  2022-07-22  6:12       ` Damien Le Moal
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2022-07-22  6:11 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: Christoph Hellwig, Jens Axboe, linux-block

On Fri, Jul 22, 2022 at 03:09:57PM +0900, Damien Le Moal wrote:
> On 7/20/22 23:24, Christoph Hellwig wrote:
> > Prepare for reusing blk_bio_segment_split for (file system controlled)
> > splits of REQ_OP_ZONE_APPEND bios by letting the caller control the
> > maximum size of the bio.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Looks good. Though I think this patch could wait for the actual users of
> this change.

I don't think passing an additional argument is in any way harmful
even now, and I'd like to reduce the cross-tree dependencies for
the next cycle.  With this series in I just need to an an export
in the btrfs series, which would be great.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] block: move the call to get_max_io_size out of blk_bio_segment_split
  2022-07-22  6:11     ` Christoph Hellwig
@ 2022-07-22  6:12       ` Damien Le Moal
  0 siblings, 0 replies; 19+ messages in thread
From: Damien Le Moal @ 2022-07-22  6:12 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block

On 7/22/22 15:11, Christoph Hellwig wrote:
> On Fri, Jul 22, 2022 at 03:09:57PM +0900, Damien Le Moal wrote:
>> On 7/20/22 23:24, Christoph Hellwig wrote:
>>> Prepare for reusing blk_bio_segment_split for (file system controlled)
>>> splits of REQ_OP_ZONE_APPEND bios by letting the caller control the
>>> maximum size of the bio.
>>>
>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>
>> Looks good. Though I think this patch could wait for the actual users of
>> this change.
> 
> I don't think passing an additional argument is in any way harmful
> even now, and I'd like to reduce the cross-tree dependencies for
> the next cycle.  With this series in I just need to an an export
> in the btrfs series, which would be great.

Works for me !


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-07-22  6:12 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-20 14:24 bio splitting cleanups Christoph Hellwig
2022-07-20 14:24 ` [PATCH 1/5] block: move ->bio_split to the gendisk Christoph Hellwig
2022-07-22  5:56   ` Damien Le Moal
2022-07-22  6:00     ` Christoph Hellwig
2022-07-20 14:24 ` [PATCH 2/5] block: fix max_zone_append_sectors inheritance in blk_stack_limits Christoph Hellwig
2022-07-22  5:59   ` Damien Le Moal
2022-07-22  6:03     ` Christoph Hellwig
2022-07-20 14:24 ` [PATCH 3/5] block: move the call to get_max_io_size out of blk_bio_segment_split Christoph Hellwig
2022-07-22  6:09   ` Damien Le Moal
2022-07-22  6:11     ` Christoph Hellwig
2022-07-22  6:12       ` Damien Le Moal
2022-07-20 14:24 ` [PATCH 4/5] block: move bio_allowed_max_sectors to blk-merge.c Christoph Hellwig
2022-07-21  6:31   ` Johannes Thumshirn
2022-07-21  6:42     ` Christoph Hellwig
2022-07-21  6:42       ` Johannes Thumshirn
2022-07-22  6:02   ` Damien Le Moal
2022-07-20 14:24 ` [PATCH 5/5] block: pass struct queue_limits to the bio splitting helpers Christoph Hellwig
2022-07-22  6:08   ` Damien Le Moal
2022-07-21  6:33 ` bio splitting cleanups Johannes Thumshirn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox