Re: [PATCH 09/10] btrfs-progs: refrom block groups caches structure

Qu Wenruo
2019-12-05T07:52:02Z

[-- Attachment #1.1: Type: text/plain, Size: 2110 bytes --]

On 2019/12/5 下午12:29, damenly.su@gmail.com wrote:
> From: Su Yue <Damenly_Su@gmx.com>
>
> This commit organises block groups cache in
> btrfs_fs_info::block_group_cache_tree. And any dirty block groups are
> linked in transaction_handle::dirty_bgs.
>
> To keep coherence of bisect, it does almost replace in place:
> 1. Replace the old btrfs group lookup functions with new functions
> introduced in former commits.
> 2. set_extent_bits(..., BLOCK_GROUP_DIRYT) things are replaced by linking
> the block group cache into trans::dirty_bgs. Checking and clearing bits
> are transformed too.
> 3. set_extent_bits(..., bit | EXTENT_LOCKED) things are replaced by
> new the btrfs_add_block_group_cache() which inserts caches into
> btrfs_fs_info::block_group_cache_tree directly. Other operations are
> converted to tree operations.

Great cleanup and code unification.

Overall looks good, just small nitpicks inlined below.
>
> Signed-off-by: Su Yue <Damenly_Su@gmx.com>
> ---
> cmds/rescue-chunk-recover.c | 4 +-
> extent-tree.c | 211 ++++++------------------------------
> image/main.c | 5 +-
> transaction.c | 3 +-
> 4 files changed, 38 insertions(+), 185 deletions(-)
>
> diff --git a/cmds/rescue-chunk-recover.c b/cmds/rescue-chunk-recover.c
> index 461b66c6e13b..a13acc015d11 100644

> @@ -2699,25 +2571,22 @@ int btrfs_free_block_groups(struct btrfs_fs_info *info)
> struct btrfs_block_group_cache *cache;
> u64 start;
> u64 end;
> - u64 ptr;
> int ret;
>
> - while(1) {
> - ret = find_first_extent_bit(&info->block_group_cache, 0,
> - &start, &end, (unsigned int)-1);
> - if (ret)
> + while (rb_first(&info->block_group_cache_tree)) {
> + cache = btrfs_lookup_first_block_group(info, 0);
> + if (!cache)

Since we're freeing all block groups, what about
rbtree_postorder_for_each_entry_safe()?

That would be faster than rb_first() as we don't need to balance the tree.

Despite that, the patch looks great to me.
Especially for that -185 part.

Thanks,
Qu

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]