Re: [PATCH v2 05/10] btrfs: relocation: Refactor tree backref processing into its own function

Nikolay Borisov
2020-03-04T12:23:53Z

On 2.03.20 г. 11:45 ч., Qu Wenruo wrote:
> build_backref_tree() function is painfully long, as it has 3 big parts:
> - Tree backref handling
> - Weaving backref nodes
> - Useless nodes pruning
>
> This patch will move the tree backref handling into its own function,
> handle_one_tree_backref().
>
> And inside that function, the main works are determined by the backref
> key:
> - BTRFS_SHARED_BLOCK_REF_KEY
> We know the parent node bytenr directly.
> If the parent is cached, or it's root, call it a day.
> If the parent is not cached, add it pending list.
>
> - BTRFS_TREE_BLOCK_REF_KEY
> The most complex work.
> We need to grab the fs root, do a tree search to locate all its
> parent nodes, weaving all needed edges, and put all uncached edges to
> pending edge list.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/relocation.c | 395 +++++++++++++++++++++---------------------
> 1 file changed, 202 insertions(+), 193 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index d1e1d613ab98..04416489d87a 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -666,6 +666,206 @@ static struct btrfs_root *read_fs_root(struct btrfs_fs_info *fs_info,
> return btrfs_get_fs_root(fs_info, &key, false);
> }
>
> +static int handle_one_tree_backref(struct reloc_control *rc,
> + struct list_head *useless_node,
> + struct list_head *pending_edge,
> + struct btrfs_path *path,
> + struct btrfs_key *ref_key,
> + struct btrfs_key *tree_key,
> + struct backref_node *cur)

That function has 7 parameters and while @rc and @path are somewhat
self-explanatory others are not. Also it's really doing 2 distinct
things which, in my opinion, would be much better split across 2
functions - 1 handling BTRFS_SHARED_BLOCK_REF_KEY and the other handling
BTRFS_TREE_BLOCK_REF_KEY. For example the @tree_key,@useless_node and
@path are not used in BTRFS_SHARED_BLOCK_REF_KEY case. You'd have two
functions named:

handle_(in)?direct_tree_backref

One will take only 4 parameters the other will have to, unfortunately,
take all 7.