Re: [PATCH 8/8] btrfs: add support for multiple global roots

Qu Wenruo
2021-11-06T01:51:37Z

On 2021/11/6 09:18, Qu Wenruo wrote:
>
>
> On 2021/11/6 04:49, Josef Bacik wrote:
>> With extent tree v2 you will be able to create multiple csum, extent,
>> and free space trees.  They will be used based on the block group, which
>> will now use the block_group_item->chunk_objectid to point to the set of
>> global roots that it will use.  When allocating new block groups we'll
>> simply mod the gigabyte offset of the block group against the number of
>> global roots we have and that will be the block groups global id.
>>
>>  From there we can take the bytenr that we're modifying in the respective
>> tree, look up the block group and get that block groups corresponding
>> global root id.  From there we can get to the appropriate global root
>> for that bytenr.
>>
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>> ---
>>   fs/btrfs/block-group.c     | 11 +++++++--
>>   fs/btrfs/block-group.h     |  1 +
>>   fs/btrfs/ctree.h           |  2 ++
>>   fs/btrfs/disk-io.c         | 49 +++++++++++++++++++++++++++++++-------
>>   fs/btrfs/free-space-tree.c |  2 ++
>>   fs/btrfs/transaction.c     | 15 ++++++++++++
>>   fs/btrfs/tree-checker.c    | 21 ++++++++++++++--
>>   7 files changed, 88 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
>> index 7eb0a8632a01..85516f2fd5da 100644
>> --- a/fs/btrfs/block-group.c
>> +++ b/fs/btrfs/block-group.c
>> @@ -2002,6 +2002,7 @@ static int read_one_block_group(struct
>> btrfs_fs_info *info,
>>       cache->length = key->offset;
>>       cache->used = btrfs_stack_block_group_used(bgi);
>>       cache->flags = btrfs_stack_block_group_flags(bgi);
>> +    cache->global_root_id = btrfs_stack_block_group_chunk_objectid(bgi);
>>
>>       set_free_space_tree_thresholds(cache);
>>
>> @@ -2284,7 +2285,7 @@ static int insert_block_group_item(struct
>> btrfs_trans_handle *trans,
>>       spin_lock(&block_group->lock);
>>       btrfs_set_stack_block_group_used(&bgi, block_group->used);
>>       btrfs_set_stack_block_group_chunk_objectid(&bgi,
>> -                BTRFS_FIRST_CHUNK_TREE_OBJECTID);
>> +                           block_group->global_root_id);
>>       btrfs_set_stack_block_group_flags(&bgi, block_group->flags);
>>       key.objectid = block_group->start;
>>       key.type = BTRFS_BLOCK_GROUP_ITEM_KEY;
>> @@ -2460,6 +2461,12 @@ struct btrfs_block_group
>> *btrfs_make_block_group(struct btrfs_trans_handle *tran
>>       cache->flags = type;
>>       cache->last_byte_to_unpin = (u64)-1;
>>       cache->cached = BTRFS_CACHE_FINISHED;
>> +    cache->global_root_id = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
>> +
>> +    if (btrfs_fs_incompat(fs_info, EXTENT_TREE_V2))
>> +        cache->global_root_id = div64_u64(cache->start, SZ_1G) %
>> +            fs_info->nr_global_roots;
>> +
>
> Any special reason for this complex global_root_id calculation?
>
> My initial assumption for global trees is pretty simple, just something
> like (CSUM_TREE, ROOT_ITEM, bg bytenr) or (EXTENT_TREE, ROOT_ITEM, bg
> bytenr) as their root key items.
>
> But this is definitely not the case here.
>
> Thus I'm wondering why we're not using something more simple.

And I also forgot that, for smaller fs, we could have metadata only
sized several megabytes.

In that case, several metadata block groups would share the same
global_root_id, which is definitely not a good idea.

Thanks,
Qu
>
> Thanks,
> Qu
>
>>       if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE))
>>           cache->needs_free_space = 1;
>>
>> @@ -2676,7 +2683,7 @@ static int update_block_group_item(struct
>> btrfs_trans_handle *trans,
>>       bi = btrfs_item_ptr_offset(leaf, path->slots[0]);
>>       btrfs_set_stack_block_group_used(&bgi, cache->used);
>>       btrfs_set_stack_block_group_chunk_objectid(&bgi,
>> -            BTRFS_FIRST_CHUNK_TREE_OBJECTID);
>> +                           cache->global_root_id);
>>       btrfs_set_stack_block_group_flags(&bgi, cache->flags);
>>       write_extent_buffer(leaf, &bgi, bi, sizeof(bgi));
>>       btrfs_mark_buffer_dirty(leaf);
>> diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
>> index 5878b7ce3b78..93aabc68bb6a 100644
>> --- a/fs/btrfs/block-group.h
>> +++ b/fs/btrfs/block-group.h
>> @@ -68,6 +68,7 @@ struct btrfs_block_group {
>>       u64 bytes_super;
>>       u64 flags;
>>       u64 cache_generation;
>> +    u64 global_root_id;
>>
>>       /*
>>        * If the free space extent count exceeds this number, convert
>> the block
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index b57367141b95..7de0cd2b87ec 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -1057,6 +1057,8 @@ struct btrfs_fs_info {
>>       spinlock_t relocation_bg_lock;
>>       u64 data_reloc_bg;
>>
>> +    u64 nr_global_roots;
>> +
>>       spinlock_t zone_active_bgs_lock;
>>       struct list_head zone_active_bgs;
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 45b2bde43150..a8bc00d17b26 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -1295,13 +1295,33 @@ struct btrfs_root *btrfs_global_root(struct
>> btrfs_fs_info *fs_info,
>>       return root;
>>   }
>>
>> +static u64 btrfs_global_root_id(struct btrfs_fs_info *fs_info, u64
>> bytenr)
>> +{
>> +    struct btrfs_block_group *block_group;
>> +    u64 ret;
>> +
>> +    if (!btrfs_fs_incompat(fs_info, EXTENT_TREE_V2))
>> +        return 0;
>> +
>> +    if (likely(bytenr))
>> +        block_group = btrfs_lookup_block_group(fs_info, bytenr);
>> +    else
>> +        block_group = btrfs_lookup_first_block_group(fs_info, bytenr);
>> +    ASSERT(block_group);
>> +    if (!block_group)
>> +        return 0;
>> +    ret = block_group->global_root_id;
>> +    btrfs_put_block_group(block_group);
>> +    return ret;
>> +}
>> +
>>   struct btrfs_root *btrfs_csum_root(struct btrfs_fs_info *fs_info,
>>                      u64 bytenr)
>>   {
>>       struct btrfs_key key = {
>>           .objectid = BTRFS_CSUM_TREE_OBJECTID,
>>           .type = BTRFS_ROOT_ITEM_KEY,
>> -        .offset = 0,
>> +        .offset = btrfs_global_root_id(fs_info, bytenr),
>>       };
>>
>>       return btrfs_global_root(fs_info, &key);
>> @@ -1313,7 +1333,7 @@ struct btrfs_root *btrfs_extent_root(struct
>> btrfs_fs_info *fs_info,
>>       struct btrfs_key key = {
>>           .objectid = BTRFS_EXTENT_TREE_OBJECTID,
>>           .type = BTRFS_ROOT_ITEM_KEY,
>> -        .offset = 0,
>> +        .offset = btrfs_global_root_id(fs_info, bytenr),
>>       };
>>
>>       return btrfs_global_root(fs_info, &key);
>> @@ -2094,7 +2114,6 @@ static void backup_super_roots(struct
>> btrfs_fs_info *info)
>>   {
>>       const int next_backup = info->backup_root_index;
>>       struct btrfs_root_backup *root_backup;
>> -    struct btrfs_root *csum_root = btrfs_csum_root(info, 0);
>>
>>       root_backup = info->super_for_commit->super_roots + next_backup;
>>
>> @@ -2128,6 +2147,7 @@ static void backup_super_roots(struct
>> btrfs_fs_info *info)
>>               btrfs_header_level(info->block_group_root->node));
>>       } else {
>>           struct btrfs_root *extent_root = btrfs_extent_root(info, 0);
>> +        struct btrfs_root *csum_root = btrfs_csum_root(info, 0);
>>
>>           btrfs_set_backup_extent_root(root_backup,
>>                            extent_root->node->start);
>> @@ -2135,6 +2155,12 @@ static void backup_super_roots(struct
>> btrfs_fs_info *info)
>>                   btrfs_header_generation(extent_root->node));
>>           btrfs_set_backup_extent_root_level(root_backup,
>>                       btrfs_header_level(extent_root->node));
>> +
>> +        btrfs_set_backup_csum_root(root_backup, csum_root->node->start);
>> +        btrfs_set_backup_csum_root_gen(root_backup,
>> +                           btrfs_header_generation(csum_root->node));
>> +        btrfs_set_backup_csum_root_level(root_backup,
>> +                         btrfs_header_level(csum_root->node));
>>       }
>>
>>       /*
>> @@ -2156,12 +2182,6 @@ static void backup_super_roots(struct
>> btrfs_fs_info *info)
>>       btrfs_set_backup_dev_root_level(root_backup,
>>                          btrfs_header_level(info->dev_root->node));
>>
>> -    btrfs_set_backup_csum_root(root_backup, csum_root->node->start);
>> -    btrfs_set_backup_csum_root_gen(root_backup,
>> -                       btrfs_header_generation(csum_root->node));
>> -    btrfs_set_backup_csum_root_level(root_backup,
>> -                     btrfs_header_level(csum_root->node));
>> -
>>       btrfs_set_backup_total_bytes(root_backup,
>>                    btrfs_super_total_bytes(info->super_copy));
>>       btrfs_set_backup_bytes_used(root_backup,
>> @@ -2550,6 +2570,7 @@ static int load_global_roots_objectid(struct
>> btrfs_root *tree_root,
>>   {
>>       struct btrfs_fs_info *fs_info = tree_root->fs_info;
>>       struct btrfs_root *root;
>> +    u64 max_global_id = 0;
>>       int ret;
>>       struct btrfs_key key = {
>>           .objectid = objectid,
>> @@ -2586,6 +2607,13 @@ static int load_global_roots_objectid(struct
>> btrfs_root *tree_root,
>>               break;
>>           btrfs_release_path(path);
>>
>> +        /*
>> +         * Just worry about this for extent tree, it'll be the same for
>> +         * everybody.
>> +         */
>> +        if (objectid == BTRFS_EXTENT_TREE_OBJECTID)
>> +            max_global_id = max(max_global_id, key.offset);
>> +
>>           found = true;
>>           root = read_tree_root_path(tree_root, path, &key);
>>           if (IS_ERR(root)) {
>> @@ -2603,6 +2631,9 @@ static int load_global_roots_objectid(struct
>> btrfs_root *tree_root,
>>       }
>>       btrfs_release_path(path);
>>
>> +    if (objectid == BTRFS_EXTENT_TREE_OBJECTID)
>> +        fs_info->nr_global_roots = max_global_id + 1;
>> +
>>       if (!found || ret) {
>>           if (objectid == BTRFS_CSUM_TREE_OBJECTID)
>>               set_bit(BTRFS_FS_STATE_NO_CSUMS, &fs_info->fs_state);
>> diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
>> index cf227450f356..60a73bcffaf1 100644
>> --- a/fs/btrfs/free-space-tree.c
>> +++ b/fs/btrfs/free-space-tree.c
>> @@ -24,6 +24,8 @@ static struct btrfs_root *btrfs_free_space_root(
>>           .type = BTRFS_ROOT_ITEM_KEY,
>>           .offset = 0,
>>       };
>> +    if (btrfs_fs_incompat(block_group->fs_info, EXTENT_TREE_V2))
>> +        key.offset = block_group->global_root_id;
>>       return btrfs_global_root(block_group->fs_info, &key);
>>   }
>>
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index ba8dd90ac3ce..e343ff8db05d 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -1827,6 +1827,14 @@ static void update_super_roots(struct
>> btrfs_fs_info *fs_info)
>>           super->cache_generation = 0;
>>       if (test_bit(BTRFS_FS_UPDATE_UUID_TREE_GEN, &fs_info->flags))
>>           super->uuid_tree_generation = root_item->generation;
>> +
>> +    if (btrfs_fs_incompat(fs_info, EXTENT_TREE_V2)) {
>> +        root_item = &fs_info->block_group_root->root_item;
>> +
>> +        super->block_group_root = root_item->bytenr;
>> +        super->block_group_root_generation = root_item->generation;
>> +        super->block_group_root_level = root_item->level;
>> +    }
>>   }
>>
>>   int btrfs_transaction_in_commit(struct btrfs_fs_info *info)
>> @@ -2261,6 +2269,13 @@ int btrfs_commit_transaction(struct
>> btrfs_trans_handle *trans)
>>       list_add_tail(&fs_info->chunk_root->dirty_list,
>>                 &cur_trans->switch_commits);
>>
>> +    if (btrfs_fs_incompat(fs_info, EXTENT_TREE_V2)) {
>> +        btrfs_set_root_node(&fs_info->block_group_root->root_item,
>> +                    fs_info->block_group_root->node);
>> +        list_add_tail(&fs_info->block_group_root->dirty_list,
>> +                  &cur_trans->switch_commits);
>> +    }
>> +
>>       switch_commit_roots(trans);
>>
>>       ASSERT(list_empty(&cur_trans->dirty_bgs));
>> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
>> index 1c33dd0e4afc..572f52d78297 100644
>> --- a/fs/btrfs/tree-checker.c
>> +++ b/fs/btrfs/tree-checker.c
>> @@ -639,8 +639,10 @@ static void block_group_err(const struct
>> extent_buffer *eb, int slot,
>>   static int check_block_group_item(struct extent_buffer *leaf,
>>                     struct btrfs_key *key, int slot)
>>   {
>> +    struct btrfs_fs_info *fs_info = leaf->fs_info;
>>       struct btrfs_block_group_item bgi;
>>       u32 item_size = btrfs_item_size_nr(leaf, slot);
>> +    u64 chunk_objectid;
>>       u64 flags;
>>       u64 type;
>>
>> @@ -663,8 +665,23 @@ static int check_block_group_item(struct
>> extent_buffer *leaf,
>>
>>       read_extent_buffer(leaf, &bgi, btrfs_item_ptr_offset(leaf, slot),
>>                  sizeof(bgi));
>> -    if (unlikely(btrfs_stack_block_group_chunk_objectid(&bgi) !=
>> -             BTRFS_FIRST_CHUNK_TREE_OBJECTID)) {
>> +    chunk_objectid = btrfs_stack_block_group_chunk_objectid(&bgi);
>> +    if (btrfs_fs_incompat(fs_info, EXTENT_TREE_V2)) {
>> +        /*
>> +         * We don't init the nr_global_roots until we load the global
>> +         * roots, so this could be 0 at mount time.  If it's 0 we'll
>> +         * just assume we're fine, and later we'll check against our
>> +         * actual value.
>> +         */
>> +        if (unlikely(fs_info->nr_global_roots &&
>> +                 chunk_objectid >= fs_info->nr_global_roots)) {
>> +            block_group_err(leaf, slot,
>> +    "invalid block group global root id, have %llu, needs to be <=
>> %llu",
>> +                    chunk_objectid,
>> +                    fs_info->nr_global_roots);
>> +            return -EUCLEAN;
>> +        }
>> +    } else if (unlikely(chunk_objectid !=
>> BTRFS_FIRST_CHUNK_TREE_OBJECTID)) {
>>           block_group_err(leaf, slot,
>>           "invalid block group chunk objectid, have %llu expect %llu",
>>                   btrfs_stack_block_group_chunk_objectid(&bgi),
>>