> +     bool new_bio;
>  
>       if (!bio)
>               return 0;
> @@ -377,9 +377,10 @@ static unsigned int __blk_recalc_rq_segments(struct 
> request_queue *q,
>       fbio = bio;
>       seg_size = 0;
>       nr_phys_segs = 0;
> +     new_bio = false;

I'd just initialize it to false in the declaration line.

> +/* only try to merge bvecs into one sg if they are from two bios */
> +static inline bool
> +__blk_segment_map_sg_merge(struct request_queue *q, struct bio_vec *bvec,
> +                        struct bio_vec *bvprv, struct scatterlist **sg)
>  {
>       int nbytes = bvec->bv_len;
>  
> +     if (!*sg)
> +             return false;
>  
> +     if ((*sg)->length + nbytes > queue_max_segment_size(q))
> +             return false;
> +
> +     if (!biovec_phys_mergeable(q, bvprv, bvec))
> +             return false;
> +
> +     (*sg)->length += nbytes;
> +
> +     return true;
> +}
> +
> +static inline void
> +__blk_segment_map_sg(struct request_queue *q, struct bio_vec *bvec,
> +                  struct scatterlist *sglist, struct scatterlist **sg,
> +                  int *nsegs)
> +{
> +     if (bvec->bv_offset + bvec->bv_len <= PAGE_SIZE) {
> +             *sg = blk_next_sg(sg, sglist);
> +             sg_set_page(*sg, bvec->bv_page, bvec->bv_len, bvec->bv_offset);
> +             (*nsegs) += 1;

This branch is basically the same as __blk_bvec_map_sg, I wonder if
we can reuse it.

> +     } else
> +             (*nsegs) += blk_bvec_map_sg(q, bvec, sglist, sg);
>  }

And then maybe just kill off __blk_segment_map_sg entirely by moving
the if else into the caller.

> @@ -535,11 +543,24 @@ static int __blk_bios_map_sg(struct request_queue *q, 
> struct bio *bio,
>       struct bio_vec bvec, bvprv = { NULL };
>       struct bvec_iter iter;
>       int nsegs = 0;
> +     bool new_bio = false;
>  
> +     for_each_bio(bio) {
> +             bio_for_each_bvec(bvec, bio, iter) {
> +                     /*
> +                      * Only try to merge bvecs from two bios given we
> +                      * have done bio internal merge when adding pages
> +                      * to bio
> +                      */
> +                     if (!new_bio || !__blk_segment_map_sg_merge(q,
> +                                             &bvec, &bvprv, sg))

Somewhat hard to read.  Why not:

                        if (new_bio ||
                            !__blk_segment_map_sg_merge(q, &bvec, &bvprv, sg))

That being said I'd really like to see some stats on workloads that
actually trigger cross-bio segment merges.  If we get a lot of those
we are doing something wrong.

Reply via email to