Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1356429

Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers" broken

From Ming Lei <tom.leiming@gmail.com>
Newsgroups linux.kernel
Subject Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers" broken
Date 2016-03-12 15:30 +0100
Message-ID <rbSIi-7nR-7@gated-at.bofh.it> (permalink)
References (3 earlier) <rbP7I-4zU-9@gated-at.bofh.it> <rbQGu-5Rb-9@gated-at.bofh.it> <rbRVU-6O6-17@gated-at.bofh.it> <rbS5A-6Sv-9@gated-at.bofh.it> <rbSoV-7fy-1@gated-at.bofh.it>
Organization Ming

Show all headers | View raw


On Sat, 12 Mar 2016 05:02:56 -0900
Kent Overstreet <kent.overstreet@gmail.com> wrote:

> Here's the output of the patch below:
> 
> generic/036 11s ...run fstests generic/036 at 2016-03-12 13:58:21
> end 4096 0 ffffea0001d611c0 end2 1024 0 ffffea0001d611c0
> len 1024 offset 0 page ffffea0001d611c0
> KGDB: Waiting for remote debugger
> 
> Your code gives a biovec with bv_len of 4096, the old code gives a biovec with
> bv_len of 1024 (and then we dump every biovec, we see that the bio had only a
> single biovec that did indeed have bv_len == 1024).

I guess we shouldn't have optimized for the case of non-cloned bio, could you
try the following patch?

--
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 1e7248f..4abc129 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -267,11 +267,6 @@ static inline void bio_get_last_bvec(struct bio *bio, struct bio_vec *bv)
 	struct bvec_iter iter = bio->bi_iter;
 	int idx;
 
-	if (!bio_flagged(bio, BIO_CLONED)) {
-		*bv = bio->bi_io_vec[bio->bi_vcnt - 1];
-		return;
-	}
-
 	if (unlikely(!bio_multiple_segments(bio))) {
 		*bv = bio_iovec(bio);
 		return;

Thanks,
Ming

> 
> bio_get_last_bvec() is broken, please revert your patch for v4.5.
> 
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 261353166d..231aec4fa3 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -318,6 +318,34 @@ static int blk_phys_contig_segment(struct request_queue *q, struct bio *bio,
>  	bio_get_last_bvec(bio, &end_bv);
>  	bio_get_first_bvec(nxt, &nxt_bv);
>  
> +	{
> +		struct bvec_iter iter;
> +		struct bio_vec end2, bv;
> +
> +		bio_for_each_segment(end2, bio, iter)
> +			if (end2.bv_len == iter.bi_size)
> +				break;
> +
> +		if (end_bv.bv_len	!= end2.bv_len ||
> +		    end_bv.bv_offset	!= end2.bv_offset ||
> +		    end_bv.bv_page	!= end2.bv_page) {
> +			printk(KERN_ERR "end %u %u %p end2 %u %u %p\n",
> +			      end_bv.bv_len,
> +			      end_bv.bv_offset,
> +			      end_bv.bv_page,
> +			      end2.bv_len,
> +			      end2.bv_offset,
> +			      end2.bv_page);
> +
> +			bio_for_each_segment(bv, bio, iter)
> +				printk(KERN_ERR "len %u offset %u page %p\n",
> +				       bv.bv_len,
> +				       bv.bv_offset,
> +				       bv.bv_page);
> +			BUG();
> +		}
> +	}
> +
>  	if (!BIOVEC_PHYS_MERGEABLE(&end_bv, &nxt_bv))
>  		return 0;
>  

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

e827091cb1 "block: merge: get the 1st and last bvec via helpers"  broken Kent Overstreet <kent.overstreet@gmail.com> - 2016-03-12 08:50 +0100
  Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers" broken Ming Lin <mlin@kernel.org> - 2016-03-12 09:10 +0100
  Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers" broken Ming Lei <tom.leiming@gmail.com> - 2016-03-12 09:50 +0100
    Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers"  broken Kent Overstreet <kent.overstreet@gmail.com> - 2016-03-12 10:30 +0100
      Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers" broken Ming Lei <tom.leiming@gmail.com> - 2016-03-12 11:40 +0100
        Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers"  broken Kent Overstreet <kent.overstreet@gmail.com> - 2016-03-12 13:20 +0100
          Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers" broken Ming Lei <tom.leiming@gmail.com> - 2016-03-12 14:40 +0100
            Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers"  broken Kent Overstreet <kent.overstreet@gmail.com> - 2016-03-12 14:50 +0100
              Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers"  broken Kent Overstreet <kent.overstreet@gmail.com> - 2016-03-12 15:10 +0100
                Re: e827091cb1 "block: merge: get the 1st and last bvec via  helpers" broken Ming Lei <tom.leiming@gmail.com> - 2016-03-12 15:30 +0100
                Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers"  broken Kent Overstreet <kent.overstreet@gmail.com> - 2016-03-12 15:40 +0100
                Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers" broken Ming Lei <tom.leiming@gmail.com> - 2016-03-12 15:50 +0100
                Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers" broken Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-12 21:00 +0100
                Re: e827091cb1 "block: merge: get the 1st and last bvec via helpers"  broken Jens Axboe <axboe@kernel.dk> - 2016-03-12 22:20 +0100

csiph-web