[PATCH] block: fix build warning in __blk_bios_map_sg()

[PATCH] block: fix build warning in __blk_bios_map_sg()

[Ming Lei]

Commit f6970f83ef79 ("block: don't check if adjacent bvecs in one bio can
be mergeable") changes bvec merge by only considering two bvecs from
different bios. However, if the former bio doesn't inlcude any io bvec,
then the following warning may be triggered:

 warning: ‘bvec.bv_offset’ may be used uninitialized in this function [-Wmaybe-uninitialized]

In practice, it shouldn't be triggered.

Fixes it by adding check on former bio, the check shouldn't add any cost
given 'bio->bi_iter' can be hit in cache.

Reported-by: Jens Axboe <axboe@kernel.dk>
Fixes: f6970f83ef79 ("block: don't check if adjacent bvecs in one bio can be mergeable")
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-merge.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 8f96d683b577..1fc66d03893f 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -527,7 +527,7 @@ static int __blk_bios_map_sg(struct request_queue *q, struct bio *bio,
 			     struct scatterlist *sglist,
 			     struct scatterlist **sg)
 {
-	struct bio_vec bvec, bvprv = { NULL };
+	struct bio_vec uninitialized_var(bvec), bvprv = { NULL };
 	struct bvec_iter iter;
 	int nsegs = 0;
 	bool new_bio = false;
@@ -550,8 +550,10 @@ static int __blk_bios_map_sg(struct request_queue *q, struct bio *bio,
  next_bvec:
 			new_bio = false;
 		}
-		bvprv = bvec;
-		new_bio = true;
+		if (likely(bio->bi_iter.bi_size)) {
+			bvprv = bvec;
+			new_bio = true;
+		}
 	}
 
 	return nsegs;
-- 
2.9.5

Re: [PATCH] block: fix build warning in __blk_bios_map_sg()

[Ming Lei]

On Tue, Apr 02, 2019 at 09:46:38AM +0800, Ming Lei wrote:
> Commit f6970f83ef79 ("block: don't check if adjacent bvecs in one bio can
> be mergeable") changes bvec merge by only considering two bvecs from
> different bios. However, if the former bio doesn't inlcude any io bvec,
> then the following warning may be triggered:
> 
>  warning: ‘bvec.bv_offset’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> In practice, it shouldn't be triggered.
> 
> Fixes it by adding check on former bio, the check shouldn't add any cost
> given 'bio->bi_iter' can be hit in cache.
> 
> Reported-by: Jens Axboe <axboe@kernel.dk>
> Fixes: f6970f83ef79 ("block: don't check if adjacent bvecs in one bio can be mergeable")
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  block/blk-merge.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 8f96d683b577..1fc66d03893f 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -527,7 +527,7 @@ static int __blk_bios_map_sg(struct request_queue *q, struct bio *bio,
>  			     struct scatterlist *sglist,
>  			     struct scatterlist **sg)
>  {
> -	struct bio_vec bvec, bvprv = { NULL };
> +	struct bio_vec uninitialized_var(bvec), bvprv = { NULL };
>  	struct bvec_iter iter;
>  	int nsegs = 0;
>  	bool new_bio = false;
> @@ -550,8 +550,10 @@ static int __blk_bios_map_sg(struct request_queue *q, struct bio *bio,
>   next_bvec:
>  			new_bio = false;
>  		}
> -		bvprv = bvec;
> -		new_bio = true;
> +		if (likely(bio->bi_iter.bi_size)) {
> +			bvprv = bvec;
> +			new_bio = true;
> +		}
>  	}

Looks __blk_recalc_rq_segments() needs the similar fix too, so please
ignore this one, and I will handle them all in one patch.

Thanks,
Ming