Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/5] Optimize bio splitting
@ 2019-08-01 22:50 Bart Van Assche
  2019-08-01 22:50 ` [PATCH v2 1/5] block: Declare several function pointer arguments 'const' Bart Van Assche
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Bart Van Assche @ 2019-08-01 22:50 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Christoph Hellwig, Bart Van Assche

Hi Jens,

This patch series improves bio splitting in several ways:
- Reduce the number of CPU cycles spent in bio splitting code.
- Make the bio splittig code easier to read.
- Optimize alignment of split bios.

Please consider this patch series for kernel v5.4.

Thanks,

Bart.

Changes compared to v1: addressed Johannes' review feedback.

Bart Van Assche (5):
  block: Declare several function pointer arguments 'const'
  block: Document the bio splitting functions
  block: Simplify bvec_split_segs()
  block: Simplify blk_bio_segment_split()
  block: Improve physical block alignment of split bios

 block/bio.c            |   4 +-
 block/blk-merge.c      | 151 ++++++++++++++++++++++++++++-------------
 include/linux/blkdev.h |  32 ++++-----
 3 files changed, 120 insertions(+), 67 deletions(-)

-- 
2.22.0.770.g0f2c4a37fd-goog


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

* [PATCH v2 1/5] block: Declare several function pointer arguments 'const'
  2019-08-01 22:50 [PATCH v2 0/5] Optimize bio splitting Bart Van Assche
@ 2019-08-01 22:50 ` Bart Van Assche
  2019-09-13 22:57   ` Chaitanya Kulkarni
  2019-08-01 22:50 ` [PATCH v2 2/5] block: Document the bio splitting functions Bart Van Assche
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Bart Van Assche @ 2019-08-01 22:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, Christoph Hellwig, Bart Van Assche,
	Johannes Thumshirn, Christoph Hellwig, Ming Lei, Hannes Reinecke

Make it clear to the compiler and also to humans that the functions
that query request queue properties do not modify any member of the
request_queue data structure.

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-merge.c      |  7 ++++---
 include/linux/blkdev.h | 32 ++++++++++++++++----------------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 57f7990b342d..8344d94f13e0 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -144,7 +144,7 @@ static inline unsigned get_max_io_size(struct request_queue *q,
 	return sectors;
 }
 
-static unsigned get_max_segment_size(struct request_queue *q,
+static unsigned get_max_segment_size(const struct request_queue *q,
 				     unsigned offset)
 {
 	unsigned long mask = queue_segment_boundary(q);
@@ -161,8 +161,9 @@ static unsigned get_max_segment_size(struct request_queue *q,
  * Split the bvec @bv into segments, and update all kinds of
  * variables.
  */
-static bool bvec_split_segs(struct request_queue *q, struct bio_vec *bv,
-		unsigned *nsegs, unsigned *sectors, unsigned max_segs)
+static bool bvec_split_segs(const struct request_queue *q,
+			    const struct bio_vec *bv, unsigned *nsegs,
+			    unsigned *sectors, unsigned max_segs)
 {
 	unsigned len = bv->bv_len;
 	unsigned total_len = 0;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1ef375dafb1c..96a29a72fd4a 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1232,42 +1232,42 @@ enum blk_default_limits {
 	BLK_SEG_BOUNDARY_MASK	= 0xFFFFFFFFUL,
 };
 
-static inline unsigned long queue_segment_boundary(struct request_queue *q)
+static inline unsigned long queue_segment_boundary(const struct request_queue *q)
 {
 	return q->limits.seg_boundary_mask;
 }
 
-static inline unsigned long queue_virt_boundary(struct request_queue *q)
+static inline unsigned long queue_virt_boundary(const struct request_queue *q)
 {
 	return q->limits.virt_boundary_mask;
 }
 
-static inline unsigned int queue_max_sectors(struct request_queue *q)
+static inline unsigned int queue_max_sectors(const struct request_queue *q)
 {
 	return q->limits.max_sectors;
 }
 
-static inline unsigned int queue_max_hw_sectors(struct request_queue *q)
+static inline unsigned int queue_max_hw_sectors(const struct request_queue *q)
 {
 	return q->limits.max_hw_sectors;
 }
 
-static inline unsigned short queue_max_segments(struct request_queue *q)
+static inline unsigned short queue_max_segments(const struct request_queue *q)
 {
 	return q->limits.max_segments;
 }
 
-static inline unsigned short queue_max_discard_segments(struct request_queue *q)
+static inline unsigned short queue_max_discard_segments(const struct request_queue *q)
 {
 	return q->limits.max_discard_segments;
 }
 
-static inline unsigned int queue_max_segment_size(struct request_queue *q)
+static inline unsigned int queue_max_segment_size(const struct request_queue *q)
 {
 	return q->limits.max_segment_size;
 }
 
-static inline unsigned short queue_logical_block_size(struct request_queue *q)
+static inline unsigned short queue_logical_block_size(const struct request_queue *q)
 {
 	int retval = 512;
 
@@ -1282,7 +1282,7 @@ static inline unsigned short bdev_logical_block_size(struct block_device *bdev)
 	return queue_logical_block_size(bdev_get_queue(bdev));
 }
 
-static inline unsigned int queue_physical_block_size(struct request_queue *q)
+static inline unsigned int queue_physical_block_size(const struct request_queue *q)
 {
 	return q->limits.physical_block_size;
 }
@@ -1292,7 +1292,7 @@ static inline unsigned int bdev_physical_block_size(struct block_device *bdev)
 	return queue_physical_block_size(bdev_get_queue(bdev));
 }
 
-static inline unsigned int queue_io_min(struct request_queue *q)
+static inline unsigned int queue_io_min(const struct request_queue *q)
 {
 	return q->limits.io_min;
 }
@@ -1302,7 +1302,7 @@ static inline int bdev_io_min(struct block_device *bdev)
 	return queue_io_min(bdev_get_queue(bdev));
 }
 
-static inline unsigned int queue_io_opt(struct request_queue *q)
+static inline unsigned int queue_io_opt(const struct request_queue *q)
 {
 	return q->limits.io_opt;
 }
@@ -1312,7 +1312,7 @@ static inline int bdev_io_opt(struct block_device *bdev)
 	return queue_io_opt(bdev_get_queue(bdev));
 }
 
-static inline int queue_alignment_offset(struct request_queue *q)
+static inline int queue_alignment_offset(const struct request_queue *q)
 {
 	if (q->limits.misaligned)
 		return -1;
@@ -1342,7 +1342,7 @@ static inline int bdev_alignment_offset(struct block_device *bdev)
 	return q->limits.alignment_offset;
 }
 
-static inline int queue_discard_alignment(struct request_queue *q)
+static inline int queue_discard_alignment(const struct request_queue *q)
 {
 	if (q->limits.discard_misaligned)
 		return -1;
@@ -1432,7 +1432,7 @@ static inline sector_t bdev_zone_sectors(struct block_device *bdev)
 	return 0;
 }
 
-static inline int queue_dma_alignment(struct request_queue *q)
+static inline int queue_dma_alignment(const struct request_queue *q)
 {
 	return q ? q->dma_alignment : 511;
 }
@@ -1543,7 +1543,7 @@ static inline void blk_queue_max_integrity_segments(struct request_queue *q,
 }
 
 static inline unsigned short
-queue_max_integrity_segments(struct request_queue *q)
+queue_max_integrity_segments(const struct request_queue *q)
 {
 	return q->limits.max_integrity_segments;
 }
@@ -1626,7 +1626,7 @@ static inline void blk_queue_max_integrity_segments(struct request_queue *q,
 						    unsigned int segs)
 {
 }
-static inline unsigned short queue_max_integrity_segments(struct request_queue *q)
+static inline unsigned short queue_max_integrity_segments(const struct request_queue *q)
 {
 	return 0;
 }
-- 
2.22.0.770.g0f2c4a37fd-goog


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

* [PATCH v2 2/5] block: Document the bio splitting functions
  2019-08-01 22:50 [PATCH v2 0/5] Optimize bio splitting Bart Van Assche
  2019-08-01 22:50 ` [PATCH v2 1/5] block: Declare several function pointer arguments 'const' Bart Van Assche
@ 2019-08-01 22:50 ` Bart Van Assche
  2019-09-13 23:02   ` Chaitanya Kulkarni
  2019-08-01 22:50 ` [PATCH v2 3/5] block: Simplify bvec_split_segs() Bart Van Assche
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Bart Van Assche @ 2019-08-01 22:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, Christoph Hellwig, Bart Van Assche,
	Johannes Thumshirn, Christoph Hellwig, Ming Lei, Hannes Reinecke

Since what the bio splitting functions do is nontrivial, document these
functions.

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/bio.c       |  4 ++--
 block/blk-merge.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 299a0e7651ec..0fff4eb9eb1e 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1842,8 +1842,8 @@ EXPORT_SYMBOL(bio_endio);
  * @bio, and updates @bio to represent the remaining sectors.
  *
  * Unless this is a discard request the newly allocated bio will point
- * to @bio's bi_io_vec; it is the caller's responsibility to ensure that
- * @bio is not freed before the split.
+ * to @bio's bi_io_vec. It is the caller's responsibility to ensure that
+ * neither @bio nor @bs are freed before the split bio.
  */
 struct bio *bio_split(struct bio *bio, int sectors,
 		      gfp_t gfp, struct bio_set *bs)
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 8344d94f13e0..51ed971709c3 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -195,6 +195,25 @@ static bool bvec_split_segs(const struct request_queue *q,
 	return !!len;
 }
 
+/**
+ * blk_bio_segment_split - split a bio in two bios
+ * @q:    [in] request queue pointer
+ * @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
+ *
+ * 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 queue_max_segments(@q) segments.
+ *
+ * Except for discard requests the cloned bio will point at the bi_io_vec of
+ * the original bio. It is the responsibility of the caller to ensure that the
+ * original bio is not freed before the cloned bio. The caller is also
+ * 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,
 					 struct bio *bio,
 					 struct bio_set *bs,
@@ -251,6 +270,19 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
 	return bio_split(bio, sectors, GFP_NOIO, bs);
 }
 
+/**
+ * __blk_queue_split - split a bio and submit the second half
+ * @q:       [in] request queue pointer
+ * @bio:     [in, out] bio to be split
+ * @nr_segs: [out] number of segments in the first bio
+ *
+ * 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 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)
 {
@@ -295,6 +327,17 @@ void __blk_queue_split(struct request_queue *q, struct bio **bio,
 	}
 }
 
+/**
+ * blk_queue_split - split a bio and submit the second half
+ * @q:   [in] request queue pointer
+ * @bio: [in, out] bio to be split
+ *
+ * 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 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;
-- 
2.22.0.770.g0f2c4a37fd-goog


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

* [PATCH v2 3/5] block: Simplify bvec_split_segs()
  2019-08-01 22:50 [PATCH v2 0/5] Optimize bio splitting Bart Van Assche
  2019-08-01 22:50 ` [PATCH v2 1/5] block: Declare several function pointer arguments 'const' Bart Van Assche
  2019-08-01 22:50 ` [PATCH v2 2/5] block: Document the bio splitting functions Bart Van Assche
@ 2019-08-01 22:50 ` Bart Van Assche
  2019-08-01 22:50 ` [PATCH v2 4/5] block: Simplify blk_bio_segment_split() Bart Van Assche
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Bart Van Assche @ 2019-08-01 22:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, Christoph Hellwig, Bart Van Assche,
	Johannes Thumshirn, Ming Lei, Hannes Reinecke

Simplify this function by by removing two if-tests. Other than requiring
that the @sectors pointer is not NULL, this patch does not change the
behavior of bvec_split_segs().

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-merge.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 51ed971709c3..7cea5050bbcf 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -167,17 +167,17 @@ static bool bvec_split_segs(const struct request_queue *q,
 {
 	unsigned len = bv->bv_len;
 	unsigned total_len = 0;
-	unsigned new_nsegs = 0, seg_size = 0;
+	unsigned seg_size = 0;
 
 	/*
 	 * Multi-page bvec may be too big to hold in one segment, so the
 	 * current bvec has to be splitted as multiple segments.
 	 */
-	while (len && new_nsegs + *nsegs < max_segs) {
+	while (len && *nsegs < max_segs) {
 		seg_size = get_max_segment_size(q, bv->bv_offset + total_len);
 		seg_size = min(seg_size, len);
 
-		new_nsegs++;
+		(*nsegs)++;
 		total_len += seg_size;
 		len -= seg_size;
 
@@ -185,11 +185,7 @@ static bool bvec_split_segs(const struct request_queue *q,
 			break;
 	}
 
-	if (new_nsegs) {
-		*nsegs += new_nsegs;
-		if (sectors)
-			*sectors += total_len >> 9;
-	}
+	*sectors += total_len >> 9;
 
 	/* split in the middle of the bvec if len != 0 */
 	return !!len;
@@ -349,6 +345,7 @@ EXPORT_SYMBOL(blk_queue_split);
 unsigned int blk_recalc_rq_segments(struct request *rq)
 {
 	unsigned int nr_phys_segs = 0;
+	unsigned int nr_sectors = 0;
 	struct req_iterator iter;
 	struct bio_vec bv;
 
@@ -365,7 +362,8 @@ 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, NULL, UINT_MAX);
+		bvec_split_segs(rq->q, &bv, &nr_phys_segs, &nr_sectors,
+				UINT_MAX);
 	return nr_phys_segs;
 }
 
-- 
2.22.0.770.g0f2c4a37fd-goog


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

* [PATCH v2 4/5] block: Simplify blk_bio_segment_split()
  2019-08-01 22:50 [PATCH v2 0/5] Optimize bio splitting Bart Van Assche
                   ` (2 preceding siblings ...)
  2019-08-01 22:50 ` [PATCH v2 3/5] block: Simplify bvec_split_segs() Bart Van Assche
@ 2019-08-01 22:50 ` Bart Van Assche
  2019-08-01 22:50 ` [PATCH v2 5/5] block: Improve physical block alignment of split bios Bart Van Assche
  2019-08-02 13:36 ` [PATCH v2 0/5] Optimize bio splitting Jens Axboe
  5 siblings, 0 replies; 9+ messages in thread
From: Bart Van Assche @ 2019-08-01 22:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, Christoph Hellwig, Bart Van Assche, Ming Lei,
	Hannes Reinecke

Move the max_sectors check into bvec_split_segs() such that a single
call to that function can do all the necessary checks. This patch
optimizes the fast path further, namely if a bvec fits in a page.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-merge.c | 68 +++++++++++++++++++++++------------------------
 1 file changed, 33 insertions(+), 35 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 7cea5050bbcf..a6bc08255b1b 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -157,22 +157,36 @@ static unsigned get_max_segment_size(const struct request_queue *q,
 		     queue_max_segment_size(q));
 }
 
-/*
- * Split the bvec @bv into segments, and update all kinds of
- * variables.
+/**
+ * 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
+ * @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
+ *            bio without exceeding @max_segs
+ * @sectors:  [in,out] Number of sectors in the bio being built. Incremented
+ *            by the number of sectors from @bv that may be appended to that
+ *            bio without exceeding @max_sectors
+ * @max_segs: [in] upper bound for *@nsegs
+ * @max_sectors: [in] upper bound for *@sectors
+ *
+ * When splitting a bio, it can happen that a bvec is encountered that is too
+ * big to fit in a single segment and hence that it has to be split in the
+ * middle. This function verifies whether or not that should happen. The value
+ * %true is returned if and only if appending the entire @bv to a bio with
+ * *@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 *sectors, unsigned max_segs)
+			    unsigned *sectors, unsigned max_segs,
+			    unsigned max_sectors)
 {
-	unsigned len = bv->bv_len;
+	unsigned max_len = (min(max_sectors, UINT_MAX >> 9) - *sectors) << 9;
+	unsigned len = min(bv->bv_len, max_len);
 	unsigned total_len = 0;
 	unsigned seg_size = 0;
 
-	/*
-	 * Multi-page bvec may be too big to hold in one segment, so the
-	 * current bvec has to be splitted as multiple segments.
-	 */
 	while (len && *nsegs < max_segs) {
 		seg_size = get_max_segment_size(q, bv->bv_offset + total_len);
 		seg_size = min(seg_size, len);
@@ -187,8 +201,8 @@ static bool bvec_split_segs(const struct request_queue *q,
 
 	*sectors += total_len >> 9;
 
-	/* split in the middle of the bvec if len != 0 */
-	return !!len;
+	/* tell the caller to split the bvec if it is too big to fit */
+	return len > 0 || bv->bv_len > max_len;
 }
 
 /**
@@ -229,34 +243,18 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
 		if (bvprvp && bvec_gap_to_prev(q, bvprvp, bv.bv_offset))
 			goto split;
 
-		if (sectors + (bv.bv_len >> 9) > max_sectors) {
-			/*
-			 * Consider this a new segment if we're splitting in
-			 * the middle of this vector.
-			 */
-			if (nsegs < max_segs &&
-			    sectors < max_sectors) {
-				/* split in the middle of bvec */
-				bv.bv_len = (max_sectors - sectors) << 9;
-				bvec_split_segs(q, &bv, &nsegs,
-						&sectors, max_segs);
-			}
+		if (nsegs < max_segs &&
+		    sectors + (bv.bv_len >> 9) <= max_sectors &&
+		    bv.bv_offset + bv.bv_len <= PAGE_SIZE) {
+			nsegs++;
+			sectors += bv.bv_len >> 9;
+		} else if (bvec_split_segs(q, &bv, &nsegs, &sectors, max_segs,
+					 max_sectors)) {
 			goto split;
 		}
 
-		if (nsegs == max_segs)
-			goto split;
-
 		bvprv = bv;
 		bvprvp = &bvprv;
-
-		if (bv.bv_offset + bv.bv_len <= PAGE_SIZE) {
-			nsegs++;
-			sectors += bv.bv_len >> 9;
-		} else if (bvec_split_segs(q, &bv, &nsegs, &sectors,
-				max_segs)) {
-			goto split;
-		}
 	}
 
 	*segs = nsegs;
@@ -363,7 +361,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, &nr_sectors,
-				UINT_MAX);
+				UINT_MAX, UINT_MAX);
 	return nr_phys_segs;
 }
 
-- 
2.22.0.770.g0f2c4a37fd-goog


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

* [PATCH v2 5/5] block: Improve physical block alignment of split bios
  2019-08-01 22:50 [PATCH v2 0/5] Optimize bio splitting Bart Van Assche
                   ` (3 preceding siblings ...)
  2019-08-01 22:50 ` [PATCH v2 4/5] block: Simplify blk_bio_segment_split() Bart Van Assche
@ 2019-08-01 22:50 ` Bart Van Assche
  2019-08-02 13:36 ` [PATCH v2 0/5] Optimize bio splitting Jens Axboe
  5 siblings, 0 replies; 9+ messages in thread
From: Bart Van Assche @ 2019-08-01 22:50 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, Christoph Hellwig, Bart Van Assche,
	Christoph Hellwig, Ming Lei, Hannes Reinecke

Consider the following example:
* The logical block size is 4 KB.
* The physical block size is 8 KB.
* max_sectors equals (16 KB >> 9) sectors.
* A non-aligned 4 KB and an aligned 64 KB bio are merged into a single
  non-aligned 68 KB bio.

The current behavior is to split such a bio into (16 KB + 16 KB + 16 KB
+ 16 KB + 4 KB). The start of none of these five bio's is aligned to a
physical block boundary.

This patch ensures that such a bio is split into four aligned and
one non-aligned bio instead of being split into five non-aligned bios.
This improves performance because most block devices can handle aligned
requests faster than non-aligned requests.

Since the physical block size is larger than or equal to the logical
block size, this patch preserves the guarantee that the returned
value is a multiple of the logical block size.

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-merge.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index a6bc08255b1b..48e6725b32ee 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -132,16 +132,29 @@ static struct bio *blk_bio_write_same_split(struct request_queue *q,
 	return bio_split(bio, q->limits.max_write_same_sectors, GFP_NOIO, bs);
 }
 
+/*
+ * Return the maximum number of sectors from the start of a bio that may be
+ * submitted as a single request to a block device. If enough sectors remain,
+ * align the end to the physical block size. Otherwise align the end to the
+ * logical block size. This approach minimizes the number of non-aligned
+ * 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,
 				       struct bio *bio)
 {
 	unsigned sectors = blk_max_size_offset(q, bio->bi_iter.bi_sector);
-	unsigned mask = queue_logical_block_size(q) - 1;
+	unsigned max_sectors = sectors;
+	unsigned pbs = queue_physical_block_size(q) >> SECTOR_SHIFT;
+	unsigned lbs = queue_logical_block_size(q) >> SECTOR_SHIFT;
+	unsigned start_offset = bio->bi_iter.bi_sector & (pbs - 1);
 
-	/* aligned to logical block size */
-	sectors &= ~(mask >> 9);
+	max_sectors += start_offset;
+	max_sectors &= ~(pbs - 1);
+	if (max_sectors > start_offset)
+		return max_sectors - start_offset;
 
-	return sectors;
+	return sectors & (lbs - 1);
 }
 
 static unsigned get_max_segment_size(const struct request_queue *q,
-- 
2.22.0.770.g0f2c4a37fd-goog


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

* Re: [PATCH v2 0/5] Optimize bio splitting
  2019-08-01 22:50 [PATCH v2 0/5] Optimize bio splitting Bart Van Assche
                   ` (4 preceding siblings ...)
  2019-08-01 22:50 ` [PATCH v2 5/5] block: Improve physical block alignment of split bios Bart Van Assche
@ 2019-08-02 13:36 ` Jens Axboe
  5 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2019-08-02 13:36 UTC (permalink / raw)
  To: Bart Van Assche; +Cc: linux-block, Christoph Hellwig

On 8/1/19 4:50 PM, Bart Van Assche wrote:
> Hi Jens,
> 
> This patch series improves bio splitting in several ways:
> - Reduce the number of CPU cycles spent in bio splitting code.
> - Make the bio splittig code easier to read.
> - Optimize alignment of split bios.
> 
> Please consider this patch series for kernel v5.4.

Applied, thanks.


-- 
Jens Axboe


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

* Re: [PATCH v2 1/5] block: Declare several function pointer arguments 'const'
  2019-08-01 22:50 ` [PATCH v2 1/5] block: Declare several function pointer arguments 'const' Bart Van Assche
@ 2019-09-13 22:57   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 9+ messages in thread
From: Chaitanya Kulkarni @ 2019-09-13 22:57 UTC (permalink / raw)
  To: Bart Van Assche, Jens Axboe
  Cc: linux-block, Christoph Hellwig, Johannes Thumshirn,
	Christoph Hellwig, Ming Lei, Hannes Reinecke

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>

On 08/01/2019 03:51 PM, Bart Van Assche wrote:
> Make it clear to the compiler and also to humans that the functions
> that query request queue properties do not modify any member of the
> request_queue data structure.
>
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Hannes Reinecke <hare@suse.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   block/blk-merge.c      |  7 ++++---
>   include/linux/blkdev.h | 32 ++++++++++++++++----------------
>   2 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 57f7990b342d..8344d94f13e0 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -144,7 +144,7 @@ static inline unsigned get_max_io_size(struct request_queue *q,
>   	return sectors;
>   }
>
> -static unsigned get_max_segment_size(struct request_queue *q,
> +static unsigned get_max_segment_size(const struct request_queue *q,
>   				     unsigned offset)
>   {
>   	unsigned long mask = queue_segment_boundary(q);
> @@ -161,8 +161,9 @@ static unsigned get_max_segment_size(struct request_queue *q,
>    * Split the bvec @bv into segments, and update all kinds of
>    * variables.
>    */
> -static bool bvec_split_segs(struct request_queue *q, struct bio_vec *bv,
> -		unsigned *nsegs, unsigned *sectors, unsigned max_segs)
> +static bool bvec_split_segs(const struct request_queue *q,
> +			    const struct bio_vec *bv, unsigned *nsegs,
> +			    unsigned *sectors, unsigned max_segs)
>   {
>   	unsigned len = bv->bv_len;
>   	unsigned total_len = 0;
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 1ef375dafb1c..96a29a72fd4a 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1232,42 +1232,42 @@ enum blk_default_limits {
>   	BLK_SEG_BOUNDARY_MASK	= 0xFFFFFFFFUL,
>   };
>
> -static inline unsigned long queue_segment_boundary(struct request_queue *q)
> +static inline unsigned long queue_segment_boundary(const struct request_queue *q)
>   {
>   	return q->limits.seg_boundary_mask;
>   }
>
> -static inline unsigned long queue_virt_boundary(struct request_queue *q)
> +static inline unsigned long queue_virt_boundary(const struct request_queue *q)
>   {
>   	return q->limits.virt_boundary_mask;
>   }
>
> -static inline unsigned int queue_max_sectors(struct request_queue *q)
> +static inline unsigned int queue_max_sectors(const struct request_queue *q)
>   {
>   	return q->limits.max_sectors;
>   }
>
> -static inline unsigned int queue_max_hw_sectors(struct request_queue *q)
> +static inline unsigned int queue_max_hw_sectors(const struct request_queue *q)
>   {
>   	return q->limits.max_hw_sectors;
>   }
>
> -static inline unsigned short queue_max_segments(struct request_queue *q)
> +static inline unsigned short queue_max_segments(const struct request_queue *q)
>   {
>   	return q->limits.max_segments;
>   }
>
> -static inline unsigned short queue_max_discard_segments(struct request_queue *q)
> +static inline unsigned short queue_max_discard_segments(const struct request_queue *q)
>   {
>   	return q->limits.max_discard_segments;
>   }
>
> -static inline unsigned int queue_max_segment_size(struct request_queue *q)
> +static inline unsigned int queue_max_segment_size(const struct request_queue *q)
>   {
>   	return q->limits.max_segment_size;
>   }
>
> -static inline unsigned short queue_logical_block_size(struct request_queue *q)
> +static inline unsigned short queue_logical_block_size(const struct request_queue *q)
>   {
>   	int retval = 512;
>
> @@ -1282,7 +1282,7 @@ static inline unsigned short bdev_logical_block_size(struct block_device *bdev)
>   	return queue_logical_block_size(bdev_get_queue(bdev));
>   }
>
> -static inline unsigned int queue_physical_block_size(struct request_queue *q)
> +static inline unsigned int queue_physical_block_size(const struct request_queue *q)
>   {
>   	return q->limits.physical_block_size;
>   }
> @@ -1292,7 +1292,7 @@ static inline unsigned int bdev_physical_block_size(struct block_device *bdev)
>   	return queue_physical_block_size(bdev_get_queue(bdev));
>   }
>
> -static inline unsigned int queue_io_min(struct request_queue *q)
> +static inline unsigned int queue_io_min(const struct request_queue *q)
>   {
>   	return q->limits.io_min;
>   }
> @@ -1302,7 +1302,7 @@ static inline int bdev_io_min(struct block_device *bdev)
>   	return queue_io_min(bdev_get_queue(bdev));
>   }
>
> -static inline unsigned int queue_io_opt(struct request_queue *q)
> +static inline unsigned int queue_io_opt(const struct request_queue *q)
>   {
>   	return q->limits.io_opt;
>   }
> @@ -1312,7 +1312,7 @@ static inline int bdev_io_opt(struct block_device *bdev)
>   	return queue_io_opt(bdev_get_queue(bdev));
>   }
>
> -static inline int queue_alignment_offset(struct request_queue *q)
> +static inline int queue_alignment_offset(const struct request_queue *q)
>   {
>   	if (q->limits.misaligned)
>   		return -1;
> @@ -1342,7 +1342,7 @@ static inline int bdev_alignment_offset(struct block_device *bdev)
>   	return q->limits.alignment_offset;
>   }
>
> -static inline int queue_discard_alignment(struct request_queue *q)
> +static inline int queue_discard_alignment(const struct request_queue *q)
>   {
>   	if (q->limits.discard_misaligned)
>   		return -1;
> @@ -1432,7 +1432,7 @@ static inline sector_t bdev_zone_sectors(struct block_device *bdev)
>   	return 0;
>   }
>
> -static inline int queue_dma_alignment(struct request_queue *q)
> +static inline int queue_dma_alignment(const struct request_queue *q)
>   {
>   	return q ? q->dma_alignment : 511;
>   }
> @@ -1543,7 +1543,7 @@ static inline void blk_queue_max_integrity_segments(struct request_queue *q,
>   }
>
>   static inline unsigned short
> -queue_max_integrity_segments(struct request_queue *q)
> +queue_max_integrity_segments(const struct request_queue *q)
>   {
>   	return q->limits.max_integrity_segments;
>   }
> @@ -1626,7 +1626,7 @@ static inline void blk_queue_max_integrity_segments(struct request_queue *q,
>   						    unsigned int segs)
>   {
>   }
> -static inline unsigned short queue_max_integrity_segments(struct request_queue *q)
> +static inline unsigned short queue_max_integrity_segments(const struct request_queue *q)
>   {
>   	return 0;
>   }
>


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

* Re: [PATCH v2 2/5] block: Document the bio splitting functions
  2019-08-01 22:50 ` [PATCH v2 2/5] block: Document the bio splitting functions Bart Van Assche
@ 2019-09-13 23:02   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 9+ messages in thread
From: Chaitanya Kulkarni @ 2019-09-13 23:02 UTC (permalink / raw)
  To: Bart Van Assche, Jens Axboe
  Cc: linux-block, Christoph Hellwig, Johannes Thumshirn,
	Christoph Hellwig, Ming Lei, Hannes Reinecke

Looks good.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>

On 08/01/2019 03:51 PM, Bart Van Assche wrote:
> Since what the bio splitting functions do is nontrivial, document these
> functions.
>
> Reviewed-by: Johannes Thumshirn<jthumshirn@suse.de>
> Cc: Christoph Hellwig<hch@infradead.org>
> Cc: Ming Lei<ming.lei@redhat.com>
> Cc: Hannes Reinecke<hare@suse.com>
> Signed-off-by: Bart Van Assche<bvanassche@acm.org>


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

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01 22:50 [PATCH v2 0/5] Optimize bio splitting Bart Van Assche
2019-08-01 22:50 ` [PATCH v2 1/5] block: Declare several function pointer arguments 'const' Bart Van Assche
2019-09-13 22:57   ` Chaitanya Kulkarni
2019-08-01 22:50 ` [PATCH v2 2/5] block: Document the bio splitting functions Bart Van Assche
2019-09-13 23:02   ` Chaitanya Kulkarni
2019-08-01 22:50 ` [PATCH v2 3/5] block: Simplify bvec_split_segs() Bart Van Assche
2019-08-01 22:50 ` [PATCH v2 4/5] block: Simplify blk_bio_segment_split() Bart Van Assche
2019-08-01 22:50 ` [PATCH v2 5/5] block: Improve physical block alignment of split bios Bart Van Assche
2019-08-02 13:36 ` [PATCH v2 0/5] Optimize bio splitting Jens Axboe

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org
	public-inbox-index linux-block

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git