linux-kernel - Re: [PATCH] btrfs: zoned: remove redundant space_info lock and variable in do_allocation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260112185637.GB450687@zen.localdomain>
Date: Mon, 12 Jan 2026 10:56:37 -0800
From: Boris Burkov <boris@....io>
To: Jiasheng Jiang <jiashengjiangcool@...il.com>
Cc: Chris Mason <clm@...com>, David Sterba <dsterba@...e.com>,
	linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs: zoned: remove redundant space_info lock and
 variable in do_allocation_zoned

On Mon, Jan 12, 2026 at 03:40:40PM +0000, Jiasheng Jiang wrote:
> In do_allocation_zoned, the code acquires space_info->lock followed by
> block_group->lock. However, the critical section does not access or
> modify any members of the space_info structure, making the lock
> acquisition and the space_info local variable redundant.
> 
> This unnecessary locking also creates a potential circular wait deadlock
> (AB-BA) risk. Other paths, such as background reclaim or space reporting,
> could acquire block_group->lock and subsequently attempt to acquire
> space_info->lock to update global counters.

This is not a valid point. The lock order between space_info->lock and
block_group->lock is already strongly established and is the same as
here: space_info, then block_group. Any call site that did the opposite
would already be a deadlock risk.

With that said, this patch does look OK in the sense that it doesn't
appear that we need to synchronize with anything blocking on the
space_info->lock here.

Please update the commit message to not include anything about avoiding
possible deadlocks.

> 
> This behavior diverges from other Zoned-specific paths like
> __btrfs_add_free_space_zoned which only rely on block_group->lock.
> Removing the acquisition of space_info->lock and its corresponding
> definition in do_allocation_zoned eliminates the deadlock risk and
> improves concurrency by reducing contention on the global space_info lock.
> 
> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@...il.com>
> ---
>  fs/btrfs/extent-tree.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index e4cae34620d1..43d78056c274 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3839,7 +3839,6 @@ static int do_allocation_zoned(struct btrfs_block_group *block_group,
>  			       struct btrfs_block_group **bg_ret)
>  {
>  	struct btrfs_fs_info *fs_info = block_group->fs_info;
> -	struct btrfs_space_info *space_info = block_group->space_info;
>  	struct btrfs_free_space_ctl *ctl = block_group->free_space_ctl;
>  	u64 start = block_group->start;
>  	u64 num_bytes = ffe_ctl->num_bytes;
> @@ -3900,7 +3899,6 @@ static int do_allocation_zoned(struct btrfs_block_group *block_group,
>  		 */
>  	}
>  
> -	spin_lock(&space_info->lock);
>  	spin_lock(&block_group->lock);
>  	spin_lock(&fs_info->treelog_bg_lock);
>  	spin_lock(&fs_info->relocation_bg_lock);
> @@ -4002,7 +4000,6 @@ static int do_allocation_zoned(struct btrfs_block_group *block_group,
>  	spin_unlock(&fs_info->relocation_bg_lock);
>  	spin_unlock(&fs_info->treelog_bg_lock);
>  	spin_unlock(&block_group->lock);
> -	spin_unlock(&space_info->lock);
>  	return ret;
>  }
>  
> -- 
> 2.25.1
>