Skip to content

Commit

Permalink
block: remove bio_rewind_iter()
Browse files Browse the repository at this point in the history
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 5151842 (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: Kent Overstreet <kent.overstreet@gmail.com>
Acked-by: Kent Overstreet <kent.overstreet@gmail.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
  • Loading branch information
Ming Lei authored and Jens Axboe committed Sep 6, 2018
1 parent 3d0e637 commit 7759eb2
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 30 deletions.
12 changes: 4 additions & 8 deletions block/bio-integrity.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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,
bi->profile->verify_fn);
} else {
bio->bi_status = BLK_STS_IOERR;
}

bio->bi_status = bio_integrity_process(bio, &bip->bio_iter,
bi->profile->verify_fn);
bio_integrity_free(bio);
bio_endio(bio);
}
Expand Down
1 change: 0 additions & 1 deletion block/bio.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
22 changes: 4 additions & 18 deletions include/linux/bio.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,27 +170,11 @@ 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) \
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 0 additions & 3 deletions include/linux/bvec.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
};
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 7759eb2

Please sign in to comment.