On 2.07.20 г. 17:04 ч., Johannes Thumshirn wrote:
> On 02/07/2020 15:47, Nikolay Borisov wrote:
> [...]
>> - bbio->raid_map = (u64 *)((void *)bbio->stripes +
>> - sizeof(struct btrfs_bio_stripe) *
>> - num_alloc_stripes +
>> - sizeof(int) * tgtdev_indexes);
>
> That one took me a while to be convinced it is correct.
There are 2 aspects to this:
1. I think 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. The second is that 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.
>
>>
>> /* Work out the disk rotation on this stripe-set */
>> div_u64_rem(stripe_nr, num_stripes, &rot);
>> @@ -6171,25 +6178,14 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info,
>> if (map->type & BTRFS_BLOCK_GROUP_RAID6)
>> bbio->raid_map[(i+rot+1) % num_stripes] =
>> RAID6_Q_STRIPE;
>> - }
>> -
>>
>> - 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++;
>> + sort_parity_stripes(bbio, num_stripes);
>> }
>>
>> +
>
> Stray newline.
>
>
