On 2022/9/1 15:42, Christoph Hellwig wrote:
Currently the I/O submitters have to split bios according to the
chunk stripe boundaries. This leads to extra lookups in the extent
trees and a lot of boilerplate code.
To drop this requirement, split the bio when __btrfs_map_block
returns a mapping that is smaller than the requested size and
keep a count of pending bios in the original btrfs_bio so that
the upper level completion is only invoked when all clones have
completed.
Sorry for the late reply, but I still have a question related the
chained bio way.
Since we go the chained method, it means, if we hit an error for the
splitted bio, the whole bio will be marked error.
Especially for read bios, that can be a problem (currently only for
RAID10 though), which can affect the read repair behavior.
E.g. we have a 4-disks RAID10 looks like this:
Disk 1 (unreliable): Mirror 1 of logical range [X, X + 64K)
Disk 2 (reliable): Mirror 2 of logical range [X, X + 64K)
Disk 3 (reliable): Mirror 1 of logical range [X + 64K, X + 128K)
Disk 4 (unreliable): Mirror 2 of logical range [X + 64K, X + 128K)
And we submit a read for range [X, X + 128K)
The first 64K will use mirror 1, thus reading from Disk 1.
The second 64K will also use mirror 1, thus read from Disk 2.
But the first 64K read failed due to whatever reason, thus we mark the
whole range error, and needs to go repair code.
Note that, the original bio is using mirror 1, thus for read-repair we
can only read from mirror 2 to repair.
But in that case, Disk 4 is also unreliable, if at read-repair time, we
didn't try all mirrors, including the failed one (which is no longer a
reliable mirror_num for all ranges), we may failed to repair some range.
Does the read-repair code now has something to compensate the chained
behavior?
Thanks,
Qu
Based on a patch from Qu Wenruo.
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
---
fs/btrfs/volumes.c | 106 +++++++++++++++++++++++++++++++++++++--------
fs/btrfs/volumes.h | 1 +
2 files changed, 90 insertions(+), 17 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5c6535e10085d..0a2d144c20604 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -35,6 +35,7 @@
#include "zoned.h"
static struct bio_set btrfs_bioset;
+static struct bio_set btrfs_clone_bioset;
static struct bio_set btrfs_repair_bioset;
static mempool_t btrfs_failed_bio_pool;
@@ -6661,6 +6662,7 @@ static void btrfs_bio_init(struct btrfs_bio *bbio, struct inode *inode,
bbio->inode = inode;
bbio->end_io = end_io;
bbio->private = private;
+ atomic_set(&bbio->pending_ios, 1);
}
/*
@@ -6698,6 +6700,57 @@ struct bio *btrfs_bio_clone_partial(struct bio *orig, u64 offset, u64 size,
return bio;
}
+static struct bio *btrfs_split_bio(struct bio *orig, u64 map_length)
+{
+ struct btrfs_bio *orig_bbio = btrfs_bio(orig);
+ struct bio *bio;
+
+ bio = bio_split(orig, map_length >> SECTOR_SHIFT, GFP_NOFS,
+ &btrfs_clone_bioset);
+ btrfs_bio_init(btrfs_bio(bio), orig_bbio->inode, NULL, orig_bbio);
+
+ btrfs_bio(bio)->file_offset = orig_bbio->file_offset;
+ orig_bbio->file_offset += map_length;
+
+ atomic_inc(&orig_bbio->pending_ios);
+ return bio;
+}
+
+static void btrfs_orig_write_end_io(struct bio *bio);
+static void btrfs_bbio_propagate_error(struct btrfs_bio *bbio,
+ struct btrfs_bio *orig_bbio)
+{
+ /*
+ * For writes btrfs tolerates nr_mirrors - 1 write failures, so we
+ * can't just blindly propagate a write failure here.
+ * Instead increment the error count in the original I/O context so
+ * that it is guaranteed to be larger than the error tolerance.
+ */
+ if (bbio->bio.bi_end_io == &btrfs_orig_write_end_io) {
+ struct btrfs_io_stripe *orig_stripe = orig_bbio->bio.bi_private;
+ struct btrfs_io_context *orig_bioc = orig_stripe->bioc;
+
+ atomic_add(orig_bioc->max_errors, &orig_bioc->error);
+ } else {
+ orig_bbio->bio.bi_status = bbio->bio.bi_status;
+ }
+}
+
+static void btrfs_orig_bbio_end_io(struct btrfs_bio *bbio)
+{
+ if (bbio->bio.bi_pool == &btrfs_clone_bioset) {
+ struct btrfs_bio *orig_bbio = bbio->private;
+
+ if (bbio->bio.bi_status)
+ btrfs_bbio_propagate_error(bbio, orig_bbio);
+ bio_put(&bbio->bio);
+ bbio = orig_bbio;
+ }
+
+ if (atomic_dec_and_test(&bbio->pending_ios))
+ bbio->end_io(bbio);
+}
+
static int next_repair_mirror(struct btrfs_failed_bio *fbio, int cur_mirror)
{
if (cur_mirror == fbio->num_copies)
@@ -6715,7 +6768,7 @@ static int prev_repair_mirror(struct btrfs_failed_bio *fbio, int cur_mirror)
static void btrfs_repair_done(struct btrfs_failed_bio *fbio)
{
if (atomic_dec_and_test(&fbio->repair_count)) {
- fbio->bbio->end_io(fbio->bbio);
+ btrfs_orig_bbio_end_io(fbio->bbio);
mempool_free(fbio, &btrfs_failed_bio_pool);
}
}
@@ -6857,7 +6910,7 @@ static void btrfs_check_read_bio(struct btrfs_bio *bbio,
if (unlikely(fbio))
btrfs_repair_done(fbio);
else
- bbio->end_io(bbio);
+ btrfs_orig_bbio_end_io(bbio);
}
static void btrfs_log_dev_io_error(struct bio *bio, struct btrfs_device *dev)
@@ -6908,7 +6961,7 @@ static void btrfs_simple_end_io(struct bio *bio)
} else {
if (bio_op(bio) == REQ_OP_ZONE_APPEND)
btrfs_record_physical_zoned(bbio);
- bbio->end_io(bbio);
+ btrfs_orig_bbio_end_io(bbio);
}
}
@@ -6922,7 +6975,7 @@ static void btrfs_raid56_end_io(struct bio *bio)
if (bio_op(bio) == REQ_OP_READ)
btrfs_check_read_bio(bbio, NULL);
else
- bbio->end_io(bbio);
+ btrfs_orig_bbio_end_io(bbio);
btrfs_put_bioc(bioc);
}
@@ -6949,7 +7002,7 @@ static void btrfs_orig_write_end_io(struct bio *bio)
else
bio->bi_status = BLK_STS_OK;
- bbio->end_io(bbio);
+ btrfs_orig_bbio_end_io(bbio);
btrfs_put_bioc(bioc);
}
@@ -7190,8 +7243,8 @@ static bool btrfs_wq_submit_bio(struct btrfs_bio *bbio,
return true;
}
-void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
- int mirror_num)
+static bool btrfs_submit_chunk(struct btrfs_fs_info *fs_info, struct bio *bio,
+ int mirror_num)
{
struct btrfs_bio *bbio = btrfs_bio(bio);
u64 logical = bio->bi_iter.bi_sector << 9;
@@ -7207,11 +7260,10 @@ void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
if (ret)
goto fail;
+ map_length = min(map_length, length);
if (map_length < length) {
- btrfs_crit(fs_info,
- "mapping failed logical %llu bio len %llu len %llu",
- logical, length, map_length);
- BUG();
+ bio = btrfs_split_bio(bio, map_length);
+ bbio = btrfs_bio(bio);
}
/*
@@ -7222,7 +7274,7 @@ void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
bbio->saved_iter = bio->bi_iter;
ret = btrfs_lookup_bio_sums(bbio);
if (ret)
- goto fail;
+ goto fail_put_bio;
}
if (btrfs_op(bio) == BTRFS_MAP_WRITE) {
@@ -7231,7 +7283,7 @@ void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
if (bio_op(bio) == REQ_OP_ZONE_APPEND) {
ret = btrfs_extract_ordered_extent(btrfs_bio(bio));
if (ret)
- goto fail;
+ goto fail_put_bio;
}
/*
@@ -7243,22 +7295,36 @@ void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
!btrfs_is_data_reloc_root(bi->root)) {
if (should_async_write(bbio) &&
btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
- return;
+ goto done;
if (bio->bi_opf & REQ_META)
ret = btree_csum_one_bio(bbio);
else
ret = btrfs_csum_one_bio(bbio);
if (ret)
- goto fail;
+ goto fail_put_bio;
}
}
__btrfs_submit_bio(bio, bioc, &smap, mirror_num);
- return;
+done:
+ return map_length == length;
+
+fail_put_bio:
+ if (map_length < length)
+ bio_put(bio);
fail:
btrfs_bio_counter_dec(fs_info);
btrfs_bio_end_io(bbio, errno_to_blk_status(ret));
+ /* Do not submit another chunk */
+ return true;
+}
+
+void btrfs_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
+ int mirror_num)
+{
+ while (!btrfs_submit_chunk(fs_info, bio, mirror_num))
+ ;
}
/*
@@ -8858,10 +8924,13 @@ int __init btrfs_bioset_init(void)
offsetof(struct btrfs_bio, bio),
BIOSET_NEED_BVECS))
return -ENOMEM;
+ if (bioset_init(&btrfs_clone_bioset, BIO_POOL_SIZE,
+ offsetof(struct btrfs_bio, bio), 0))
+ goto out_free_bioset;
if (bioset_init(&btrfs_repair_bioset, BIO_POOL_SIZE,
offsetof(struct btrfs_bio, bio),
BIOSET_NEED_BVECS))
- goto out_free_bioset;
+ goto out_free_clone_bioset;
if (mempool_init_kmalloc_pool(&btrfs_failed_bio_pool, BIO_POOL_SIZE,
sizeof(struct btrfs_failed_bio)))
goto out_free_repair_bioset;
@@ -8869,6 +8938,8 @@ int __init btrfs_bioset_init(void)
out_free_repair_bioset:
bioset_exit(&btrfs_repair_bioset);
+out_free_clone_bioset:
+ bioset_exit(&btrfs_clone_bioset);
out_free_bioset:
bioset_exit(&btrfs_bioset);
return -ENOMEM;
@@ -8878,5 +8949,6 @@ void __cold btrfs_bioset_exit(void)
{
mempool_exit(&btrfs_failed_bio_pool);
bioset_exit(&btrfs_repair_bioset);
+ bioset_exit(&btrfs_clone_bioset);
bioset_exit(&btrfs_bioset);
}
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 8b248c9bd602b..97877184d0db1 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -386,6 +386,7 @@ struct btrfs_bio {
/* For internal use in read end I/O handling */
unsigned int mirror_num;
+ atomic_t pending_ios;
struct work_struct end_io_work;
/*
