[PATCH v2] btrfs: fix a lockdep caused by path resolution during device scan

Qu Wenruo <[email protected]>
Newsgroups org.kernel.vger.linux-btrfs
Message-ID <49654338e775adb4b40124ea5a3abeb0dc6fdb42.1784027853.git.wqu@suse.com>
[BUG]
There is a lockdep report related to device scan:

======================================================
WARNING: possible circular locking dependency detected
7.2.0-20260712.rc2.git0.e3321fa3034d.300.fc44.s390x+debug #1 Not tainted
------------------------------------------------------
(udev-worker)/1653 is trying to acquire lock:
0000006919232220 (&type->i_mutex_dir_key#2){++++}-{3:3}, at: lookup_slow+0x3e/0x70
but task is already holding lock:
00000069238564d8 (&fs_devs->device_list_mutex){+.+.}-{3:3}, at: device_list_add.constprop.0+0x148/0xc60
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #5 (&fs_devs->device_list_mutex){+.+.}-{3:3}:
       lock_acquire+0x150/0x3f0
       __mutex_lock+0xba/0xdc0
       mutex_lock_nested+0x32/0x40
       write_all_supers+0x7a/0x670
       btrfs_sync_log+0xae6/0xdd0
       btrfs_sync_file+0x4fa/0x7a0
       __s390x_sys_fsync+0x52/0xa0
       __do_syscall+0x172/0x750
       system_call+0x72/0x90
-> #4 (&fs_info->tree_log_mutex){+.+.}-{3:3}:
       lock_acquire+0x150/0x3f0
       __mutex_lock+0xba/0xdc0
       mutex_lock_nested+0x32/0x40
       btrfs_sync_log+0xaba/0xdd0
       btrfs_sync_file+0x4fa/0x7a0
       __s390x_sys_fsync+0x52/0xa0
       __do_syscall+0x172/0x750
       system_call+0x72/0x90
-> #3 (btrfs_trans_num_extwriters){.+.+}-{0:0}:
       lock_acquire+0x150/0x3f0
       join_transaction+0x108/0x680
       start_transaction+0x21a/0x660
       btrfs_join_transaction+0x32/0x40
       btrfs_dirty_inode+0x52/0xf0
       touch_atime+0x90/0xc0
       filemap_read+0x446/0x450
       vfs_read+0x208/0x370
       ksys_read+0x88/0x120
       __do_syscall+0x172/0x750
       system_call+0x72/0x90
-> #2 (btrfs_trans_num_writers){.+.+}-{0:0}:
       reacquire_held_locks+0x14c/0x240
       __lock_release.isra.0+0xd8/0x380
       lock_release+0xf6/0x270
       percpu_up_read+0x28/0xf0
       __btrfs_end_transaction+0x178/0x1f0
       btrfs_dirty_inode+0x82/0xf0
       touch_atime+0x90/0xc0
       btrfs_file_mmap_prepare+0x8c/0xa0
       __mmap_region+0x214/0x780
       mmap_region+0x108/0x160
       do_mmap+0x402/0x5a0
       vm_mmap_pgoff+0x156/0x230
       ksys_mmap_pgoff+0x17e/0x220
       __s390x_sys_old_mmap+0xa8/0x140
       __do_syscall+0x172/0x750
       system_call+0x72/0x90
-> #1 (&mm->mmap_lock){++++}-{3:3}:
       lock_acquire+0x150/0x3f0
       __might_fault+0x7a/0xa0
       filldir64+0x11c/0x210
       offset_readdir+0x92/0x200
       iterate_dir+0xcc/0x2d0
       __do_sys_getdents64+0x7a/0x130
       __do_syscall+0x172/0x750
       system_call+0x72/0x90
-> #0 (&type->i_mutex_dir_key#2){++++}-{3:3}:
       check_prev_add+0x160/0xf40
       __lock_acquire+0x12aa/0x15a0
       lock_acquire+0x150/0x3f0
       down_read+0x5a/0x280
       lookup_slow+0x3e/0x70
       path_lookupat+0x1f0/0x370
       filename_lookup+0xce/0x1f0
       kern_path+0x48/0x70
       is_same_device+0x146/0x300
       device_list_add.constprop.0+0x1be/0xc60
       btrfs_scan_one_device+0x13a/0x2f0
       btrfs_control_ioctl+0x110/0x1e0
       __s390x_sys_ioctl+0xfa/0x130
       __do_syscall+0x172/0x750
       system_call+0x72/0x90
other info that might help us debug this:
Chain exists of:
  &type->i_mutex_dir_key#2 --> &fs_info->tree_log_mutex --> &fs_devs->device_list_mutex
 Possible unsafe locking scenario:
       CPU0                    CPU1
       ----                    ----
  lock(&fs_devs->device_list_mutex);
                               lock(&fs_info->tree_log_mutex);
                               lock(&fs_devs->device_list_mutex);
  rlock(&type->i_mutex_dir_key#2);
 *** DEADLOCK ***
2 locks held by (udev-worker)/1653:
 #0: 0000016c727051c8 (uuid_mutex){+.+.}-{3:3}, at: btrfs_control_ioctl+0x102/0x1e0
 #1: 00000069238564d8 (&fs_devs->device_list_mutex){+.+.}-{3:3}, at: device_list_add.constprop.0+0x148/0xc60
stack backtrace:
CPU: 2 UID: 0 PID: 1653 Comm: (udev-worker) Not tainted 7.2.0-20260712.rc2.git0.e3321fa3034d.300.fc44.s390x+debug #1 PREEMPT
Hardware name: IBM 3931 A01 701 (LPAR)
Call Trace:
 [<0000016c70680e3e>] dump_stack_lvl+0xae/0x108
 [<0000016c7078aa44>] print_circular_bug+0x1a4/0x230
 [<0000016c7078ac5c>] check_noncircular+0x18c/0x1b0
 [<0000016c7078c030>] check_prev_add+0x160/0xf40
 [<0000016c7078fbaa>] __lock_acquire+0x12aa/0x15a0
 [<0000016c7078fff0>] lock_acquire+0x150/0x3f0
 [<0000016c7180e2fa>] down_read+0x5a/0x280
 [<0000016c70b88dde>] lookup_slow+0x3e/0x70
 [<0000016c70b8f5d0>] path_lookupat+0x1f0/0x370
 [<0000016c70b900ae>] filename_lookup+0xce/0x1f0
 [<0000016c70b90218>] kern_path+0x48/0x70
 [<0000016c70f5b4a6>] is_same_device+0x146/0x300
 [<0000016c70f67cfe>] device_list_add.constprop.0+0x1be/0xc60
 [<0000016c70f688da>] btrfs_scan_one_device+0x13a/0x2f0
 [<0000016c70ed9cb0>] btrfs_control_ioctl+0x110/0x1e0
 [<0000016c70b97d0a>] __s390x_sys_ioctl+0xfa/0x130
 [<0000016c718004d2>] __do_syscall+0x172/0x750
 [<0000016c718155d2>] system_call+0x72/0x90

[CAUSE]
Btrfs device scan will call is_same_device() with device_list_mutex
held. But is_same_device() will call kern_path() which will do path
resolution and lock the inode.

So device scan has the following lock sequence:

 mutex_lock(device_list_mutex) from device_list_add()
   |
   v
 inode_lock_shared() from lookup_slow() during kern_path().

Meanwhile another thread is fsyncing, which has the following
lock sequence:

 inode_lock() from btrfs_inode_lock() inside btrfs_direct_write()
   |
   v
 mutex_lock(tree_log_mutex() from btrfs_sync_log(), which is further
 triggered from
 iomap_dio_complete()->generic_write_sync()->btrfs_sync_file().
   |
   v
 mutex_lock(device_list_mutex) from write_all_supers() inside
 btrfs_sync_log().

So the device scan has a reversed lock sequence, compared to the fsync
one, this means we can have the following deadlock:

          Device scan                   |           Fsync
----------------------------------------+--------------------------------
 device_list_mutex locked               |
                                        |   inode locked
					|   try to lock device_list_mutex
 try to lock inode                      |

[FIX]
Instead of a full path lookup, use dev_t to determine if two device
paths are pointing to the same block device.

Inside kernel dev_t is going to uniquely determine a block device, and
the device path lookup is already done by lookup_bdev(), which is done
without device_list_mutex held, thus no reversed locking sequence.

Reported-by: Christian Borntraeger <[email protected]>
Link: https://lore.kernel.org/linux-btrfs/[email protected]/
Fixes: 2e8b6bc0ab41 ("btrfs: avoid unnecessary device path update for the same device")
Signed-off-by: Qu Wenruo <[email protected]>
---
Changelog:
- Update the [CAUSE] to make it more clear why we have inode locked
---
 fs/btrfs/volumes.c | 37 +------------------------------------
 1 file changed, 1 insertion(+), 36 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 2d132c827913..3d1063bc73f8 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -740,41 +740,6 @@ const u8 *btrfs_sb_fsid_ptr(const struct btrfs_super_block *sb)
 	return has_metadata_uuid ? sb->metadata_uuid : sb->fsid;
 }
 
-static bool is_same_device(struct btrfs_device *device, const char *new_path)
-{
-	struct path old = { .mnt = NULL, .dentry = NULL };
-	struct path new = { .mnt = NULL, .dentry = NULL };
-	char AUTO_KFREE(old_path);
-	bool is_same = false;
-	int ret;
-
-	if (!device->name)
-		goto out;
-
-	old_path = kzalloc(PATH_MAX, GFP_NOFS);
-	if (!old_path)
-		goto out;
-
-	rcu_read_lock();
-	ret = strscpy(old_path, rcu_dereference(device->name), PATH_MAX);
-	rcu_read_unlock();
-	if (ret < 0)
-		goto out;
-
-	ret = kern_path(old_path, LOOKUP_FOLLOW, &old);
-	if (ret)
-		goto out;
-	ret = kern_path(new_path, LOOKUP_FOLLOW, &new);
-	if (ret)
-		goto out;
-	if (path_equal(&old, &new))
-		is_same = true;
-out:
-	path_put(&old);
-	path_put(&new);
-	return is_same;
-}
-
 /*
  * Add new device to list of registered devices
  *
@@ -895,7 +860,7 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 				MAJOR(path_devt), MINOR(path_devt),
 				current->comm, task_pid_nr(current));
 
-	} else if (!device->name || !is_same_device(device, path)) {
+	} else if (!device->name || device->devt != path_devt) {
 		const char *old_name;
 
 		/*
-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.