btrfs: zoned: fix lock ordering in btrfs_zone_activate()

Commit Message

Naohiro Aota Dec. 22, 2023, 4:56 a.m. UTC

The btrfs CI reported a lockdep warning as follows by running generic
generic/129.

   ======================================================
   WARNING: possible circular locking dependency detected
   6.7.0-rc5+ #1 Not tainted
   ------------------------------------------------------
   kworker/u5:5/793427 is trying to acquire lock:
   ffff88813256d028 (&cache->lock){+.+.}-{2:2}, at: btrfs_zone_finish_one_bg+0x5e/0x130
   but task is already holding lock:
   ffff88810a23a318 (&fs_info->zone_active_bgs_lock){+.+.}-{2:2}, at: btrfs_zone_finish_one_bg+0x34/0x130
   which lock already depends on the new lock.

   the existing dependency chain (in reverse order) is:
   -> #1 (&fs_info->zone_active_bgs_lock){+.+.}-{2:2}:
   ...
   -> #0 (&cache->lock){+.+.}-{2:2}:
   ...

This is because we take fs_info->zone_active_bgs_lock after a block_group's
lock in btrfs_zone_activate() while doing the opposite other place.

Fix the issue by expanding the fs_info->zone_active_bgs_lock's critical
section and taking it before a block_group's lock.

CC: [e-mail] # 6.6
Fixes: a7e1ac7bdc5a ("btrfs: zoned: reserve zones for an active metadata/system block group")
Signed-off-by: Naohiro Aota <[e-mail]>
---
 fs/btrfs/zoned.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

David Sterba Jan. 4, 2024, 4:28 p.m. UTC | #1

On Fri, Dec 22, 2023 at 01:56:34PM +0900, Naohiro Aota wrote:
> The btrfs CI reported a lockdep warning as follows by running generic
> generic/129.
> 
>    ======================================================
>    WARNING: possible circular locking dependency detected
>    6.7.0-rc5+ #1 Not tainted
>    ------------------------------------------------------
>    kworker/u5:5/793427 is trying to acquire lock:
>    ffff88813256d028 (&cache->lock){+.+.}-{2:2}, at: btrfs_zone_finish_one_bg+0x5e/0x130
>    but task is already holding lock:
>    ffff88810a23a318 (&fs_info->zone_active_bgs_lock){+.+.}-{2:2}, at: btrfs_zone_finish_one_bg+0x34/0x130
>    which lock already depends on the new lock.
> 
>    the existing dependency chain (in reverse order) is:
>    -> #1 (&fs_info->zone_active_bgs_lock){+.+.}-{2:2}:
>    ...
>    -> #0 (&cache->lock){+.+.}-{2:2}:
>    ...
> 
> This is because we take fs_info->zone_active_bgs_lock after a block_group's
> lock in btrfs_zone_activate() while doing the opposite other place.
> 
> Fix the issue by expanding the fs_info->zone_active_bgs_lock's critical
> section and taking it before a block_group's lock.
> 
> CC: [e-mail] # 6.6
> Fixes: a7e1ac7bdc5a ("btrfs: zoned: reserve zones for an active metadata/system block group")
> Signed-off-by: Naohiro Aota <[e-mail]>

Added to misc-next, thanks.

Patch

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 12066afc235c..ac9bbe0c4ffe 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -2072,6 +2072,7 @@  bool btrfs_zone_activate(struct btrfs_block_group *block_group)
 
 	map = block_group->physical_map;
 
+	spin_lock(&fs_info->zone_active_bgs_lock);
 	spin_lock(&block_group->lock);
 	if (test_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &block_group->runtime_flags)) {
 		ret = true;
@@ -2084,7 +2085,6 @@  bool btrfs_zone_activate(struct btrfs_block_group *block_group)
 		goto out_unlock;
 	}
 
-	spin_lock(&fs_info->zone_active_bgs_lock);
 	for (i = 0; i < map->num_stripes; i++) {
 		struct btrfs_zoned_device_info *zinfo;
 		int reserved = 0;
@@ -2104,20 +2104,17 @@  bool btrfs_zone_activate(struct btrfs_block_group *block_group)
 		 */
 		if (atomic_read(&zinfo->active_zones_left) <= reserved) {
 			ret = false;
-			spin_unlock(&fs_info->zone_active_bgs_lock);
 			goto out_unlock;
 		}
 
 		if (!btrfs_dev_set_active_zone(device, physical)) {
 			/* Cannot activate the zone */
 			ret = false;
-			spin_unlock(&fs_info->zone_active_bgs_lock);
 			goto out_unlock;
 		}
 		if (!is_data)
 			zinfo->reserved_active_zones--;
 	}
-	spin_unlock(&fs_info->zone_active_bgs_lock);
 
 	/* Successfully activated all the zones */
 	set_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &block_group->runtime_flags);
@@ -2125,8 +2122,6 @@  bool btrfs_zone_activate(struct btrfs_block_group *block_group)
 
 	/* For the active block group list */
 	btrfs_get_block_group(block_group);
-
-	spin_lock(&fs_info->zone_active_bgs_lock);
 	list_add_tail(&block_group->active_bg_list, &fs_info->zone_active_bgs);
 	spin_unlock(&fs_info->zone_active_bgs_lock);
 
@@ -2134,6 +2129,7 @@  bool btrfs_zone_activate(struct btrfs_block_group *block_group)
 
 out_unlock:
 	spin_unlock(&block_group->lock);
+	spin_unlock(&fs_info->zone_active_bgs_lock);
 	return ret;
 }