Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | Date | Wed, 13 Dec 2023 00:58:38 -0800 | From | Christoph Hellwig <> | Subject | Re: [PATCH 11/13] btrfs: open code set_io_stripe for RAID56 |
| |
On Tue, Dec 12, 2023 at 04:38:09AM -0800, Johannes Thumshirn wrote: > Open code set_io_stripe() for RAID56, as it a) uses a different method to > calculate the stripe_index and b) doesn't need to go through raid-stripe-tree > mapping code.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
I think raid stripe tree handling also really should move out of set_io_stripe. Below is the latest I have, although it probably won't apply to your tree:
--- From ac208da48d7f9d11eef8a01ac0c6fbf9681665b5 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig <hch@lst.de> Date: Thu, 22 Jun 2023 05:53:13 +0200 Subject: btrfs: move raid-stripe-tree handling out of set_io_stripe
set_io_stripe gets a little too complicated with the raid-stripe-tree handling. Move it out into the only callers that actually needs it.
The only reads with more than a single stripe is the parity raid recovery case thast will need very special handling anyway once implemented.
Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/volumes.c | 61 ++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 34 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 30ee5d1670d034..e32eefa242b0a4 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -6233,22 +6233,12 @@ static u64 btrfs_max_io_len(struct map_lookup *map, enum btrfs_map_op op, return U64_MAX; } -static int set_io_stripe(struct btrfs_fs_info *fs_info, enum btrfs_map_op op, - u64 logical, u64 *length, struct btrfs_io_stripe *dst, - struct map_lookup *map, u32 stripe_index, - u64 stripe_offset, u64 stripe_nr) +static void set_io_stripe(struct btrfs_io_stripe *dst, const struct map_lookup *map, + u32 stripe_index, u64 stripe_offset, u32 stripe_nr) { dst->dev = map->stripes[stripe_index].dev; - - if (op == BTRFS_MAP_READ && - btrfs_use_raid_stripe_tree(fs_info, map->type)) - return btrfs_get_raid_extent_offset(fs_info, logical, length, - map->type, stripe_index, - dst); - dst->physical = map->stripes[stripe_index].physical + stripe_offset + ((u64)stripe_nr << BTRFS_STRIPE_LEN_SHIFT); - return 0; } int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op, @@ -6423,15 +6413,24 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op, * physical block information on the stack instead of allocating an * I/O context structure. */ - if (smap && num_alloc_stripes == 1 && - !(btrfs_use_raid_stripe_tree(fs_info, map->type) && - op != BTRFS_MAP_READ) && - !((map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) && mirror_num > 1)) { - ret = set_io_stripe(fs_info, op, logical, length, smap, map, - stripe_index, stripe_offset, stripe_nr); - *mirror_num_ret = mirror_num; - *bioc_ret = NULL; - goto out; + if (smap && num_alloc_stripes == 1) { + if (op == BTRFS_MAP_READ && + btrfs_use_raid_stripe_tree(fs_info, map->type)) { + ret = btrfs_get_raid_extent_offset(fs_info, logical, + length, map->type, + stripe_index, smap); + *mirror_num_ret = mirror_num; + *bioc_ret = NULL; + goto out; + } else if (!(map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) || + mirror_num == 0) { + set_io_stripe(smap, map, stripe_index, stripe_offset, + stripe_nr); + *mirror_num_ret = mirror_num; + *bioc_ret = NULL; + ret = 0; + goto out; + } } bioc = alloc_btrfs_io_context(fs_info, logical, num_alloc_stripes); @@ -6448,6 +6447,8 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op, * * It's still mostly the same as other profiles, just with extra rotation. */ + ASSERT(op != BTRFS_MAP_READ || + btrfs_use_raid_stripe_tree(fs_info, map->type)); if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK && need_raid_map && (op != BTRFS_MAP_READ || mirror_num > 1)) { /* @@ -6461,29 +6462,21 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op, bioc->full_stripe_logical = em->start + ((stripe_nr * data_stripes) << BTRFS_STRIPE_LEN_SHIFT); for (i = 0; i < num_stripes; i++) - ret = set_io_stripe(fs_info, op, logical, length, - &bioc->stripes[i], map, - (i + stripe_nr) % num_stripes, - stripe_offset, stripe_nr); + set_io_stripe(&bioc->stripes[i], map, + (i + stripe_nr) % num_stripes, + stripe_offset, stripe_nr); } else { /* * For all other non-RAID56 profiles, just copy the target * stripe into the bioc. */ for (i = 0; i < num_stripes; i++) { - ret = set_io_stripe(fs_info, op, logical, length, - &bioc->stripes[i], map, stripe_index, - stripe_offset, stripe_nr); + set_io_stripe(&bioc->stripes[i], map, stripe_index, + stripe_offset, stripe_nr); stripe_index++; } } - if (ret) { - *bioc_ret = NULL; - btrfs_put_bioc(bioc); - goto out; - } - if (op != BTRFS_MAP_READ) max_errors = btrfs_chunk_max_errors(map); -- 2.39.2
| ![\](/images/icornerr.gif) |