On Wed, Jun 27, 2018 at 10:48:06AM -0700, Bart Van Assche wrote: > On 06/26/18 18:13, Ming Lei wrote: > > On Tue, Jun 26, 2018 at 03:26:24PM -0700, Bart Van Assche wrote: > > > There is no good reason to use different code paths for different > > > request operations. Hence remove the switch/case statement from > > > bio_clone_bioset(). > > > > > > Signed-off-by: Bart Van Assche <[email protected]> > > > Cc: Christoph Hellwig <[email protected]> > > > Cc: Ming Lei <[email protected]> > > > --- > > > block/bio.c | 15 ++------------- > > > 1 file changed, 2 insertions(+), 13 deletions(-) > > > > > > diff --git a/block/bio.c b/block/bio.c > > > index f7e3d88bd0b6..4c27cc9ea55e 100644 > > > --- a/block/bio.c > > > +++ b/block/bio.c > > > @@ -691,19 +691,8 @@ struct bio *bio_clone_bioset(struct bio *bio_src, > > > gfp_t gfp_mask, > > > bio->bi_iter.bi_sector = bio_src->bi_iter.bi_sector; > > > bio->bi_iter.bi_size = bio_src->bi_iter.bi_size; > > > - switch (bio_op(bio)) { > > > - case REQ_OP_DISCARD: > > > - case REQ_OP_SECURE_ERASE: > > > - case REQ_OP_WRITE_ZEROES: > > > - break; > > > - case REQ_OP_WRITE_SAME: > > > - bio->bi_io_vec[bio->bi_vcnt++] = bio_src->bi_io_vec[0]; > > > - break; > > > - default: > > > - bio_for_each_segment(bv, bio_src, iter) > > > - bio->bi_io_vec[bio->bi_vcnt++] = bv; > > > - break; > > > - } > > > + bio_for_each_segment(bv, bio_src, iter) > > > + bio->bi_io_vec[bio->bi_vcnt++] = bv; > > > > The above change may not be correct for WRITE_SAME, since > > bio_src->bi_iter.bi_size should be the actual bytes to write by drive. > > Since bio_for_each_segment() neither modifies bio_src->bi_iter nor > bio->bi_iter, the above patch retains the value copied into > bio->bi_iter.bi_size before bio_for_each_segment() was called. In other
Yes. > words, bio_src->bi_iter.bi_size is not modified and the resulting > bio->bi_iter.bi_size should be identical with or without this patch. That is true too. But, What we need to do is to only copy the 1st bvec for WRITE_SAME, your patch changes to copy (bio->bi_iter.bi_size / block size) bvecs, then memory corruption may be triggered. So bio_for_each_segment() can't be used here. Thanks, Ming
