On 2.01.19 г. 12:08 ч., Qu Wenruo wrote:
>
>
> On 2019/1/2 下午4:57, Nikolay Borisov wrote:
>>
>>
>> On 25.12.18 г. 7:28 ч., Qu Wenruo wrote:
>>> The code is mostly ported from kernel with minimal change.
>>>
>>> Since btrfs-progs doesn't support replaying log, there is some code
>>> unnecessary for btrfs-progs, but to keep the code the same, that
>>> unnecessary code is kept as it.
>>>
>>> Now "btrfs check --repair" will update backup roots correctly.
>>>
>>> Signed-off-by: Qu Wenruo <w...@suse.com>
>>> ---
>>> disk-io.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 78 insertions(+)
>>>
>>> diff --git a/disk-io.c b/disk-io.c
>>> index 133835a4d063..f28a6b526391 100644
>>> --- a/disk-io.c
>>> +++ b/disk-io.c
>>> @@ -1621,6 +1621,83 @@ write_err:
>>> return ret;
>>> }
>>>
>>> +/*
>>> + * copy all the root pointers into the super backup array.
>>> + * this will bump the backup pointer by one when it is
>>> + * done
>>> + */
>>> +static void backup_super_roots(struct btrfs_fs_info *info)
>>> +{
>>> + struct btrfs_root_backup *root_backup;
>>> + int next_backup;
>>> + int last_backup;
>>> +
>>> + last_backup = find_best_backup_root(info->super_copy);
>>
>> Slightly offtopic: but find_best_backup_root only partially resembles
>> it's kernel counterpart - find_newest_super_backup. And it's missing the
>> code for handling wraparound. So in kernel space if the newest root is
>> at index 3 and index 0 backup also has the same gen then we switch the
>> index to 0, why is this needed?
>
> I'm not sure either.
>
> IIRC kernel shouldn't create two backup roots with the same gen.
So I heard back from Chris, his original code could have created
duplicates but since fed3b381145e ("Btrfs: do not backup tree roots when
fsync") this is no longer the case. The overflow handling should have
been removed as part of fed3b381145e but it didn't. So it makes sense to
actually remove it in a future patch, perhaps when the progs and kernel
versions are unified.
>
>> In any case I think
>> 'find_best_backup_root' in progs should be renamed to
>> 'find_newest_super_backup' and copied from kernel space.
>>
>>> + next_backup = (last_backup + 1) % BTRFS_NUM_BACKUP_ROOTS;
>>
>> This logic is different than the kernel one which does:
>> last_backup = (next_backup + BTRFS_NUM_BACKUP_ROOTS - 1) %
>> BTRFS_NUM_BACKUP_ROOTS;
>>
>> So in the kernel when info->backup_root_index == 0 then the newest
>> backup root is going to be written at position 3, the next one in pos 1,
>> the next in pos 2 and so on. OTOH in userspace we might potentially
>> start writing at 1 (which doesn't seem problematic per se). So why the
>> difference in logic?
>
> The logic is the same, for kernel it's 'last_backup' which should, and
> is, going backward one slot.
>
> And the next one is still written to next slot.
>
> Just as the dump super shows:
> $ btrfs ins dump-super -f /dev/data/btrfs | grep -A1 "backup "
> backup 0:
> backup_tree_root: 30818304 gen: 10 level: 0
> --
> backup 1:
> backup_tree_root: 30621696 gen: 7 level: 0
> --
> backup 2:
> backup_tree_root: 30703616 gen: 8 level: 0
> --
> backup 3:
> backup_tree_root: 30785536 gen: 9 level: 0
>
> So the logic should be the same.
>
> Just simpler implementation in btrfs-progs, without
> fs_info->backup_root_index.
>
> Thanks,
> Qu
>
>>
>>> +
>>> + /* just overwrite the last backup if we're at the same generation */
>>> + root_backup = info->super_copy->super_roots + last_backup;
>>> + if (btrfs_backup_tree_root_gen(root_backup) ==
>>> + btrfs_header_generation(info->tree_root->node))
>>> + next_backup = last_backup;
>>> +
>>> + root_backup = info->super_copy->super_roots + next_backup;
>>> +
>>> + /*
>>> + * make sure all of our padding and empty slots get zero filled
>>> + * regardless of which ones we use today
>>> + */
>>> + memset(root_backup, 0, sizeof(*root_backup));
>>> + btrfs_set_backup_tree_root(root_backup, info->tree_root->node->start);
>>> + btrfs_set_backup_tree_root_gen(root_backup,
>>> + btrfs_header_generation(info->tree_root->node));
>>> + btrfs_set_backup_tree_root_level(root_backup,
>>> + btrfs_header_level(info->tree_root->node));
>>> +
>>> + btrfs_set_backup_chunk_root(root_backup, info->chunk_root->node->start);
>>> + btrfs_set_backup_chunk_root_gen(root_backup,
>>> + btrfs_header_generation(info->chunk_root->node));
>>> + btrfs_set_backup_chunk_root_level(root_backup,
>>> + btrfs_header_level(info->chunk_root->node));
>>> +
>>> + btrfs_set_backup_extent_root(root_backup,
>>> info->extent_root->node->start);
>>> + btrfs_set_backup_extent_root_gen(root_backup,
>>> +
>>> btrfs_header_generation(info->extent_root->node));
>>> + btrfs_set_backup_extent_root_level(root_backup,
>>> + btrfs_header_level(info->extent_root->node));
>>> + /*
>>> + * we might commit during log recovery, which happens before we set
>>> + * the fs_root. Make sure it is valid before we fill it in.
>>> + */
>>> + if (info->fs_root && info->fs_root->node) {
>>> + btrfs_set_backup_fs_root(root_backup,
>>> + info->fs_root->node->start);
>>> + btrfs_set_backup_fs_root_gen(root_backup,
>>> + btrfs_header_generation(info->fs_root->node));
>>> + btrfs_set_backup_fs_root_level(root_backup,
>>> + btrfs_header_level(info->fs_root->node));
>>> + }
>>> +
>>> + btrfs_set_backup_dev_root(root_backup, info->dev_root->node->start);
>>> + btrfs_set_backup_dev_root_gen(root_backup,
>>> + btrfs_header_generation(info->dev_root->node));
>>> + btrfs_set_backup_dev_root_level(root_backup,
>>> +
>>> btrfs_header_level(info->dev_root->node));
>>> +
>>> + btrfs_set_backup_csum_root(root_backup, info->csum_root->node->start);
>>> + btrfs_set_backup_csum_root_gen(root_backup,
>>> + btrfs_header_generation(info->csum_root->node));
>>> + btrfs_set_backup_csum_root_level(root_backup,
>>> + btrfs_header_level(info->csum_root->node));
>>> +
>>> + btrfs_set_backup_total_bytes(root_backup,
>>> + btrfs_super_total_bytes(info->super_copy));
>>> + btrfs_set_backup_bytes_used(root_backup,
>>> + btrfs_super_bytes_used(info->super_copy));
>>> + btrfs_set_backup_num_devices(root_backup,
>>> + btrfs_super_num_devices(info->super_copy));
>>> +};
>>> +
>>> int write_all_supers(struct btrfs_fs_info *fs_info)
>>> {
>>> struct list_head *head = &fs_info->fs_devices->devices;
>>> @@ -1630,6 +1707,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info)
>>> int ret;
>>> u64 flags;
>>>
>>> + backup_super_roots(fs_info);
>>> sb = fs_info->super_copy;
>>> dev_item = &sb->dev_item;
>>> list_for_each_entry(dev, head, dev_list) {
>>>
>
