ACK: [SRU][Xenial][PATCH 1/1] block: fix bio_will_gap() for first bvec with offset

[Sultan Alsawaf]

Looks good and doesn't have a subsequent 'Fixes' commit in upstream.

Acked-by: Sultan Alsawaf <sultan.alsawaf at canonical.com>

On Thu, Mar 26, 2020 at 03:27:36PM -0300, Heitor Alves de Siqueira wrote:
> From: Ming Lei <ming.lei at redhat.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1869229
> 
> Commit 729204ef49ec("block: relax check on sg gap") allows us to merge
> bios, if both are physically contiguous.  This change can merge a huge
> number of small bios, through mkfs for example, mkfs.ntfs running time
> can be decreased to ~1/10.
> 
> But if one rq starts with a non-aligned buffer (the 1st bvec's bv_offset
> is non-zero) and if we allow the merge, it is quite difficult to respect
> sg gap limit, especially the max segment size, or we risk having an
> unaligned virtual boundary.  This patch tries to avoid the issue by
> disallowing a merge, if the req starts with an unaligned buffer.
> 
> Also add comments to explain why the merged segment can't end in
> unaligned virt boundary.
> 
> Fixes: 729204ef49ec ("block: relax check on sg gap")
> Tested-by: Johannes Thumshirn <jthumshirn at suse.de>
> Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
> Signed-off-by: Ming Lei <ming.lei at redhat.com>
> 
> Rewrote parts of the commit message and comments.
> 
> Signed-off-by: Jens Axboe <axboe at fb.com>
> (cherry picked from commit 5a8d75a1b8c99bdc926ba69b7b7dbe4fae81a5af)
> Signed-off-by: Matthew Ruffell <matthew.ruffell at canonical.com>
> Signed-off-by: Heitor Alves de Siqueira <halves at canonical.com>
> ---
>  include/linux/blkdev.h | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 7548f332121a..01a696b0a4d3 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1672,12 +1672,36 @@ static inline bool bios_segs_mergeable(struct request_queue *q,
>  	return true;
>  }
>  
> -static inline bool bio_will_gap(struct request_queue *q, struct bio *prev,
> -			 struct bio *next)
> +static inline bool bio_will_gap(struct request_queue *q,
> +				struct request *prev_rq,
> +				struct bio *prev,
> +				struct bio *next)
>  {
>  	if (bio_has_data(prev) && queue_virt_boundary(q)) {
>  		struct bio_vec pb, nb;
>  
> +		/*
> +		 * don't merge if the 1st bio starts with non-zero
> +		 * offset, otherwise it is quite difficult to respect
> +		 * sg gap limit. We work hard to merge a huge number of small
> +		 * single bios in case of mkfs.
> +		 */
> +		if (prev_rq)
> +			bio_get_first_bvec(prev_rq->bio, &pb);
> +		else
> +			bio_get_first_bvec(prev, &pb);
> +		if (pb.bv_offset)
> +			return true;
> +
> +		/*
> +		 * We don't need to worry about the situation that the
> +		 * merged segment ends in unaligned virt boundary:
> +		 *
> +		 * - if 'pb' ends aligned, the merged segment ends aligned
> +		 * - if 'pb' ends unaligned, the next bio must include
> +		 *   one single bvec of 'nb', otherwise the 'nb' can't
> +		 *   merge with 'pb'
> +		 */
>  		bio_get_last_bvec(prev, &pb);
>  		bio_get_first_bvec(next, &nb);
>  
> @@ -1690,12 +1714,12 @@ static inline bool bio_will_gap(struct request_queue *q, struct bio *prev,
>  
>  static inline bool req_gap_back_merge(struct request *req, struct bio *bio)
>  {
> -	return bio_will_gap(req->q, req->biotail, bio);
> +	return bio_will_gap(req->q, req, req->biotail, bio);
>  }
>  
>  static inline bool req_gap_front_merge(struct request *req, struct bio *bio)
>  {
> -	return bio_will_gap(req->q, bio, req->bio);
> +	return bio_will_gap(req->q, NULL, bio, req->bio);
>  }
>  
>  int kblockd_schedule_work(struct work_struct *work);
> -- 
> 2.26.0
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team