Re: [PATCH 06/21] btrfs: zoned: move mark_block_group_to_copy to zoned code

[David Sterba <dsterba@suse.cz>]

On Wed, Nov 24, 2021 at 01:30:32AM -0800, Johannes Thumshirn wrote:
> mark_block_group_to_copy() is only used in zoned filesystems, so move the
> code to zoned code.

Should it rather be moved to block-group.c, as it logically belongs to
the bg API? That it is used by zoned mode is ok, it's the zoned mode
using that particular helper from the API, but otherwise I don't see
anything specific in that helper.

> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  fs/btrfs/dev-replace.c | 126 +----------------------------------------
>  fs/btrfs/zoned.c       | 124 ++++++++++++++++++++++++++++++++++++++++
>  fs/btrfs/zoned.h       |   8 +++
>  3 files changed, 133 insertions(+), 125 deletions(-)
> 
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index 66fa61cb3f235..7572d80bff2ac 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -460,130 +460,6 @@ static char* btrfs_dev_name(struct btrfs_device *device)
>  		return rcu_str_deref(device->name);
>  }
>  
> -static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
> -				    struct btrfs_device *src_dev)
> -{
> -	struct btrfs_path *path;
> -	struct btrfs_key key;
> -	struct btrfs_key found_key;
> -	struct btrfs_root *root = fs_info->dev_root;
> -	struct btrfs_dev_extent *dev_extent = NULL;
> -	struct btrfs_block_group *cache;
> -	struct btrfs_trans_handle *trans;
> -	int ret = 0;
> -	u64 chunk_offset;
> -
> -	/* Do not use "to_copy" on non zoned filesystem for now */
> -	if (!btrfs_is_zoned(fs_info))
> -		return 0;

Let's take this as an example. My idea of removing the btrfs_is_zoned
calls is something like:

Caller:

	if (need_to_copy_bg(fs_info)) {
		ret = btrfs_mark_block_group_to_copy();
		if (ret)
			...;
	}

Where the need_to_copy_bg obviously wraps the is-zoned check. This
allows to separate the layers a and keep btrfs_mark_block_group_to_copy
in block-group.c where it belongs, while not opencoding the zoned inside
random functions.

This adds the indirection and perhaps too trivial wrappers, but reading
the code is not hurt and layers are more separated.

In summary:
- move the is-zoned checks to callers
- add wrappers that follow the semantics of the check (ie. is-zoned is
  an implementation detail)
- move functions to their respective API file if it exists