Re: [PATCH] btrfs: relocation: Use btrfs_find_all_leaves() to locate parent tree leaves of a data extent

David Sterba
2020-03-12T20:49:02Z

On Tue, Mar 10, 2020 at 04:14:15PM +0800, Qu Wenruo wrote:
> In relocation, we need to locate all parent tree leaves referring one
> data extent, thus we have a complex mechanism to iterate throught extent
> tree and subvolume trees to locate related leaves.
>
> However this is already done in backref.c, we have
> btrfs_find_all_leaves(), which can return a ulist containing all leaves
> referring to that data extent.
>
> Use btrfs_find_all_leaves() to replace find_data_references().
^^^^^^^^^^J^^^^^^^^^^

The function is called btrfs_find_all_leafs, which is wrong spelling of
leaves, I was tempted to rename it in the same patch but unfortunately
ther's one more untouched caller so it's for another one.

> There is a special handling for v1 space cache data extents, where we
> need to delete the v1 space cache data extents, to avoid those data
> extents to hang the data relocation.
>
> In this patch, the special handling is done by re-iterating the root
> tree leaf.
> Although it's a little less efficient than the old handling, considering
> we can reuse a lot of code, it should be acceptable.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> This patch is originally in my backref cache branch, but since it's
> pretty independent from other backref cache code, and straightforward to
> test/review, it's sent for more comprehensive test/review/merge.
> ---
> fs/btrfs/backref.c | 8 +-
> fs/btrfs/backref.h | 4 +
> fs/btrfs/relocation.c | 314 ++++++++----------------------------------
> 3 files changed, 62 insertions(+), 264 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 327e4480957b..f2728fb3ee8f 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -1409,10 +1409,10 @@ static void free_leaf_list(struct ulist *blocks)
> *
> * returns 0 on success, <0 on error
> */
> -static int btrfs_find_all_leafs(struct btrfs_trans_handle *trans,
> - struct btrfs_fs_info *fs_info, u64 bytenr,
> - u64 time_seq, struct ulist **leafs,
> - const u64 *extent_item_pos, bool ignore_offset)
> +int btrfs_find_all_leafs(struct btrfs_trans_handle *trans,
> + struct btrfs_fs_info *fs_info, u64 bytenr,
> + u64 time_seq, struct ulist **leafs,
> + const u64 *extent_item_pos, bool ignore_offset)
> {
> int ret;
>
> diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
> index 777f61dc081e..723d6da99114 100644
> --- a/fs/btrfs/backref.h
> +++ b/fs/btrfs/backref.h
> @@ -40,6 +40,10 @@ int iterate_inodes_from_logical(u64 logical, struct btrfs_fs_info *fs_info,
>
> int paths_from_inode(u64 inum, struct inode_fs_paths *ipath);
>
> +int btrfs_find_all_leafs(struct btrfs_trans_handle *trans,
> + struct btrfs_fs_info *fs_info, u64 bytenr,
> + u64 time_seq, struct ulist **leafs,
> + const u64 *extent_item_pos, bool ignore_offset);
> int btrfs_find_all_roots(struct btrfs_trans_handle *trans,
> struct btrfs_fs_info *fs_info, u64 bytenr,
> u64 time_seq, struct ulist **roots, bool ignore_offset);
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 02afe294ee2d..319d50c7ada5 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -23,6 +23,7 @@
> #include "print-tree.h"
> #include "delalloc-space.h"
> #include "block-group.h"
> +#include "backref.h"
>
> /*
> * Relocation overview
> @@ -3620,31 +3621,6 @@ static int __add_tree_block(struct reloc_control *rc,
> return ret;
> }
>
> -/*
> - * helper to check if the block use full backrefs for pointers in it
> - */
> -static int block_use_full_backref(struct reloc_control *rc,
> - struct extent_buffer *eb)
> -{
> - u64 flags;
> - int ret;
> -
> - if (btrfs_header_flag(eb, BTRFS_HEADER_FLAG_RELOC) ||
> - btrfs_header_backref_rev(eb) < BTRFS_MIXED_BACKREF_REV)
> - return 1;
> -
> - ret = btrfs_lookup_extent_info(NULL, rc->extent_root->fs_info,
> - eb->start, btrfs_header_level(eb), 1,
> - NULL, &flags);
> - BUG_ON(ret);
> -
> - if (flags & BTRFS_BLOCK_FLAG_FULL_BACKREF)
> - ret = 1;
> - else
> - ret = 0;
> - return ret;
> -}
> -
> static int delete_block_group_cache(struct btrfs_fs_info *fs_info,
> struct btrfs_block_group *block_group,
> struct inode *inode,
> @@ -3688,174 +3664,42 @@ static int delete_block_group_cache(struct btrfs_fs_info *fs_info,
> }
>
> /*
> - * helper to add tree blocks for backref of type BTRFS_EXTENT_DATA_REF_KEY
> - * this function scans fs tree to find blocks reference the data extent
> + * Helper function to locate the free space cache EXTENT_DATA in root tree leaf
> + * and delete the cache inode, to avoid free space cache data extent blocking
> + * data relocation.
> */
> -static int find_data_references(struct reloc_control *rc,
> - struct btrfs_key *extent_key,
> - struct extent_buffer *leaf,
> - struct btrfs_extent_data_ref *ref,
> - struct rb_root *blocks)
> +static int delete_v1_space_cache(struct btrfs_fs_info *fs_info,
> + struct extent_buffer *leaf,

The fs_info seems to be redundant, so I've replaced it with
leaf->fs_info here

> + ret = delete_block_group_cache(fs_info, block_group, NULL,
> + space_cache_ino);
> + return ret;
> }