* [PATCH] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned
@ 2026-01-12 15:40 Jiasheng Jiang
2026-01-12 18:56 ` Boris Burkov
0 siblings, 1 reply; 8+ messages in thread
From: Jiasheng Jiang @ 2026-01-12 15:40 UTC (permalink / raw)
To: Chris Mason, David Sterba, linux-btrfs, linux-kernel; +Cc: Jiasheng Jiang
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 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@gmail.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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned
2026-01-12 15:40 [PATCH] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned Jiasheng Jiang
@ 2026-01-12 18:56 ` Boris Burkov
2026-01-12 20:22 ` [PATCH v2] " Jiasheng Jiang
0 siblings, 1 reply; 8+ messages in thread
From: Boris Burkov @ 2026-01-12 18:56 UTC (permalink / raw)
To: Jiasheng Jiang; +Cc: Chris Mason, David Sterba, linux-btrfs, linux-kernel
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@gmail.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
>
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned
2026-01-12 18:56 ` Boris Burkov
@ 2026-01-12 20:22 ` Jiasheng Jiang
2026-01-13 18:29 ` Boris Burkov
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jiasheng Jiang @ 2026-01-12 20:22 UTC (permalink / raw)
To: boris; +Cc: clm, dsterba, jiashengjiangcool, linux-btrfs, linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned
2026-01-12 20:22 ` [PATCH v2] " Jiasheng Jiang
@ 2026-01-13 18:29 ` Boris Burkov
2026-01-14 11:24 ` Johannes Thumshirn
2026-01-14 13:09 ` Naohiro Aota
2 siblings, 0 replies; 8+ messages in thread
From: Boris Burkov @ 2026-01-13 18:29 UTC (permalink / raw)
To: Jiasheng Jiang; +Cc: clm, dsterba, linux-btrfs, linux-kernel
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned
2026-01-12 20:22 ` [PATCH v2] " Jiasheng Jiang
2026-01-13 18:29 ` Boris Burkov
@ 2026-01-14 11:24 ` Johannes Thumshirn
2026-01-14 13:09 ` Naohiro Aota
2 siblings, 0 replies; 8+ messages in thread
From: Johannes Thumshirn @ 2026-01-14 11:24 UTC (permalink / raw)
To: Jiasheng Jiang, boris@bur.io
Cc: clm@fb.com, dsterba@suse.com, linux-btrfs@vger.kernel.org,
linux-kernel@vger.kernel.org
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>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned
2026-01-12 20:22 ` [PATCH v2] " Jiasheng Jiang
2026-01-13 18:29 ` Boris Burkov
2026-01-14 11:24 ` Johannes Thumshirn
@ 2026-01-14 13:09 ` Naohiro Aota
2026-01-14 14:44 ` [PATCH v3] " Jiasheng Jiang
2 siblings, 1 reply; 8+ messages in thread
From: Naohiro Aota @ 2026-01-14 13:09 UTC (permalink / raw)
To: Jiasheng Jiang, boris@bur.io
Cc: clm@fb.com, dsterba@suse.com, linux-btrfs@vger.kernel.org,
linux-kernel@vger.kernel.org
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>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned
2026-01-14 13:09 ` Naohiro Aota
@ 2026-01-14 14:44 ` Jiasheng Jiang
2026-02-04 14:40 ` David Sterba
0 siblings, 1 reply; 8+ messages in thread
From: Jiasheng Jiang @ 2026-01-14 14:44 UTC (permalink / raw)
To: naohiro.aota
Cc: boris, clm, dsterba, jiashengjiangcool, linux-btrfs, linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned
2026-01-14 14:44 ` [PATCH v3] " Jiasheng Jiang
@ 2026-02-04 14:40 ` David Sterba
0 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2026-02-04 14:40 UTC (permalink / raw)
To: Jiasheng Jiang
Cc: naohiro.aota, boris, clm, dsterba, linux-btrfs, linux-kernel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-02-04 14:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-12 15:40 [PATCH] btrfs: zoned: remove redundant space_info lock and variable in do_allocation_zoned Jiasheng Jiang
2026-01-12 18:56 ` Boris Burkov
2026-01-12 20:22 ` [PATCH v2] " Jiasheng Jiang
2026-01-13 18:29 ` Boris Burkov
2026-01-14 11:24 ` Johannes Thumshirn
2026-01-14 13:09 ` Naohiro Aota
2026-01-14 14:44 ` [PATCH v3] " Jiasheng Jiang
2026-02-04 14:40 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox