Re: [PATCH] btrfs: zoned: fix missing chunk metadata reservation

On 14.07.26 05:29, Guanghui Yang wrote:
> reserve_chunk_space() stores the return value of
> btrfs_zoned_activate_one_bg() in ret. The helper can return 1 after
> successfully activating a block group, but ret is later used to decide
> whether to reserve metadata for chunk tree updates.
> 
> As a result, successful activation skips btrfs_block_rsv_add() and leaves
> trans->chunk_bytes_reserved unchanged. Use a separate variable for the
> activation result so positive success does not affect the later
> reservation.
> 
> Fixes: b6a98021e401 ("btrfs: zoned: activate necessary block group")
> Cc: [email protected]
> Signed-off-by: Guanghui Yang <[email protected]>
> ---
>   fs/btrfs/block-group.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index ab76a5173272..00540b96c163 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -4532,12 +4532,14 @@ static void reserve_chunk_space(struct btrfs_trans_handle *trans,
>   		if (IS_ERR(bg)) {
>   			ret = PTR_ERR(bg);
>   		} else {
> +			int activate_ret;
> +
>   			/*
>   			 * We have a new chunk. We also need to activate it for
>   			 * zoned filesystem.
>   			 */
> -			ret = btrfs_zoned_activate_one_bg(info, true);
> -			if (ret < 0)
> +			activate_ret = btrfs_zoned_activate_one_bg(info, true);
> +			if (activate_ret < 0)
>   				return;
Looks good to me, but sashiko has a point here:
https://sashiko.dev/#/patchset/tencent_860054603C488A379E3D21126EA610D63108%40qq.com

I think a cleanup is needed, mind tackling that as well?

Also did you do a fstests run on it (with a zoned device)?

Otherwise,
Reviewed-by: Johannes Thumshirn <[email protected]>