Re: [PATCH RESEND] btrfs: Fix exception handling in relocating_repair_kthread()

[Naohiro Aota]

On Tue Mar 4, 2025 at 5:31 AM JST, Markus Elfring wrote:
> From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> Date: Wed, 22 Mar 2023 20:10:09 +0100
>
> The label “out” was used to jump to another pointer check despite of
> the detail in the implementation of the function
> “relocating_repair_kthread” that it was determined already that
> a corresponding variable contained a null pointer because of
> a failed call of the function “btrfs_lookup_block_group”.
>
> * Thus use more appropriate labels instead.
>
> * Delete a redundant check.
>
>
> This issue was detected by using the Coccinelle software.

Since this function is local to the zoned feature, could I have "zoned: "
added to the subject line?

Other than that, it looks reasonable to me.

Reviewed-by: Naohiro Aota <naohiro.aota@xxxxxxx>

>
> Fixes: f7ef5287a63d ("btrfs: zoned: relocate block group to repair IO failure in zoned filesystems")
> Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  fs/btrfs/volumes.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6d0124b6e79e..de11ad6c8740 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -8096,23 +8096,22 @@ static int relocating_repair_kthread(void *data)
>  	/* Ensure block group still exists */
>  	cache = btrfs_lookup_block_group(fs_info, target);
>  	if (!cache)
> -		goto out;
> +		goto unlock;
>
>  	if (!test_bit(BLOCK_GROUP_FLAG_RELOCATING_REPAIR, &cache->runtime_flags))
> -		goto out;
> +		goto put_block_group;
>
>  	ret = btrfs_may_alloc_data_chunk(fs_info, target);
>  	if (ret < 0)
> -		goto out;
> +		goto put_block_group;
>
>  	btrfs_info(fs_info,
>  		   "zoned: relocating block group %llu to repair IO failure",
>  		   target);
>  	ret = btrfs_relocate_chunk(fs_info, target);
> -
> -out:
> -	if (cache)
> -		btrfs_put_block_group(cache);
> +put_block_group:
> +	btrfs_put_block_group(cache);
> +unlock:
>  	mutex_unlock(&fs_info->reclaim_bgs_lock);
>  	btrfs_exclop_finish(fs_info);
>  	sb_end_write(fs_info->sb);
> --
> 2.40.0