2023-12-13 14:43:24

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 00/13] btrfs: clean up RAID I/O geometry calculation

The calculation of the RAID I/O geometry in btrfs_map_block has been a maze of
if-else statements for a very long time and the advent of the
raid-stripe-tree made the situation even worse.

This patchset refactors btrfs_map_block() to untagle the maze and make I/O
geometry setting easier to follow, but does not introduce any functional
changes.

I've also run it through Josef's CI and there have been test failures, but
none of them introduced by these patches.

---
Changes in v2:
- add btrfs_map_op into struct btrfs_io_geometry
- split RAID56 read and write into two different helpers
- drop redundand 'for' in helper function names
- kept dev_replace_is_ongoing variable name
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Johannes Thumshirn (13):
btrfs: factor out helper for single device IO check
btrfs: re-introduce struct btrfs_io_geometry
btrfs: factor out block-mapping for RAID0
btrfs: factor out RAID1 block mapping
btrfs: factor out block mapping for DUP profiles
btrfs: factor out block mapping for RAID10
btrfs: reduce scope of data_stripes in btrfs_map_block
btrfs: factor out block mapping for RAID5/6
btrfs: factor out block mapping for single profiles
btrfs: btrfs: untagle if else maze in btrfs_map_block
btrfs: open code set_io_stripe for RAID56
btrfs: pass struct btrfs_io_geometry to set_io_stripe
btrfs: pass btrfs_io_geometry into btrfs_max_io_len

fs/btrfs/volumes.c | 410 +++++++++++++++++++++++++++++++++--------------------
1 file changed, 259 insertions(+), 151 deletions(-)
---
base-commit: 14d1d39586246ca9d4ce97049c98be849e3bbcd9
change-id: 20231207-btrfs_map_block-cleanup-346f53aff90d

Best regards,
--
Johannes Thumshirn <[email protected]>


2023-12-13 14:43:26

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 01/13] btrfs: factor out helper for single device IO check

The check in btrfs_map_block() deciding if a particular I/O is targeting a
single device is getting more and more convoluted.

Factor out the check conditions into a helper function, with no functional
change otherwise.

Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 1cc6b5d5eb61..1011178a244c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6330,6 +6330,28 @@ static int set_io_stripe(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
return 0;
}

+static bool is_single_device_io(struct btrfs_fs_info *fs_info,
+ struct btrfs_io_stripe *smap,
+ struct btrfs_chunk_map *map,
+ int num_alloc_stripes,
+ enum btrfs_map_op op, int mirror_num)
+{
+ if (!smap)
+ return false;
+
+ if (num_alloc_stripes != 1)
+ return false;
+
+ if (btrfs_need_stripe_tree_update(fs_info, map->type) &&
+ op != BTRFS_MAP_READ)
+ return false;
+
+ if ((map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) && mirror_num > 1)
+ return false;
+
+ return true;
+}
+
/*
* Map one logical range to one or more physical ranges.
*
@@ -6532,10 +6554,8 @@ 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_need_stripe_tree_update(fs_info, map->type) &&
- op != BTRFS_MAP_READ) &&
- !((map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) && mirror_num > 1)) {
+ if (is_single_device_io(fs_info, smap, map, num_alloc_stripes, op,
+ mirror_num)) {
ret = set_io_stripe(fs_info, op, logical, length, smap, map,
stripe_index, stripe_offset, stripe_nr);
if (mirror_num_ret)

--
2.43.0

2023-12-13 14:43:29

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 03/13] btrfs: factor out block-mapping for RAID0

Now that we have a container for the I/O geometry that has all the needed
information for the block mappings of RAID0, factor out a helper calculating
this information.

Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index ea830ff0c0e3..1d2e6fb7b9bf 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6363,6 +6363,15 @@ static bool is_single_device_io(struct btrfs_fs_info *fs_info,
return true;
}

+static void map_blocks_raid0(struct btrfs_chunk_map *map,
+ struct btrfs_io_geometry *io_geom)
+{
+ io_geom->stripe_index = io_geom->stripe_nr % map->num_stripes;
+ io_geom->stripe_nr /= map->num_stripes;
+ if (io_geom->op == BTRFS_MAP_READ)
+ io_geom->mirror_num = 1;
+}
+
/*
* Map one logical range to one or more physical ranges.
*
@@ -6450,10 +6459,7 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
up_read(&dev_replace->rwsem);

if (map->type & BTRFS_BLOCK_GROUP_RAID0) {
- io_geom.stripe_index = io_geom.stripe_nr % map->num_stripes;
- io_geom.stripe_nr /= map->num_stripes;
- if (op == BTRFS_MAP_READ)
- io_geom.mirror_num = 1;
+ map_blocks_raid0(map, &io_geom);
} else if (map->type & BTRFS_BLOCK_GROUP_RAID1_MASK) {
if (op != BTRFS_MAP_READ) {
io_geom.num_stripes = map->num_stripes;

--
2.43.0

2023-12-13 14:43:53

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 04/13] btrfs: factor out RAID1 block mapping

Now that we have a container for the I/O geometry that has all the needed
information for the block mappings of RAID1, factor out a helper calculating
this information.

Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 1d2e6fb7b9bf..c11fb6db4679 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6372,6 +6372,26 @@ static void map_blocks_raid0(struct btrfs_chunk_map *map,
io_geom->mirror_num = 1;
}

+static void map_blocks_raid1(struct btrfs_fs_info *fs_info,
+ struct btrfs_chunk_map *map,
+ struct btrfs_io_geometry *io_geom,
+ bool dev_replace_is_ongoing)
+{
+ if (io_geom->op != BTRFS_MAP_READ) {
+ io_geom->num_stripes = map->num_stripes;
+ return;
+ }
+
+ if (io_geom->mirror_num) {
+ io_geom->stripe_index = io_geom->mirror_num - 1;
+ return;
+ }
+
+ io_geom->stripe_index = find_live_mirror(fs_info, map, 0,
+ dev_replace_is_ongoing);
+ io_geom->mirror_num = io_geom->stripe_index + 1;
+}
+
/*
* Map one logical range to one or more physical ranges.
*
@@ -6461,16 +6481,8 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
if (map->type & BTRFS_BLOCK_GROUP_RAID0) {
map_blocks_raid0(map, &io_geom);
} else if (map->type & BTRFS_BLOCK_GROUP_RAID1_MASK) {
- if (op != BTRFS_MAP_READ) {
- io_geom.num_stripes = map->num_stripes;
- } else if (io_geom.mirror_num) {
- io_geom.stripe_index = io_geom.mirror_num - 1;
- } else {
- io_geom.stripe_index = find_live_mirror(fs_info, map, 0,
- dev_replace_is_ongoing);
- io_geom.mirror_num = io_geom.stripe_index + 1;
- }
-
+ map_blocks_raid1(fs_info, map, &io_geom,
+ dev_replace_is_ongoing);
} else if (map->type & BTRFS_BLOCK_GROUP_DUP) {
if (op != BTRFS_MAP_READ) {
io_geom.num_stripes = map->num_stripes;

--
2.43.0

2023-12-13 14:43:58

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 09/13] btrfs: factor out block mapping for single profiles

Now that we have a container for the I/O geometry that has all the needed
information for the block mappings of SINGLE profiles, factor out a helper
calculating this information.

Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 55614a9eb8a5..e23c7d2842a6 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6487,6 +6487,14 @@ static void map_blocks_raid56_read(struct btrfs_chunk_map *map,
io_geom->mirror_num = 1;
}

+static void map_blocks_single(struct btrfs_chunk_map *map,
+ struct btrfs_io_geometry *io_geom)
+{
+ io_geom->stripe_index = io_geom->stripe_nr % map->num_stripes;
+ io_geom->stripe_nr /= map->num_stripes;
+ io_geom->mirror_num = io_geom->stripe_index + 1;
+}
+
/*
* Map one logical range to one or more physical ranges.
*
@@ -6591,9 +6599,7 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
* device we have to walk to find the data, and stripe_index is
* the number of our device in the stripe array
*/
- io_geom.stripe_index = io_geom.stripe_nr % map->num_stripes;
- io_geom.stripe_nr /= map->num_stripes;
- io_geom.mirror_num = io_geom.stripe_index + 1;
+ map_blocks_single(map, &io_geom);
}
if (io_geom.stripe_index >= map->num_stripes) {
btrfs_crit(fs_info,

--
2.43.0

2023-12-13 14:43:58

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 06/13] btrfs: factor out block mapping for RAID10

Now that we have a container for the I/O geometry that has all the needed
information for the block mappings of RAID10, factor out a helper calculating
this information.

Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 48 +++++++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index fe2807bb0935..4c9c130cdfd0 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6408,6 +6408,35 @@ static void map_blocks_dup(struct btrfs_chunk_map *map,
io_geom->mirror_num = 1;
}

+static void map_blocks_raid10(struct btrfs_fs_info *fs_info,
+ struct btrfs_chunk_map *map,
+ struct btrfs_io_geometry *io_geom,
+ bool dev_replace_is_ongoing)
+{
+ u32 factor = map->num_stripes / map->sub_stripes;
+ int old_stripe_index;
+
+ io_geom->stripe_index =
+ (io_geom->stripe_nr % factor) * map->sub_stripes;
+ io_geom->stripe_nr /= factor;
+
+ if (io_geom->op != BTRFS_MAP_READ) {
+ io_geom->num_stripes = map->sub_stripes;
+ return;
+ }
+
+ if (io_geom->mirror_num) {
+ io_geom->stripe_index += io_geom->mirror_num - 1;
+ return;
+ }
+
+ old_stripe_index = io_geom->stripe_index;
+ io_geom->stripe_index = find_live_mirror(fs_info, map,
+ io_geom->stripe_index,
+ dev_replace_is_ongoing);
+ io_geom->mirror_num = io_geom->stripe_index - old_stripe_index + 1;
+}
+
/*
* Map one logical range to one or more physical ranges.
*
@@ -6502,23 +6531,8 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
} else if (map->type & BTRFS_BLOCK_GROUP_DUP) {
map_blocks_dup(map, &io_geom);
} else if (map->type & BTRFS_BLOCK_GROUP_RAID10) {
- u32 factor = map->num_stripes / map->sub_stripes;
-
- io_geom.stripe_index = (io_geom.stripe_nr % factor) * map->sub_stripes;
- io_geom.stripe_nr /= factor;
-
- if (op != BTRFS_MAP_READ)
- io_geom.num_stripes = map->sub_stripes;
- else if (io_geom.mirror_num)
- io_geom.stripe_index += io_geom.mirror_num - 1;
- else {
- int old_stripe_index = io_geom.stripe_index;
- io_geom.stripe_index = find_live_mirror(fs_info, map,
- io_geom.stripe_index,
- dev_replace_is_ongoing);
- io_geom.mirror_num = io_geom.stripe_index - old_stripe_index + 1;
- }
-
+ map_blocks_raid10(fs_info, map, &io_geom,
+ dev_replace_is_ongoing);
} else if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
if (op != BTRFS_MAP_READ || io_geom.mirror_num > 1) {
/*

--
2.43.0

2023-12-13 14:44:01

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 05/13] btrfs: factor out block mapping for DUP profiles

Now that we have a container for the I/O geometry that has all the needed
information for the block mappings of DUP, factor out a helper calculating
this information.

Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index c11fb6db4679..fe2807bb0935 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6392,6 +6392,22 @@ static void map_blocks_raid1(struct btrfs_fs_info *fs_info,
io_geom->mirror_num = io_geom->stripe_index + 1;
}

+static void map_blocks_dup(struct btrfs_chunk_map *map,
+ struct btrfs_io_geometry *io_geom)
+{
+ if (io_geom->op != BTRFS_MAP_READ) {
+ io_geom->num_stripes = map->num_stripes;
+ return;
+ }
+
+ if (io_geom->mirror_num) {
+ io_geom->stripe_index = io_geom->mirror_num - 1;
+ return;
+ }
+
+ io_geom->mirror_num = 1;
+}
+
/*
* Map one logical range to one or more physical ranges.
*
@@ -6484,14 +6500,7 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
map_blocks_raid1(fs_info, map, &io_geom,
dev_replace_is_ongoing);
} else if (map->type & BTRFS_BLOCK_GROUP_DUP) {
- if (op != BTRFS_MAP_READ) {
- io_geom.num_stripes = map->num_stripes;
- } else if (io_geom.mirror_num) {
- io_geom.stripe_index = io_geom.mirror_num - 1;
- } else {
- io_geom.mirror_num = 1;
- }
-
+ map_blocks_dup(map, &io_geom);
} else if (map->type & BTRFS_BLOCK_GROUP_RAID10) {
u32 factor = map->num_stripes / map->sub_stripes;


--
2.43.0

2023-12-13 14:44:03

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 08/13] btrfs: factor out block mapping for RAID5/6

Now that we have a container for the I/O geometry that has all the needed
information for the block mappings of RAID5 and RAID6, factor out a helper
calculating this information.

Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 96 ++++++++++++++++++++++++++++++------------------------
1 file changed, 54 insertions(+), 42 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 3a6a2f71d364..55614a9eb8a5 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6437,6 +6437,56 @@ static void map_blocks_raid10(struct btrfs_fs_info *fs_info,
io_geom->mirror_num = io_geom->stripe_index - old_stripe_index + 1;
}

+static void map_blocks_raid56_write(struct btrfs_chunk_map *map,
+ struct btrfs_io_geometry *io_geom,
+ u64 logical, u64 *length)
+{
+ int data_stripes = nr_data_stripes(map);
+
+ /*
+ * Needs full stripe mapping.
+ *
+ * Push stripe_nr back to the start of the full stripe
+ * For those cases needing a full stripe, @stripe_nr
+ * is the full stripe number.
+ *
+ * Originally we go raid56_full_stripe_start / full_stripe_len,
+ * but that can be expensive. Here we just divide
+ * @stripe_nr with @data_stripes.
+ */
+ io_geom->stripe_nr /= data_stripes;
+
+ /* RAID[56] write or recovery. Return all stripes */
+ io_geom->num_stripes = map->num_stripes;
+ io_geom->max_errors = btrfs_chunk_max_errors(map);
+
+ /* Return the length to the full stripe end */
+ *length = min(logical + *length,
+ io_geom->raid56_full_stripe_start + map->start +
+ btrfs_stripe_nr_to_offset(data_stripes)) -
+ logical;
+ io_geom->stripe_index = 0;
+ io_geom->stripe_offset = 0;
+}
+
+static void map_blocks_raid56_read(struct btrfs_chunk_map *map,
+ struct btrfs_io_geometry *io_geom)
+{
+ int data_stripes = nr_data_stripes(map);
+
+ ASSERT(io_geom->mirror_num <= 1);
+ /* Just grab the data stripe directly. */
+ io_geom->stripe_index = io_geom->stripe_nr % data_stripes;
+ io_geom->stripe_nr /= data_stripes;
+
+ /* We distribute the parity blocks across stripes */
+ io_geom->stripe_index =
+ (io_geom->stripe_nr + io_geom->stripe_index) % map->num_stripes;
+
+ if (io_geom->op == BTRFS_MAP_READ && io_geom->mirror_num < 1)
+ io_geom->mirror_num = 1;
+}
+
/*
* Map one logical range to one or more physical ranges.
*
@@ -6531,48 +6581,10 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
map_blocks_raid10(fs_info, map, &io_geom,
dev_replace_is_ongoing);
} else if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
- int data_stripes = nr_data_stripes(map);
-
- if (op != BTRFS_MAP_READ || io_geom.mirror_num > 1) {
- /*
- * Needs full stripe mapping.
- *
- * Push stripe_nr back to the start of the full stripe
- * For those cases needing a full stripe, @stripe_nr
- * is the full stripe number.
- *
- * Originally we go raid56_full_stripe_start / full_stripe_len,
- * but that can be expensive. Here we just divide
- * @stripe_nr with @data_stripes.
- */
- io_geom.stripe_nr /= data_stripes;
-
- /* RAID[56] write or recovery. Return all stripes */
- io_geom.num_stripes = map->num_stripes;
- io_geom.max_errors = btrfs_chunk_max_errors(map);
-
- /* Return the length to the full stripe end */
- *length = min(logical + *length,
- io_geom.raid56_full_stripe_start +
- map->start +
- btrfs_stripe_nr_to_offset(
- data_stripes)) -
- logical;
- io_geom.stripe_index = 0;
- io_geom.stripe_offset = 0;
- } else {
- ASSERT(io_geom.mirror_num <= 1);
- /* Just grab the data stripe directly. */
- io_geom.stripe_index = io_geom.stripe_nr % data_stripes;
- io_geom.stripe_nr /= data_stripes;
-
- /* We distribute the parity blocks across stripes */
- io_geom.stripe_index =
- (io_geom.stripe_nr + io_geom.stripe_index) %
- map->num_stripes;
- if (op == BTRFS_MAP_READ && io_geom.mirror_num < 1)
- io_geom.mirror_num = 1;
- }
+ if (op != BTRFS_MAP_READ || io_geom.mirror_num > 1)
+ map_blocks_raid56_write(map, &io_geom, logical, length);
+ else
+ map_blocks_raid56_read(map, &io_geom);
} else {
/*
* After this, stripe_nr is the number of stripes on this

--
2.43.0

2023-12-13 14:44:08

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 02/13] btrfs: re-introduce struct btrfs_io_geometry

Re-introduce struct btrfs_io_geometry, holding the necessary bits and
pieces needed in btrfs_map_block() to decide the I/O geometry of a specific
block mapping.

Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 159 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 89 insertions(+), 70 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 1011178a244c..ea830ff0c0e3 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -41,6 +41,17 @@
BTRFS_BLOCK_GROUP_RAID10 | \
BTRFS_BLOCK_GROUP_RAID56_MASK)

+struct btrfs_io_geometry {
+ u32 stripe_index;
+ u32 stripe_nr;
+ int mirror_num;
+ int num_stripes;
+ u64 stripe_offset;
+ u64 raid56_full_stripe_start;
+ int max_errors;
+ enum btrfs_map_op op;
+};
+
const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
[BTRFS_RAID_RAID10] = {
.sub_stripes = 2,
@@ -6393,28 +6404,27 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
struct btrfs_io_stripe *smap, int *mirror_num_ret)
{
struct btrfs_chunk_map *map;
+ struct btrfs_io_geometry io_geom = { };
u64 map_offset;
- u64 stripe_offset;
- u32 stripe_nr;
- u32 stripe_index;
int data_stripes;
int i;
int ret = 0;
- int mirror_num = (mirror_num_ret ? *mirror_num_ret : 0);
- int num_stripes;
int num_copies;
- int max_errors = 0;
struct btrfs_io_context *bioc = NULL;
struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace;
int dev_replace_is_ongoing = 0;
u16 num_alloc_stripes;
- u64 raid56_full_stripe_start = (u64)-1;
u64 max_len;

ASSERT(bioc_ret);

+ io_geom.mirror_num = (mirror_num_ret ? *mirror_num_ret : 0);
+ io_geom.num_stripes = 1;
+ io_geom.stripe_index = 0;
+ io_geom.op = op;
+
num_copies = btrfs_num_copies(fs_info, logical, fs_info->sectorsize);
- if (mirror_num > num_copies)
+ if (io_geom.mirror_num > num_copies)
return -EINVAL;

map = btrfs_get_chunk_map(fs_info, logical, *length);
@@ -6424,8 +6434,10 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
data_stripes = nr_data_stripes(map);

map_offset = logical - map->start;
- max_len = btrfs_max_io_len(map, op, map_offset, &stripe_nr,
- &stripe_offset, &raid56_full_stripe_start);
+ io_geom.raid56_full_stripe_start = (u64)-1;
+ max_len = btrfs_max_io_len(map, io_geom.op, map_offset, &io_geom.stripe_nr,
+ &io_geom.stripe_offset,
+ &io_geom.raid56_full_stripe_start);
*length = min_t(u64, map->chunk_len - map_offset, max_len);

down_read(&dev_replace->rwsem);
@@ -6437,53 +6449,51 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
if (!dev_replace_is_ongoing)
up_read(&dev_replace->rwsem);

- num_stripes = 1;
- stripe_index = 0;
if (map->type & BTRFS_BLOCK_GROUP_RAID0) {
- stripe_index = stripe_nr % map->num_stripes;
- stripe_nr /= map->num_stripes;
+ io_geom.stripe_index = io_geom.stripe_nr % map->num_stripes;
+ io_geom.stripe_nr /= map->num_stripes;
if (op == BTRFS_MAP_READ)
- mirror_num = 1;
+ io_geom.mirror_num = 1;
} else if (map->type & BTRFS_BLOCK_GROUP_RAID1_MASK) {
if (op != BTRFS_MAP_READ) {
- num_stripes = map->num_stripes;
- } else if (mirror_num) {
- stripe_index = mirror_num - 1;
+ io_geom.num_stripes = map->num_stripes;
+ } else if (io_geom.mirror_num) {
+ io_geom.stripe_index = io_geom.mirror_num - 1;
} else {
- stripe_index = find_live_mirror(fs_info, map, 0,
+ io_geom.stripe_index = find_live_mirror(fs_info, map, 0,
dev_replace_is_ongoing);
- mirror_num = stripe_index + 1;
+ io_geom.mirror_num = io_geom.stripe_index + 1;
}

} else if (map->type & BTRFS_BLOCK_GROUP_DUP) {
if (op != BTRFS_MAP_READ) {
- num_stripes = map->num_stripes;
- } else if (mirror_num) {
- stripe_index = mirror_num - 1;
+ io_geom.num_stripes = map->num_stripes;
+ } else if (io_geom.mirror_num) {
+ io_geom.stripe_index = io_geom.mirror_num - 1;
} else {
- mirror_num = 1;
+ io_geom.mirror_num = 1;
}

} else if (map->type & BTRFS_BLOCK_GROUP_RAID10) {
u32 factor = map->num_stripes / map->sub_stripes;

- stripe_index = (stripe_nr % factor) * map->sub_stripes;
- stripe_nr /= factor;
+ io_geom.stripe_index = (io_geom.stripe_nr % factor) * map->sub_stripes;
+ io_geom.stripe_nr /= factor;

if (op != BTRFS_MAP_READ)
- num_stripes = map->sub_stripes;
- else if (mirror_num)
- stripe_index += mirror_num - 1;
+ io_geom.num_stripes = map->sub_stripes;
+ else if (io_geom.mirror_num)
+ io_geom.stripe_index += io_geom.mirror_num - 1;
else {
- int old_stripe_index = stripe_index;
- stripe_index = find_live_mirror(fs_info, map,
- stripe_index,
+ int old_stripe_index = io_geom.stripe_index;
+ io_geom.stripe_index = find_live_mirror(fs_info, map,
+ io_geom.stripe_index,
dev_replace_is_ongoing);
- mirror_num = stripe_index - old_stripe_index + 1;
+ io_geom.mirror_num = io_geom.stripe_index - old_stripe_index + 1;
}

} else if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
- if (op != BTRFS_MAP_READ || mirror_num > 1) {
+ if (op != BTRFS_MAP_READ || io_geom.mirror_num > 1) {
/*
* Needs full stripe mapping.
*
@@ -6495,29 +6505,33 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
* but that can be expensive. Here we just divide
* @stripe_nr with @data_stripes.
*/
- stripe_nr /= data_stripes;
+ io_geom.stripe_nr /= data_stripes;

/* RAID[56] write or recovery. Return all stripes */
- num_stripes = map->num_stripes;
- max_errors = btrfs_chunk_max_errors(map);
+ io_geom.num_stripes = map->num_stripes;
+ io_geom.max_errors = btrfs_chunk_max_errors(map);

/* Return the length to the full stripe end */
*length = min(logical + *length,
- raid56_full_stripe_start + map->start +
- btrfs_stripe_nr_to_offset(data_stripes)) -
+ io_geom.raid56_full_stripe_start +
+ map->start +
+ btrfs_stripe_nr_to_offset(
+ data_stripes)) -
logical;
- stripe_index = 0;
- stripe_offset = 0;
+ io_geom.stripe_index = 0;
+ io_geom.stripe_offset = 0;
} else {
- ASSERT(mirror_num <= 1);
+ ASSERT(io_geom.mirror_num <= 1);
/* Just grab the data stripe directly. */
- stripe_index = stripe_nr % data_stripes;
- stripe_nr /= data_stripes;
+ io_geom.stripe_index = io_geom.stripe_nr % data_stripes;
+ io_geom.stripe_nr /= data_stripes;

/* We distribute the parity blocks across stripes */
- stripe_index = (stripe_nr + stripe_index) % map->num_stripes;
- if (op == BTRFS_MAP_READ && mirror_num < 1)
- mirror_num = 1;
+ io_geom.stripe_index =
+ (io_geom.stripe_nr + io_geom.stripe_index) %
+ map->num_stripes;
+ if (op == BTRFS_MAP_READ && io_geom.mirror_num < 1)
+ io_geom.mirror_num = 1;
}
} else {
/*
@@ -6525,19 +6539,19 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
* device we have to walk to find the data, and stripe_index is
* the number of our device in the stripe array
*/
- stripe_index = stripe_nr % map->num_stripes;
- stripe_nr /= map->num_stripes;
- mirror_num = stripe_index + 1;
+ io_geom.stripe_index = io_geom.stripe_nr % map->num_stripes;
+ io_geom.stripe_nr /= map->num_stripes;
+ io_geom.mirror_num = io_geom.stripe_index + 1;
}
- if (stripe_index >= map->num_stripes) {
+ if (io_geom.stripe_index >= map->num_stripes) {
btrfs_crit(fs_info,
"stripe index math went horribly wrong, got stripe_index=%u, num_stripes=%u",
- stripe_index, map->num_stripes);
+ io_geom.stripe_index, map->num_stripes);
ret = -EINVAL;
goto out;
}

- num_alloc_stripes = num_stripes;
+ num_alloc_stripes = io_geom.num_stripes;
if (dev_replace_is_ongoing && dev_replace->tgtdev != NULL &&
op != BTRFS_MAP_READ)
/*
@@ -6555,11 +6569,12 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
* I/O context structure.
*/
if (is_single_device_io(fs_info, smap, map, num_alloc_stripes, op,
- mirror_num)) {
+ io_geom.mirror_num)) {
ret = set_io_stripe(fs_info, op, logical, length, smap, map,
- stripe_index, stripe_offset, stripe_nr);
+ io_geom.stripe_index, io_geom.stripe_offset,
+ io_geom.stripe_nr);
if (mirror_num_ret)
- *mirror_num_ret = mirror_num;
+ *mirror_num_ret = io_geom.mirror_num;
*bioc_ret = NULL;
goto out;
}
@@ -6579,7 +6594,7 @@ 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.
*/
if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK &&
- (op != BTRFS_MAP_READ || mirror_num > 1)) {
+ (op != BTRFS_MAP_READ || io_geom.mirror_num > 1)) {
/*
* For RAID56 @stripe_nr is already the number of full stripes
* before us, which is also the rotation value (needs to modulo
@@ -6589,12 +6604,13 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
* modulo, to reduce one modulo call.
*/
bioc->full_stripe_logical = map->start +
- btrfs_stripe_nr_to_offset(stripe_nr * data_stripes);
- for (int i = 0; i < num_stripes; i++) {
+ btrfs_stripe_nr_to_offset(io_geom.stripe_nr * data_stripes);
+ for (int i = 0; i < io_geom.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);
+ (i + io_geom.stripe_nr) % io_geom.num_stripes,
+ io_geom.stripe_offset,
+ io_geom.stripe_nr);
if (ret < 0)
break;
}
@@ -6603,13 +6619,15 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
* For all other non-RAID56 profiles, just copy the target
* stripe into the bioc.
*/
- for (i = 0; i < num_stripes; i++) {
+ for (i = 0; i < io_geom.num_stripes; i++) {
ret = set_io_stripe(fs_info, op, logical, length,
- &bioc->stripes[i], map, stripe_index,
- stripe_offset, stripe_nr);
+ &bioc->stripes[i], map,
+ io_geom.stripe_index,
+ io_geom.stripe_offset,
+ io_geom.stripe_nr);
if (ret < 0)
break;
- stripe_index++;
+ io_geom.stripe_index++;
}
}

@@ -6620,18 +6638,19 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
}

if (op != BTRFS_MAP_READ)
- max_errors = btrfs_chunk_max_errors(map);
+ io_geom.max_errors = btrfs_chunk_max_errors(map);

if (dev_replace_is_ongoing && dev_replace->tgtdev != NULL &&
op != BTRFS_MAP_READ) {
handle_ops_on_dev_replace(op, bioc, dev_replace, logical,
- &num_stripes, &max_errors);
+ &io_geom.num_stripes,
+ &io_geom.max_errors);
}

*bioc_ret = bioc;
- bioc->num_stripes = num_stripes;
- bioc->max_errors = max_errors;
- bioc->mirror_num = mirror_num;
+ bioc->num_stripes = io_geom.num_stripes;
+ bioc->max_errors = io_geom.max_errors;
+ bioc->mirror_num = io_geom.mirror_num;

out:
if (dev_replace_is_ongoing) {

--
2.43.0

2023-12-13 14:44:12

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 07/13] btrfs: reduce scope of data_stripes in btrfs_map_block

Reduce the scope of 'data_stripes' in btrfs_map_block(). While the change
allone doesn't make too much sense, it helps us factoring out a helper
function for the block mapping of RAID56 I/O.

Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 4c9c130cdfd0..3a6a2f71d364 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6480,7 +6480,6 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
struct btrfs_chunk_map *map;
struct btrfs_io_geometry io_geom = { };
u64 map_offset;
- int data_stripes;
int i;
int ret = 0;
int num_copies;
@@ -6505,8 +6504,6 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
if (IS_ERR(map))
return PTR_ERR(map);

- data_stripes = nr_data_stripes(map);
-
map_offset = logical - map->start;
io_geom.raid56_full_stripe_start = (u64)-1;
max_len = btrfs_max_io_len(map, io_geom.op, map_offset, &io_geom.stripe_nr,
@@ -6534,6 +6531,8 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
map_blocks_raid10(fs_info, map, &io_geom,
dev_replace_is_ongoing);
} else if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
+ int data_stripes = nr_data_stripes(map);
+
if (op != BTRFS_MAP_READ || io_geom.mirror_num > 1) {
/*
* Needs full stripe mapping.
@@ -6645,7 +6644,8 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
* modulo, to reduce one modulo call.
*/
bioc->full_stripe_logical = map->start +
- btrfs_stripe_nr_to_offset(io_geom.stripe_nr * data_stripes);
+ btrfs_stripe_nr_to_offset(io_geom.stripe_nr *
+ nr_data_stripes(map));
for (int i = 0; i < io_geom.num_stripes; i++) {
ret = set_io_stripe(fs_info, op, logical, length,
&bioc->stripes[i], map,

--
2.43.0

2023-12-13 14:44:16

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 10/13] btrfs: btrfs: untagle if else maze in btrfs_map_block

Untangle the if-else maze in btrfs_map_block into a switch statement,
checking the different block-group profile types and call out into the
per-profile block mapping helpers.

Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index e23c7d2842a6..efb31c3005b7 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6578,28 +6578,38 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
if (!dev_replace_is_ongoing)
up_read(&dev_replace->rwsem);

- if (map->type & BTRFS_BLOCK_GROUP_RAID0) {
+ switch (map->type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
+ case BTRFS_BLOCK_GROUP_RAID0:
map_blocks_raid0(map, &io_geom);
- } else if (map->type & BTRFS_BLOCK_GROUP_RAID1_MASK) {
+ break;
+ case BTRFS_BLOCK_GROUP_RAID1:
+ case BTRFS_BLOCK_GROUP_RAID1C3:
+ case BTRFS_BLOCK_GROUP_RAID1C4:
map_blocks_raid1(fs_info, map, &io_geom,
dev_replace_is_ongoing);
- } else if (map->type & BTRFS_BLOCK_GROUP_DUP) {
+ break;
+ case BTRFS_BLOCK_GROUP_DUP:
map_blocks_dup(map, &io_geom);
- } else if (map->type & BTRFS_BLOCK_GROUP_RAID10) {
+ break;
+ case BTRFS_BLOCK_GROUP_RAID10:
map_blocks_raid10(fs_info, map, &io_geom,
dev_replace_is_ongoing);
- } else if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
+ break;
+ case BTRFS_BLOCK_GROUP_RAID5:
+ case BTRFS_BLOCK_GROUP_RAID6:
if (op != BTRFS_MAP_READ || io_geom.mirror_num > 1)
map_blocks_raid56_write(map, &io_geom, logical, length);
else
map_blocks_raid56_read(map, &io_geom);
- } else {
+ break;
+ default:
/*
* After this, stripe_nr is the number of stripes on this
* device we have to walk to find the data, and stripe_index is
* the number of our device in the stripe array
*/
map_blocks_single(map, &io_geom);
+ break;
}
if (io_geom.stripe_index >= map->num_stripes) {
btrfs_crit(fs_info,

--
2.43.0

2023-12-13 14:44:23

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 12/13] btrfs: pass struct btrfs_io_geometry to set_io_stripe

Instead of passing three members of 'struct btrfs_io_geometry' into
set_io_stripe() pass a pointer to the whole structure and then get the needed
members out of btrfs_io_geometry.

Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5e4885a01796..440ac0c1c907 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6325,19 +6325,22 @@ static u64 btrfs_max_io_len(struct btrfs_chunk_map *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 btrfs_chunk_map *map, u32 stripe_index,
- u64 stripe_offset, u64 stripe_nr)
+static int set_io_stripe(struct btrfs_fs_info *fs_info, u64 logical,
+ u64 *length, struct btrfs_io_stripe *dst,
+ struct btrfs_chunk_map *map,
+ struct btrfs_io_geometry *io_geom)
{
- dst->dev = map->stripes[stripe_index].dev;
+ dst->dev = map->stripes[io_geom->stripe_index].dev;

- if (op == BTRFS_MAP_READ && btrfs_need_stripe_tree_update(fs_info, map->type))
+ if (io_geom->op == BTRFS_MAP_READ &&
+ btrfs_need_stripe_tree_update(fs_info, map->type))
return btrfs_get_raid_extent_offset(fs_info, logical, length,
- map->type, stripe_index, dst);
+ map->type,
+ io_geom->stripe_index, dst);

- dst->physical = map->stripes[stripe_index].physical +
- stripe_offset + btrfs_stripe_nr_to_offset(stripe_nr);
+ dst->physical = map->stripes[io_geom->stripe_index].physical +
+ io_geom->stripe_offset +
+ btrfs_stripe_nr_to_offset(io_geom->stripe_nr);
return 0;
}

@@ -6638,9 +6641,8 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
*/
if (is_single_device_io(fs_info, smap, map, num_alloc_stripes, op,
io_geom.mirror_num)) {
- ret = set_io_stripe(fs_info, op, logical, length, smap, map,
- io_geom.stripe_index, io_geom.stripe_offset,
- io_geom.stripe_nr);
+ ret = set_io_stripe(fs_info, logical, length, smap, map,
+ &io_geom);
if (mirror_num_ret)
*mirror_num_ret = io_geom.mirror_num;
*bioc_ret = NULL;
@@ -6692,11 +6694,8 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
* stripe into the bioc.
*/
for (i = 0; i < io_geom.num_stripes; i++) {
- ret = set_io_stripe(fs_info, op, logical, length,
- &bioc->stripes[i], map,
- io_geom.stripe_index,
- io_geom.stripe_offset,
- io_geom.stripe_nr);
+ ret = set_io_stripe(fs_info, logical, length,
+ &bioc->stripes[i], map, &io_geom);
if (ret < 0)
break;
io_geom.stripe_index++;

--
2.43.0

2023-12-13 14:44:25

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 13/13] btrfs: pass btrfs_io_geometry into btrfs_max_io_len

Instead of passing three individual members of 'struct btrfs_io_geometry'
into btrfs_max_io_len(), pass a pointer to btrfs_io_geometry.

Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 440ac0c1c907..af803b162d0b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6277,17 +6277,16 @@ static void handle_ops_on_dev_replace(enum btrfs_map_op op,
bioc->replace_nr_stripes = nr_extra_stripes;
}

-static u64 btrfs_max_io_len(struct btrfs_chunk_map *map, enum btrfs_map_op op,
- u64 offset, u32 *stripe_nr, u64 *stripe_offset,
- u64 *full_stripe_start)
+static u64 btrfs_max_io_len(struct btrfs_chunk_map *map, u64 offset,
+ struct btrfs_io_geometry *io_geom)
{
/*
* Stripe_nr is the stripe where this block falls. stripe_offset is
* the offset of this block in its stripe.
*/
- *stripe_offset = offset & BTRFS_STRIPE_LEN_MASK;
- *stripe_nr = offset >> BTRFS_STRIPE_LEN_SHIFT;
- ASSERT(*stripe_offset < U32_MAX);
+ io_geom->stripe_offset = offset & BTRFS_STRIPE_LEN_MASK;
+ io_geom->stripe_nr = offset >> BTRFS_STRIPE_LEN_SHIFT;
+ ASSERT(io_geom->stripe_offset < U32_MAX);

if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK) {
unsigned long full_stripe_len =
@@ -6302,18 +6301,19 @@ static u64 btrfs_max_io_len(struct btrfs_chunk_map *map, enum btrfs_map_op op,
* to go rounddown(), not round_down(), as nr_data_stripes is
* not ensured to be power of 2.
*/
- *full_stripe_start =
- btrfs_stripe_nr_to_offset(
- rounddown(*stripe_nr, nr_data_stripes(map)));
+ io_geom->raid56_full_stripe_start = btrfs_stripe_nr_to_offset(
+ rounddown(io_geom->stripe_nr, nr_data_stripes(map)));

- ASSERT(*full_stripe_start + full_stripe_len > offset);
- ASSERT(*full_stripe_start <= offset);
+ ASSERT(io_geom->raid56_full_stripe_start + full_stripe_len >
+ offset);
+ ASSERT(io_geom->raid56_full_stripe_start <= offset);
/*
* For writes to RAID56, allow to write a full stripe set, but
* no straddling of stripe sets.
*/
- if (op == BTRFS_MAP_WRITE)
- return full_stripe_len - (offset - *full_stripe_start);
+ if (io_geom->op == BTRFS_MAP_WRITE)
+ return full_stripe_len -
+ (offset - io_geom->raid56_full_stripe_start);
}

/*
@@ -6321,7 +6321,7 @@ static u64 btrfs_max_io_len(struct btrfs_chunk_map *map, enum btrfs_map_op op,
* a single disk).
*/
if (map->type & BTRFS_BLOCK_GROUP_STRIPE_MASK)
- return BTRFS_STRIPE_LEN - *stripe_offset;
+ return BTRFS_STRIPE_LEN - io_geom->stripe_offset;
return U64_MAX;
}

@@ -6567,9 +6567,7 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,

map_offset = logical - map->start;
io_geom.raid56_full_stripe_start = (u64)-1;
- max_len = btrfs_max_io_len(map, io_geom.op, map_offset, &io_geom.stripe_nr,
- &io_geom.stripe_offset,
- &io_geom.raid56_full_stripe_start);
+ max_len = btrfs_max_io_len(map, map_offset, &io_geom);
*length = min_t(u64, map->chunk_len - map_offset, max_len);

down_read(&dev_replace->rwsem);

--
2.43.0

2023-12-13 14:44:35

by Johannes Thumshirn

[permalink] [raw]
Subject: [PATCH v2 11/13] btrfs: open code set_io_stripe for RAID56

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.

Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index efb31c3005b7..5e4885a01796 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6675,13 +6675,16 @@ int btrfs_map_block(struct btrfs_fs_info *fs_info, enum btrfs_map_op op,
btrfs_stripe_nr_to_offset(io_geom.stripe_nr *
nr_data_stripes(map));
for (int i = 0; i < io_geom.num_stripes; i++) {
- ret = set_io_stripe(fs_info, op, logical, length,
- &bioc->stripes[i], map,
- (i + io_geom.stripe_nr) % io_geom.num_stripes,
- io_geom.stripe_offset,
- io_geom.stripe_nr);
- if (ret < 0)
- break;
+ struct btrfs_io_stripe *dst = &bioc->stripes[i];
+ u32 stripe_index;
+
+ stripe_index =
+ (i + io_geom.stripe_nr) % io_geom.num_stripes;
+ dst->dev = map->stripes[stripe_index].dev;
+ dst->physical =
+ map->stripes[stripe_index].physical +
+ io_geom.stripe_offset +
+ btrfs_stripe_nr_to_offset(io_geom.stripe_nr);
}
} else {
/*

--
2.43.0

2023-12-13 18:59:29

by David Sterba

[permalink] [raw]
Subject: Re: [PATCH v2 00/13] btrfs: clean up RAID I/O geometry calculation

On Wed, Dec 13, 2023 at 06:42:55AM -0800, Johannes Thumshirn wrote:
> The calculation of the RAID I/O geometry in btrfs_map_block has been a maze of
> if-else statements for a very long time and the advent of the
> raid-stripe-tree made the situation even worse.
>
> This patchset refactors btrfs_map_block() to untagle the maze and make I/O
> geometry setting easier to follow, but does not introduce any functional
> changes.
>
> I've also run it through Josef's CI and there have been test failures, but
> none of them introduced by these patches.
>
> ---
> Changes in v2:
> - add btrfs_map_op into struct btrfs_io_geometry
> - split RAID56 read and write into two different helpers
> - drop redundand 'for' in helper function names
> - kept dev_replace_is_ongoing variable name
> - Link to v1: https://lore.kernel.org/r/[email protected]
>
> ---
> Johannes Thumshirn (13):
> btrfs: factor out helper for single device IO check
> btrfs: re-introduce struct btrfs_io_geometry
> btrfs: factor out block-mapping for RAID0
> btrfs: factor out RAID1 block mapping
> btrfs: factor out block mapping for DUP profiles
> btrfs: factor out block mapping for RAID10
> btrfs: reduce scope of data_stripes in btrfs_map_block
> btrfs: factor out block mapping for RAID5/6
> btrfs: factor out block mapping for single profiles
> btrfs: btrfs: untagle if else maze in btrfs_map_block
> btrfs: open code set_io_stripe for RAID56
> btrfs: pass struct btrfs_io_geometry to set_io_stripe
> btrfs: pass btrfs_io_geometry into btrfs_max_io_len

Added to misc-next with some adjustments, thanks. I've noticed some of
the helpers can take const parameters, some lines that can be joined so
the expression is one one line, but there are calculations like in
"btrfs: open code set_io_stripe for RAID56" where it's up to your
creativity.