diff options
author | Mike Snitzer <snitzer@redhat.com> | 2015-05-22 09:14:03 -0400 |
---|---|---|
committer | Jens Axboe <axboe@fb.com> | 2015-05-22 10:58:55 -0400 |
commit | 326e1dbb57368087a36607aaebe9795b8d5453e5 (patch) | |
tree | bbc17b38188d4f2d45d255f1254d63494d37e1e4 /fs/btrfs/volumes.c | |
parent | b04a5636a665f5529fdf69ee7e5512156196f31c (diff) |
block: remove management of bi_remaining when restoring original bi_end_io
Commit c4cf5261 ("bio: skip atomic inc/dec of ->bi_remaining for
non-chains") regressed all existing callers that followed this pattern:
1) saving a bio's original bi_end_io
2) wiring up an intermediate bi_end_io
3) restoring the original bi_end_io from intermediate bi_end_io
4) calling bio_endio() to execute the restored original bi_end_io
The regression was due to BIO_CHAIN only ever getting set if
bio_inc_remaining() is called. For the above pattern it isn't set until
step 3 above (step 2 would've needed to establish BIO_CHAIN). As such
the first bio_endio(), in step 2 above, never decremented __bi_remaining
before calling the intermediate bi_end_io -- leaving __bi_remaining with
the value 1 instead of 0. When bio_inc_remaining() occurred during step
3 it brought it to a value of 2. When the second bio_endio() was
called, in step 4 above, it should've called the original bi_end_io but
it didn't because there was an extra reference that wasn't dropped (due
to atomic operations being optimized away since BIO_CHAIN wasn't set
upfront).
Fix this issue by removing the __bi_remaining management complexity for
all callers that use the above pattern -- bio_chain() is the only
interface that _needs_ to be concerned with __bi_remaining. For the
above pattern callers just expect the bi_end_io they set to get called!
Remove bio_endio_nodec() and also remove all bio_inc_remaining() calls
that aren't associated with the bio_chain() interface.
Also, the bio_inc_remaining() interface has been moved local to bio.c.
Fixes: c4cf5261 ("bio: skip atomic inc/dec of ->bi_remaining for non-chains")
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Jens Axboe <axboe@fb.com>
Diffstat (limited to 'fs/btrfs/volumes.c')
-rw-r--r-- | fs/btrfs/volumes.c | 16 |
1 files changed, 5 insertions, 11 deletions
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 8e8d1d1e28a5..dac77d42a9ab 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c | |||
@@ -5585,10 +5585,10 @@ int btrfs_rmap_block(struct btrfs_mapping_tree *map_tree, | |||
5585 | 5585 | ||
5586 | static inline void btrfs_end_bbio(struct btrfs_bio *bbio, struct bio *bio, int err) | 5586 | static inline void btrfs_end_bbio(struct btrfs_bio *bbio, struct bio *bio, int err) |
5587 | { | 5587 | { |
5588 | if (likely(bbio->flags & BTRFS_BIO_ORIG_BIO_SUBMITTED)) | 5588 | bio->bi_private = bbio->private; |
5589 | bio_endio_nodec(bio, err); | 5589 | bio->bi_end_io = bbio->end_io; |
5590 | else | 5590 | bio_endio(bio, err); |
5591 | bio_endio(bio, err); | 5591 | |
5592 | btrfs_put_bbio(bbio); | 5592 | btrfs_put_bbio(bbio); |
5593 | } | 5593 | } |
5594 | 5594 | ||
@@ -5632,8 +5632,6 @@ static void btrfs_end_bio(struct bio *bio, int err) | |||
5632 | bio = bbio->orig_bio; | 5632 | bio = bbio->orig_bio; |
5633 | } | 5633 | } |
5634 | 5634 | ||
5635 | bio->bi_private = bbio->private; | ||
5636 | bio->bi_end_io = bbio->end_io; | ||
5637 | btrfs_io_bio(bio)->mirror_num = bbio->mirror_num; | 5635 | btrfs_io_bio(bio)->mirror_num = bbio->mirror_num; |
5638 | /* only send an error to the higher layers if it is | 5636 | /* only send an error to the higher layers if it is |
5639 | * beyond the tolerance of the btrfs bio | 5637 | * beyond the tolerance of the btrfs bio |
@@ -5815,8 +5813,6 @@ static void bbio_error(struct btrfs_bio *bbio, struct bio *bio, u64 logical) | |||
5815 | /* Shoud be the original bio. */ | 5813 | /* Shoud be the original bio. */ |
5816 | WARN_ON(bio != bbio->orig_bio); | 5814 | WARN_ON(bio != bbio->orig_bio); |
5817 | 5815 | ||
5818 | bio->bi_private = bbio->private; | ||
5819 | bio->bi_end_io = bbio->end_io; | ||
5820 | btrfs_io_bio(bio)->mirror_num = bbio->mirror_num; | 5816 | btrfs_io_bio(bio)->mirror_num = bbio->mirror_num; |
5821 | bio->bi_iter.bi_sector = logical >> 9; | 5817 | bio->bi_iter.bi_sector = logical >> 9; |
5822 | 5818 | ||
@@ -5897,10 +5893,8 @@ int btrfs_map_bio(struct btrfs_root *root, int rw, struct bio *bio, | |||
5897 | if (dev_nr < total_devs - 1) { | 5893 | if (dev_nr < total_devs - 1) { |
5898 | bio = btrfs_bio_clone(first_bio, GFP_NOFS); | 5894 | bio = btrfs_bio_clone(first_bio, GFP_NOFS); |
5899 | BUG_ON(!bio); /* -ENOMEM */ | 5895 | BUG_ON(!bio); /* -ENOMEM */ |
5900 | } else { | 5896 | } else |
5901 | bio = first_bio; | 5897 | bio = first_bio; |
5902 | bbio->flags |= BTRFS_BIO_ORIG_BIO_SUBMITTED; | ||
5903 | } | ||
5904 | 5898 | ||
5905 | submit_stripe_bio(root, bbio, bio, | 5899 | submit_stripe_bio(root, bbio, bio, |
5906 | bbio->stripes[dev_nr].physical, dev_nr, rw, | 5900 | bbio->stripes[dev_nr].physical, dev_nr, rw, |