All of lore.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

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

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

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

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.