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

[Nikolay Borisov <nborisov@suse.com>]

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.