Commit 608769a4 authored by Nikolay Borisov's avatar Nikolay Borisov Committed by David Sterba

btrfs: always initialize btrfs_bio::tgtdev_map/raid_map pointers

Since btrfs_bio always contains the extra space for the tgtdev_map and
raid_maps it's pointless to make the assignment iff specific conditions
are met.

Instead, always assign the pointers to their correct value at allocation
time. To accommodate this change also move code a bit in
__btrfs_map_block so that btrfs_bio::stripes array is always initialized
before the raid_map, subsequently move the call to sort_parity_stripes
in the 'if' building the raid_map, retaining the old behavior.

To better understand the change, there are 2 aspects to this:

1. The original code is harder to grasp because the calculations for
   initializing raid_map/tgtdev ponters are apart from the initial
   allocation of memory. Having them predicated on 2 separate checks
   doesn't help that either... So by moving the initialisation in
   alloc_btrfs_bio puts everything together.

2. tgtdev/raid_maps are now always initialized despite sometimes they
   might be equal i.e __btrfs_map_block_for_discard calls
   alloc_btrfs_bio with tgtdev = 0 but their usage should be predicated
   on external checks i.e. just because those pointers are non-null
   doesn't mean they are valid per-se. And actually while taking another
   look at __btrfs_map_block I saw a discrepancy:

   Original code initialised tgtdev_map if the following check is true:

	   if (dev_replace_is_ongoing && dev_replace->tgtdev != NULL)

   However, further down tgtdev_map is only used if the following check
   is true:

	if (dev_replace_is_ongoing && dev_replace->tgtdev != NULL && need_full_stripe(op))

  e.g. the additional need_full_stripe(op) predicate is there.
Signed-off-by: default avatarNikolay Borisov <nborisov@suse.com>
Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
[ copy more details from mail discussion ]
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent 3092c68f
...@@ -5522,6 +5522,9 @@ static struct btrfs_bio *alloc_btrfs_bio(int total_stripes, int real_stripes) ...@@ -5522,6 +5522,9 @@ static struct btrfs_bio *alloc_btrfs_bio(int total_stripes, int real_stripes)
atomic_set(&bbio->error, 0); atomic_set(&bbio->error, 0);
refcount_set(&bbio->refs, 1); refcount_set(&bbio->refs, 1);
bbio->tgtdev_map = (int *)(bbio->stripes + total_stripes);
bbio->raid_map = (u64 *)(bbio->tgtdev_map + real_stripes);
return bbio; return bbio;
} }
...@@ -6144,8 +6147,13 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, ...@@ -6144,8 +6147,13 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info,
ret = -ENOMEM; ret = -ENOMEM;
goto out; goto out;
} }
if (dev_replace_is_ongoing && dev_replace->tgtdev != NULL)
bbio->tgtdev_map = (int *)(bbio->stripes + num_alloc_stripes); for (i = 0; i < num_stripes; i++) {
bbio->stripes[i].physical = map->stripes[stripe_index].physical +
stripe_offset + stripe_nr * map->stripe_len;
bbio->stripes[i].dev = map->stripes[stripe_index].dev;
stripe_index++;
}
/* build raid_map */ /* build raid_map */
if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK && need_raid_map && if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK && need_raid_map &&
...@@ -6153,11 +6161,6 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, ...@@ -6153,11 +6161,6 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info,
u64 tmp; u64 tmp;
unsigned rot; unsigned rot;
bbio->raid_map = (u64 *)((void *)bbio->stripes +
sizeof(struct btrfs_bio_stripe) *
num_alloc_stripes +
sizeof(int) * tgtdev_indexes);
/* Work out the disk rotation on this stripe-set */ /* Work out the disk rotation on this stripe-set */
div_u64_rem(stripe_nr, num_stripes, &rot); div_u64_rem(stripe_nr, num_stripes, &rot);
...@@ -6171,25 +6174,13 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, ...@@ -6171,25 +6174,13 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info,
if (map->type & BTRFS_BLOCK_GROUP_RAID6) if (map->type & BTRFS_BLOCK_GROUP_RAID6)
bbio->raid_map[(i+rot+1) % num_stripes] = bbio->raid_map[(i+rot+1) % num_stripes] =
RAID6_Q_STRIPE; RAID6_Q_STRIPE;
}
sort_parity_stripes(bbio, num_stripes);
for (i = 0; i < num_stripes; i++) {
bbio->stripes[i].physical =
map->stripes[stripe_index].physical +
stripe_offset +
stripe_nr * map->stripe_len;
bbio->stripes[i].dev =
map->stripes[stripe_index].dev;
stripe_index++;
} }
if (need_full_stripe(op)) if (need_full_stripe(op))
max_errors = btrfs_chunk_max_errors(map); max_errors = btrfs_chunk_max_errors(map);
if (bbio->raid_map)
sort_parity_stripes(bbio, num_stripes);
if (dev_replace_is_ongoing && dev_replace->tgtdev != NULL && if (dev_replace_is_ongoing && dev_replace->tgtdev != NULL &&
need_full_stripe(op)) { need_full_stripe(op)) {
handle_ops_on_dev_replace(op, &bbio, dev_replace, &num_stripes, handle_ops_on_dev_replace(op, &bbio, dev_replace, &num_stripes,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment