Re: Balance loops: what we know so far

[Zygo Blaxell <ce3g8jdj@umail.furryterror.org>]

[-- Attachment #1: Type: text/plain, Size: 3916 bytes --]

On Mon, May 11, 2020 at 04:31:32PM +0800, Qu Wenruo wrote:
> Hi Zygo,
> 
> Would you like to test this diff?
> 
> Although I haven't find a solid reason yet, there is another report and
> with the help from the reporter, it turns out that balance hangs at
> relocating DATA_RELOC tree block.
> 
> After some more digging, DATA_RELOC tree doesn't need REF_COW bit at all
> since we can't create snapshot for data reloc tree.
> 
> By removing the REF_COW bit, we could ensure that data reloc tree always
> get cowed for relocation (just like extent tree), this would hugely
> reduce the complexity for data reloc tree.
> 
> Not sure if this would help, but it passes my local balance run.

I ran it last night.  It did 30804 loops during a metadata block group
balance, and is now looping on a data block group as I write this.

> Thanks,
> Qu

> From 82f3b96a68561b2de9712262cb652192b8ea9b1b Mon Sep 17 00:00:00 2001
> From: Qu Wenruo <wqu@suse.com>
> Date: Mon, 11 May 2020 16:27:43 +0800
> Subject: [PATCH] btrfs: Remove the REF_COW bit for data reloc tree
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/disk-io.c    | 9 ++++++++-
>  fs/btrfs/inode.c      | 6 ++++--
>  fs/btrfs/relocation.c | 3 ++-
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 56675d3cd23a..cb90966a8aab 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1418,9 +1418,16 @@ static int btrfs_init_fs_root(struct btrfs_root *root)
>  	if (ret)
>  		goto fail;
>  
> -	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
> +	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID &&
> +	    root->root_key.objectid != BTRFS_DATA_RELOC_TREE_OBJECTID) {
>  		set_bit(BTRFS_ROOT_REF_COWS, &root->state);
>  		btrfs_check_and_init_root_item(&root->root_item);
> +	} else if (root->root_key.objectid == BTRFS_DATA_RELOC_TREE_OBJECTID) {
> +		/*
> +		 * Data reloc tree won't be snapshotted, thus it's COW only
> +		 * tree, it's needed to set TRACK_DIRTY bit for it.
> +		 */
> +		set_bit(BTRFS_ROOT_TRACK_DIRTY, &root->state);
>  	}
>  
>  	btrfs_init_free_ino_ctl(root);
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 5d567082f95a..71841535c7ca 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4129,7 +4129,8 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
>  	 * extent just the way it is.
>  	 */
>  	if (test_bit(BTRFS_ROOT_REF_COWS, &root->state) ||
> -	    root == fs_info->tree_root)
> +	    root == fs_info->tree_root ||
> +	    root->root_key.objectid == BTRFS_DATA_RELOC_TREE_OBJECTID)
>  		btrfs_drop_extent_cache(BTRFS_I(inode), ALIGN(new_size,
>  					fs_info->sectorsize),
>  					(u64)-1, 0);
> @@ -4334,7 +4335,8 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
>  
>  		if (found_extent &&
>  		    (test_bit(BTRFS_ROOT_REF_COWS, &root->state) ||
> -		     root == fs_info->tree_root)) {
> +		     root == fs_info->tree_root ||
> +		     root->root_key.objectid == BTRFS_DATA_RELOC_TREE_OBJECTID)) {
>  			struct btrfs_ref ref = { 0 };
>  
>  			bytes_deleted += extent_num_bytes;
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index f25deca18a5d..a85dd5d465f6 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -1087,7 +1087,8 @@ int replace_file_extents(struct btrfs_trans_handle *trans,
>  		 * if we are modifying block in fs tree, wait for readpage
>  		 * to complete and drop the extent cache
>  		 */
> -		if (root->root_key.objectid != BTRFS_TREE_RELOC_OBJECTID) {
> +		if (root->root_key.objectid != BTRFS_TREE_RELOC_OBJECTID &&
> +		    root->root_key.objectid != BTRFS_DATA_RELOC_TREE_OBJECTID) {
>  			if (first) {
>  				inode = find_next_inode(root, key.objectid);
>  				first = 0;
> -- 
> 2.26.2
> 





[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]