Commit 7759eb23 authored by Ming Lei's avatar Ming Lei Committed by Jens Axboe
Browse files

block: remove bio_rewind_iter() 

It is pointed that bio_rewind_iter() is one very bad API[1]:

1) bio size may not be restored after rewinding

2) it causes some bogus change, such as 5151842b (block: reset
bi_iter.bi_done after splitting bio)

3) rewinding really makes things complicated wrt. bio splitting

4) unnecessary updating of .bi_done in fast path

[1] https://marc.info/?t=153549924200005&r=1&w=2



So this patch takes Kent's suggestion to restore one bio into its original
state via saving bio iterator(struct bvec_iter) in bio_integrity_prep(),
given now bio_rewind_iter() is only used by bio integrity code.

Cc: Dmitry Monakhov <dmonakhov@openvz.org>
Cc: Hannes Reinecke <hare@suse.com>
Suggested-by: default avatarKent Overstreet <kent.overstreet@gmail.com>
Acked-by: default avatarKent Overstreet <kent.overstreet@gmail.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarMing Lei <ming.lei@redhat.com>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 3d0e6375

block/bio-integrity.c

+4 −8
Original line number Diff line number Diff line
@@ -306,6 +306,8 @@ bool bio_integrity_prep(struct bio *bio) 
	if (bio_data_dir(bio) == WRITE) {

		bio_integrity_process(bio, &bio->bi_iter,

				      bi->profile->generate_fn);

	} else {

		bip->bio_iter = bio->bi_iter;

	}

	return true;
@@ -331,20 +333,14 @@ static void bio_integrity_verify_fn(struct work_struct *work) 
		container_of(work, struct bio_integrity_payload, bip_work);

	struct bio *bio = bip->bip_bio;

	struct blk_integrity *bi = blk_get_integrity(bio->bi_disk);

	struct bvec_iter iter = bio->bi_iter;



	/*

	 * At the moment verify is called bio's iterator was advanced

	 * during split and completion, we need to rewind iterator to

	 * it's original position.

	 */

	if (bio_rewind_iter(bio, &iter, iter.bi_done)) {

		bio->bi_status = bio_integrity_process(bio, &iter,

	bio->bi_status = bio_integrity_process(bio, &bip->bio_iter,

						bi->profile->verify_fn);

	} else {

		bio->bi_status = BLK_STS_IOERR;

	}



	bio_integrity_free(bio);

	bio_endio(bio);

}

block/bio.c

+0 −1
Original line number Diff line number Diff line
@@ -1807,7 +1807,6 @@ struct bio *bio_split(struct bio *bio, int sectors, 
		bio_integrity_trim(split);



	bio_advance(bio, split->bi_iter.bi_size);

	bio->bi_iter.bi_done = 0;



	if (bio_flagged(bio, BIO_TRACE_COMPLETION))

		bio_set_flag(split, BIO_TRACE_COMPLETION);

include/linux/bio.h

+4 −18
Original line number Diff line number Diff line
@@ -170,28 +170,12 @@ static inline void bio_advance_iter(struct bio *bio, struct bvec_iter *iter, 
{

	iter->bi_sector += bytes >> 9;



	if (bio_no_advance_iter(bio)) {

	if (bio_no_advance_iter(bio))

		iter->bi_size -= bytes;

		iter->bi_done += bytes;

	} else {

	else

		bvec_iter_advance(bio->bi_io_vec, iter, bytes);

		/* TODO: It is reasonable to complete bio with error here. */

}

}



static inline bool bio_rewind_iter(struct bio *bio, struct bvec_iter *iter,

		unsigned int bytes)

{

	iter->bi_sector -= bytes >> 9;



	if (bio_no_advance_iter(bio)) {

		iter->bi_size += bytes;

		iter->bi_done -= bytes;

		return true;

	}



	return bvec_iter_rewind(bio->bi_io_vec, iter, bytes);

}



#define __bio_for_each_segment(bvl, bio, iter, start)			\

	for (iter = (start);						\
@@ -353,6 +337,8 @@ struct bio_integrity_payload { 
	unsigned short		bip_max_vcnt;	/* integrity bio_vec slots */

	unsigned short		bip_flags;	/* control flags */



	struct bvec_iter	bio_iter;	/* for rewinding parent bio */



	struct work_struct	bip_work;	/* I/O completion */



	struct bio_vec		*bip_vec;

include/linux/bvec.h

+0 −3
Original line number Diff line number Diff line
@@ -40,8 +40,6 @@ struct bvec_iter { 


	unsigned int		bi_idx;		/* current index into bvl_vec */



	unsigned int            bi_done;	/* number of bytes completed */



	unsigned int            bi_bvec_done;	/* number of bytes completed in

						   current bvec */

};
@@ -85,7 +83,6 @@ static inline bool bvec_iter_advance(const struct bio_vec *bv, 
		bytes -= len;

		iter->bi_size -= len;

		iter->bi_bvec_done += len;

		iter->bi_done += len;



		if (iter->bi_bvec_done == __bvec_iter_bvec(bv, *iter)->bv_len) {

			iter->bi_bvec_done = 0;

CRA Git | Maintained and supported by SUSTech CRA and CCSE