diff mbox series

[1/5] block: take chunk_sectors into account in bio_split_write_zeroes

Message ID 20241104062647.91160-2-hch@lst.de (mailing list archive)
State New
Headers show
Series [1/5] block: take chunk_sectors into account in bio_split_write_zeroes | expand

Commit Message

Christoph Hellwig Nov. 4, 2024, 6:26 a.m. UTC
For zoned devices, write zeroes must be split at the zone boundary
which is represented as chunk_sectors.  For other uses like the
internally RAIDed NVMe devices it is probably at least useful.

Enhance get_max_io_size to know about write zeroes and use it in
bio_split_write_zeroes.  Also add a comment about the seemingly
nonsensical zero max_write_zeroes limit.

Fixes: 885fa13f6559 ("block: implement splitting of REQ_OP_WRITE_ZEROES bios")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-merge.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

Comments

Damien Le Moal Nov. 7, 2024, 12:04 p.m. UTC | #1
On 11/4/24 15:26, Christoph Hellwig wrote:
> For zoned devices, write zeroes must be split at the zone boundary
> which is represented as chunk_sectors.  For other uses like the
> internally RAIDed NVMe devices it is probably at least useful.
> 
> Enhance get_max_io_size to know about write zeroes and use it in
> bio_split_write_zeroes.  Also add a comment about the seemingly
> nonsensical zero max_write_zeroes limit.
> 
> Fixes: 885fa13f6559 ("block: implement splitting of REQ_OP_WRITE_ZEROES bios")
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Jens Axboe Nov. 11, 2024, 4:08 p.m. UTC | #2
On Mon, 04 Nov 2024 07:26:29 +0100, Christoph Hellwig wrote:
> For zoned devices, write zeroes must be split at the zone boundary
> which is represented as chunk_sectors.  For other uses like the
> internally RAIDed NVMe devices it is probably at least useful.
> 
> Enhance get_max_io_size to know about write zeroes and use it in
> bio_split_write_zeroes.  Also add a comment about the seemingly
> nonsensical zero max_write_zeroes limit.
> 
> [...]

Applied, thanks!

[1/5] block: take chunk_sectors into account in bio_split_write_zeroes
      commit: 60dc5ea6bcfd078b71419640d49afa649acf9450
[2/5] block: fix bio_split_rw_at to take zone_write_granularity into account
      commit: 7ecd2cd4fae3e8410c0a6620f3a83dcdbb254f02
[3/5] block: lift bio_is_zone_append to bio.h
      commit: 0ef2b9e698dbf9ba78f67952a747f35eb7060470

Best regards,
diff mbox series

Patch

diff --git a/block/blk-merge.c b/block/blk-merge.c
index d813d799cee7..f440919b6c6f 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -166,17 +166,6 @@  struct bio *bio_split_discard(struct bio *bio, const struct queue_limits *lim,
 	return bio_submit_split(bio, split_sectors);
 }
 
-struct bio *bio_split_write_zeroes(struct bio *bio,
-		const struct queue_limits *lim, unsigned *nsegs)
-{
-	*nsegs = 0;
-	if (!lim->max_write_zeroes_sectors)
-		return bio;
-	if (bio_sectors(bio) <= lim->max_write_zeroes_sectors)
-		return bio;
-	return bio_submit_split(bio, lim->max_write_zeroes_sectors);
-}
-
 static inline unsigned int blk_boundary_sectors(const struct queue_limits *lim,
 						bool is_atomic)
 {
@@ -211,7 +200,9 @@  static inline unsigned get_max_io_size(struct bio *bio,
 	 * We ignore lim->max_sectors for atomic writes because it may less
 	 * than the actual bio size, which we cannot tolerate.
 	 */
-	if (is_atomic)
+	if (bio_op(bio) == REQ_OP_WRITE_ZEROES)
+		max_sectors = lim->max_write_zeroes_sectors;
+	else if (is_atomic)
 		max_sectors = lim->atomic_write_max_sectors;
 	else
 		max_sectors = lim->max_sectors;
@@ -398,6 +389,26 @@  struct bio *bio_split_zone_append(struct bio *bio,
 	return bio_submit_split(bio, split_sectors);
 }
 
+struct bio *bio_split_write_zeroes(struct bio *bio,
+		const struct queue_limits *lim, unsigned *nsegs)
+{
+	unsigned int max_sectors = get_max_io_size(bio, lim);
+
+	*nsegs = 0;
+
+	/*
+	 * An unset limit should normally not happen, as bio submission is keyed
+	 * off having a non-zero limit.  But SCSI can clear the limit in the
+	 * I/O completion handler, and we can race and see this.  Splitting to a
+	 * zero limit obviously doesn't make sense, so band-aid it here.
+	 */
+	if (!max_sectors)
+		return bio;
+	if (bio_sectors(bio) <= max_sectors)
+		return bio;
+	return bio_submit_split(bio, max_sectors);
+}
+
 /**
  * bio_split_to_limits - split a bio to fit the queue limits
  * @bio:     bio to be split