Re: [PATCH v4 04/16] btrfs: remove remapped block groups from the free-space tree

[Boris Burkov <boris@bur.io>]

On Fri, Oct 24, 2025 at 07:12:05PM +0100, Mark Harmstone wrote:
> No new allocations can be done from block groups that have the REMAPPED flag
> set, so there's no value in their having entries in the free-space tree.
> 
> Prevent a search through the free-space tree being scheduled for such a
> block group, and prevent any additions to the in-memory free-space tree.
> 
> Signed-off-by: Mark Harmstone <mark@harmstone.com>
> ---
>  fs/btrfs/block-group.c      | 15 ++++++++++++---
>  fs/btrfs/free-space-cache.c |  3 +++
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index ec1e4fc0cd51..b5f2ec8d013f 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -933,6 +933,13 @@ int btrfs_cache_block_group(struct btrfs_block_group *cache, bool wait)
>  	if (btrfs_is_zoned(fs_info))
>  		return 0;
>  
> +	/*
> +	 * No allocations can be done from remapped block groups, so they have
> +	 * no entries in the free-space tree.
> +	 */
> +	if (cache->flags & BTRFS_BLOCK_GROUP_REMAPPED)
> +		return 0;
> +
>  	caching_ctl = kzalloc(sizeof(*caching_ctl), GFP_NOFS);
>  	if (!caching_ctl)
>  		return -ENOMEM;
> @@ -1248,9 +1255,11 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
>  	 * another task to attempt to create another block group with the same
>  	 * item key (and failing with -EEXIST and a transaction abort).
>  	 */
> -	ret = btrfs_remove_block_group_free_space(trans, block_group);
> -	if (ret)
> -		goto out;
> +	if (!(block_group->flags & BTRFS_BLOCK_GROUP_REMAPPED)) {
> +		ret = btrfs_remove_block_group_free_space(trans, block_group);
> +		if (ret)
> +			goto out;
> +	}

I feel like a comment or the commit message could explain the change to
the btrfs_remove_block_group bit more clearly. Like "remapped has no free
space so removing it is a no-op". If it is in fact a no-op, is there any
problem with calling it?

With that extra bit of doc/explanation, feel free to add
Reviewed-by: Boris Burkov <boris@bur.io>

>  
>  	ret = remove_block_group_item(trans, path, block_group);
>  	if (ret < 0)
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index ab873bd67192..ec9a97d75d10 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -2756,6 +2756,9 @@ int btrfs_add_free_space(struct btrfs_block_group *block_group,
>  {
>  	enum btrfs_trim_state trim_state = BTRFS_TRIM_STATE_UNTRIMMED;
>  
> +	if (block_group->flags & BTRFS_BLOCK_GROUP_REMAPPED)
> +		return 0;
> +
>  	if (btrfs_is_zoned(block_group->fs_info))
>  		return __btrfs_add_free_space_zoned(block_group, bytenr, size,
>  						    true);
> -- 
> 2.49.1
>