Re: [Cluster-devel] [PATCH V12 09/20] block: use bio_for_each_bvec() to compute multi-page bvec count
On Mon, Nov 26, 2018 at 10:17:09AM +0800, Ming Lei wrote:
> First it is more efficient to use bio_for_each_bvec() in both
> blk_bio_segment_split() and __blk_recalc_rq_segments() to compute how
> many multi-page bvecs there are in the bio.
>
> Secondly once bio_for_each_bvec() is used, the bvec may need to be
> splitted because its length can be very longer than max segment size,
> so we have to split the big bvec into several segments.
>
> Thirdly when splitting multi-page bvec into segments, the max segment
> limit may be reached, so the bio split need to be considered under
> this situation too.
Reviewed-by: Omar Sandoval
> Signed-off-by: Ming Lei
> ---
> block/blk-merge.c | 100
> +++---
> 1 file changed, 80 insertions(+), 20 deletions(-)
>
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 51ec6ca56a0a..2d8f388d43de 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -161,6 +161,70 @@ static inline unsigned get_max_io_size(struct
> request_queue *q,
> return sectors;
> }
>
> +static unsigned get_max_segment_size(struct request_queue *q,
> + unsigned offset)
> +{
> + unsigned long mask = queue_segment_boundary(q);
> +
> + return min_t(unsigned long, mask - (mask & offset) + 1,
> + queue_max_segment_size(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 *last_seg_size,
> + unsigned *front_seg_size, unsigned *sectors)
> +{
> + unsigned len = bv->bv_len;
> + unsigned total_len = 0;
> + unsigned new_nsegs = 0, seg_size = 0;
> +
> + /*
> + * Multipage 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 < queue_max_segments(q)) {
> + seg_size = get_max_segment_size(q, bv->bv_offset + total_len);
> + seg_size = min(seg_size, len);
> +
> + new_nsegs++;
> + total_len += seg_size;
> + len -= seg_size;
> +
> + if ((bv->bv_offset + total_len) & queue_virt_boundary(q))
> + break;
> + }
> +
> + if (!new_nsegs)
> + return !!len;
> +
> + /* update front segment size */
> + if (!*nsegs) {
> + unsigned first_seg_size;
> +
> + if (new_nsegs == 1)
> + first_seg_size = get_max_segment_size(q, bv->bv_offset);
> + else
> + first_seg_size = queue_max_segment_size(q);
> +
> + if (*front_seg_size < first_seg_size)
> + *front_seg_size = first_seg_size;
> + }
> +
> + /* update other varibles */
> + *last_seg_size = seg_size;
> + *nsegs += new_nsegs;
> + if (sectors)
> + *sectors += total_len >> 9;
> +
> + /* split in the middle of the bvec if len != 0 */
> + return !!len;
> +}
> +
> static struct bio *blk_bio_segment_split(struct request_queue *q,
>struct bio *bio,
>struct bio_set *bs,
> @@ -174,7 +238,7 @@ static struct bio *blk_bio_segment_split(struct
> request_queue *q,
> struct bio *new = NULL;
> const unsigned max_sectors = get_max_io_size(q, bio);
>
> - bio_for_each_segment(bv, bio, iter) {
> + 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.
> @@ -189,8 +253,12 @@ static struct bio *blk_bio_segment_split(struct
> request_queue *q,
>*/
> if (nsegs < queue_max_segments(q) &&
> sectors < max_sectors) {
> - nsegs++;
> - sectors = max_sectors;
> + /* split in the middle of bvec */
> + bv.bv_len = (max_sectors - sectors) << 9;
> + bvec_split_segs(q, , ,
> + _size,
> + _seg_size,
> + );
> }
> goto split;
> }
> @@ -212,14 +280,12 @@ static struct bio *blk_bio_segment_split(struct
> request_queue *q,
> if (nsegs == queue_max_segments(q))
> goto split;
>
> - if (nsegs == 1 && seg_size > front_seg_size)
> - front_seg_size = seg_size;
> -
> - nsegs++;
> bvprv = bv;
> bvprvp =
> - seg_size = bv.bv_len;
> - sectors += bv.bv_len >> 9;
Re: [Cluster-devel] [PATCH V12 09/20] block: use bio_for_each_bvec() to compute multi-page bvec count
Looks fine:
Reviewed-by: Christoph Hellwig
Nitpick below:
> +{
> + unsigned len = bv->bv_len;
> + unsigned total_len = 0;
> + unsigned new_nsegs = 0, seg_size = 0;
> +
> + /*
> + * Multipage bvec may be too big to hold in one segment,
> + * so the current bvec has to be splitted as multiple
> + * segments.
> + */
Please use up all 80 chars in a line.
[Cluster-devel] [PATCH V12 09/20] block: use bio_for_each_bvec() to compute multi-page bvec count
First it is more efficient to use bio_for_each_bvec() in both
blk_bio_segment_split() and __blk_recalc_rq_segments() to compute how
many multi-page bvecs there are in the bio.
Secondly once bio_for_each_bvec() is used, the bvec may need to be
splitted because its length can be very longer than max segment size,
so we have to split the big bvec into several segments.
Thirdly when splitting multi-page bvec into segments, the max segment
limit may be reached, so the bio split need to be considered under
this situation too.
Signed-off-by: Ming Lei
---
block/blk-merge.c | 100 +++---
1 file changed, 80 insertions(+), 20 deletions(-)
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 51ec6ca56a0a..2d8f388d43de 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -161,6 +161,70 @@ static inline unsigned get_max_io_size(struct
request_queue *q,
return sectors;
}
+static unsigned get_max_segment_size(struct request_queue *q,
+unsigned offset)
+{
+ unsigned long mask = queue_segment_boundary(q);
+
+ return min_t(unsigned long, mask - (mask & offset) + 1,
+queue_max_segment_size(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 *last_seg_size,
+ unsigned *front_seg_size, unsigned *sectors)
+{
+ unsigned len = bv->bv_len;
+ unsigned total_len = 0;
+ unsigned new_nsegs = 0, seg_size = 0;
+
+ /*
+* Multipage 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 < queue_max_segments(q)) {
+ seg_size = get_max_segment_size(q, bv->bv_offset + total_len);
+ seg_size = min(seg_size, len);
+
+ new_nsegs++;
+ total_len += seg_size;
+ len -= seg_size;
+
+ if ((bv->bv_offset + total_len) & queue_virt_boundary(q))
+ break;
+ }
+
+ if (!new_nsegs)
+ return !!len;
+
+ /* update front segment size */
+ if (!*nsegs) {
+ unsigned first_seg_size;
+
+ if (new_nsegs == 1)
+ first_seg_size = get_max_segment_size(q, bv->bv_offset);
+ else
+ first_seg_size = queue_max_segment_size(q);
+
+ if (*front_seg_size < first_seg_size)
+ *front_seg_size = first_seg_size;
+ }
+
+ /* update other varibles */
+ *last_seg_size = seg_size;
+ *nsegs += new_nsegs;
+ if (sectors)
+ *sectors += total_len >> 9;
+
+ /* split in the middle of the bvec if len != 0 */
+ return !!len;
+}
+
static struct bio *blk_bio_segment_split(struct request_queue *q,
struct bio *bio,
struct bio_set *bs,
@@ -174,7 +238,7 @@ static struct bio *blk_bio_segment_split(struct
request_queue *q,
struct bio *new = NULL;
const unsigned max_sectors = get_max_io_size(q, bio);
- bio_for_each_segment(bv, bio, iter) {
+ 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.
@@ -189,8 +253,12 @@ static struct bio *blk_bio_segment_split(struct
request_queue *q,
*/
if (nsegs < queue_max_segments(q) &&
sectors < max_sectors) {
- nsegs++;
- sectors = max_sectors;
+ /* split in the middle of bvec */
+ bv.bv_len = (max_sectors - sectors) << 9;
+ bvec_split_segs(q, , ,
+ _size,
+ _seg_size,
+ );
}
goto split;
}
@@ -212,14 +280,12 @@ static struct bio *blk_bio_segment_split(struct
request_queue *q,
if (nsegs == queue_max_segments(q))
goto split;
- if (nsegs == 1 && seg_size > front_seg_size)
- front_seg_size = seg_size;
-
- nsegs++;
bvprv = bv;
bvprvp =
- seg_size = bv.bv_len;
- sectors += bv.bv_len >> 9;
+
+ if (bvec_split_segs(q, , , _size,
+ _seg_size, ))
+ goto split;
}
@@ -233,8 +299,6 @@ static struct bio
