[v2] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned

[PATCH v2] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned

Posted by Jiasheng Jiang 3 weeks, 4 days ago

In do_allocation_zoned(), the code acquires space_info->lock before
block_group->lock. However, the critical section does not access or
modify any members of the space_info structure. Thus, the lock is
redundant as it provides no necessary synchronization here.

This change simplifies the locking logic and aligns the function with
other zoned paths, such as __btrfs_add_free_space_zoned(), which only
rely on block_group->lock. Since the 'space_info' local variable is
no longer used after removing the lock calls, it is also removed.

Removing this unnecessary lock reduces contention on the global
space_info lock, improving concurrency in the zoned allocation path.

Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
---
Changelog:

v1 -> v2:

1. Removed the description about avoiding deadlocks.
---
 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

Re: [PATCH v2] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned

Posted by Naohiro Aota 3 weeks, 3 days ago

On Tue Jan 13, 2026 at 5:22 AM JST, Jiasheng Jiang wrote:
> In do_allocation_zoned(), the code acquires space_info->lock before
> block_group->lock. However, the critical section does not access or
> modify any members of the space_info structure. Thus, the lock is
> redundant as it provides no necessary synchronization here.
>
> This change simplifies the locking logic and aligns the function with
> other zoned paths, such as __btrfs_add_free_space_zoned(), which only
> rely on block_group->lock. Since the 'space_info' local variable is
> no longer used after removing the lock calls, it is also removed.
>
> Removing this unnecessary lock reduces contention on the global
> space_info lock, improving concurrency in the zoned allocation path.
>
> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>

Looks good to me.

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

[PATCH v3] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned

Posted by Jiasheng Jiang 3 weeks, 3 days ago

In do_allocation_zoned(), the code acquires space_info->lock before
block_group->lock. However, the critical section does not access or
modify any members of the space_info structure. Thus, the lock is
redundant as it provides no necessary synchronization here.

This change simplifies the locking logic and aligns the function with
other zoned paths, such as __btrfs_add_free_space_zoned(), which only
rely on block_group->lock. Since the 'space_info' local variable is
no longer used after removing the lock calls, it is also removed.

Removing this unnecessary lock reduces contention on the global
space_info lock, improving concurrency in the zoned allocation path.

Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
---
Changelog:

v2 -> v3:

1. Removed the description about the 'space_info->lock' in the comment block above do_allocation_zoned().

v1 -> v2:

1. Removed the description about avoiding deadlocks.
---
 fs/btrfs/extent-tree.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index e4cae34620d1..36b06ee47c1a 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3824,9 +3824,8 @@ static int do_allocation_clustered(struct btrfs_block_group *block_group,
  * Lock nesting
  * ============
  *
- * space_info::lock
- *   block_group::lock
- *     fs_info::treelog_bg_lock
+ * block_group::lock
+ *   fs_info::treelog_bg_lock
  */
 
 /*
@@ -3839,7 +3838,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 +3898,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 +3999,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

Re: [PATCH v3] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned

Posted by David Sterba 3 days, 1 hour ago

On Wed, Jan 14, 2026 at 02:44:50PM +0000, Jiasheng Jiang wrote:
> In do_allocation_zoned(), the code acquires space_info->lock before
> block_group->lock. However, the critical section does not access or
> modify any members of the space_info structure. Thus, the lock is
> redundant as it provides no necessary synchronization here.
> 
> This change simplifies the locking logic and aligns the function with
> other zoned paths, such as __btrfs_add_free_space_zoned(), which only
> rely on block_group->lock. Since the 'space_info' local variable is
> no longer used after removing the lock calls, it is also removed.
> 
> Removing this unnecessary lock reduces contention on the global
> space_info lock, improving concurrency in the zoned allocation path.
> 
> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>

Added to for-next, thanks.

Re: [PATCH v2] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned

Posted by Johannes Thumshirn 3 weeks, 3 days ago

Above do_allocation_zoned() is a comment block describing the locking 
order, which even after this patch still states that space_info->lock 
has to be taken before block_group->lock. Please remove it as well.


With that fixed:

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH v2] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned

Posted by Boris Burkov 3 weeks, 3 days ago

On Mon, Jan 12, 2026 at 08:22:27PM +0000, Jiasheng Jiang wrote:
> In do_allocation_zoned(), the code acquires space_info->lock before
> block_group->lock. However, the critical section does not access or
> modify any members of the space_info structure. Thus, the lock is
> redundant as it provides no necessary synchronization here.
> 
> This change simplifies the locking logic and aligns the function with
> other zoned paths, such as __btrfs_add_free_space_zoned(), which only
> rely on block_group->lock. Since the 'space_info' local variable is
> no longer used after removing the lock calls, it is also removed.
> 
> Removing this unnecessary lock reduces contention on the global
> space_info lock, improving concurrency in the zoned allocation path.
> 

It would probably be best if Johannes or Naohiro had a look at this,
just in case there is some space_info synchronization need I'm missing
(without accessing any fields like you pointed out) but I think it
looks correct.

Reviewed-by: Boris Burkov <boris@bur.io>

> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
> ---
> Changelog:
> 
> v1 -> v2:
> 
> 1. Removed the description about avoiding deadlocks.
> ---
>  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
>