]> www.infradead.org Git - users/dwmw2/linux.git/commitdiff
bcachefs: Fix snapshot_create_lock lock ordering
authorKent Overstreet <kent.overstreet@linux.dev>
Sat, 8 Jun 2024 01:02:06 +0000 (21:02 -0400)
committerKent Overstreet <kent.overstreet@linux.dev>
Mon, 10 Jun 2024 17:17:16 +0000 (13:17 -0400)
======================================================
WARNING: possible circular locking dependency detected
6.10.0-rc2-ktest-00018-gebd1d148b278 #144 Not tainted
------------------------------------------------------
fio/1345 is trying to acquire lock:
ffff88813e200ab8 (&c->snapshot_create_lock){++++}-{3:3}, at: bch2_truncate+0x76/0xf0

but task is already holding lock:
ffff888105a1fa38 (&sb->s_type->i_mutex_key#13){+.+.}-{3:3}, at: do_truncate+0x7b/0xc0

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #2 (&sb->s_type->i_mutex_key#13){+.+.}-{3:3}:
       down_write+0x3d/0xd0
       bch2_write_iter+0x1c0/0x10f0
       vfs_write+0x24a/0x560
       __x64_sys_pwrite64+0x77/0xb0
       x64_sys_call+0x17e5/0x1ab0
       do_syscall_64+0x68/0x130
       entry_SYSCALL_64_after_hwframe+0x4b/0x53

-> #1 (sb_writers#10){.+.+}-{0:0}:
       mnt_want_write+0x4a/0x1d0
       filename_create+0x69/0x1a0
       user_path_create+0x38/0x50
       bch2_fs_file_ioctl+0x315/0xbf0
       __x64_sys_ioctl+0x297/0xaf0
       x64_sys_call+0x10cb/0x1ab0
       do_syscall_64+0x68/0x130
       entry_SYSCALL_64_after_hwframe+0x4b/0x53

-> #0 (&c->snapshot_create_lock){++++}-{3:3}:
       __lock_acquire+0x1445/0x25b0
       lock_acquire+0xbd/0x2b0
       down_read+0x40/0x180
       bch2_truncate+0x76/0xf0
       bchfs_truncate+0x240/0x3f0
       bch2_setattr+0x7b/0xb0
       notify_change+0x322/0x4b0
       do_truncate+0x8b/0xc0
       do_ftruncate+0x110/0x270
       __x64_sys_ftruncate+0x43/0x80
       x64_sys_call+0x1373/0x1ab0
       do_syscall_64+0x68/0x130
       entry_SYSCALL_64_after_hwframe+0x4b/0x53

other info that might help us debug this:

Chain exists of:
  &c->snapshot_create_lock --> sb_writers#10 --> &sb->s_type->i_mutex_key#13

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&sb->s_type->i_mutex_key#13);
                               lock(sb_writers#10);
                               lock(&sb->s_type->i_mutex_key#13);
  rlock(&c->snapshot_create_lock);

 *** DEADLOCK ***

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
fs/bcachefs/fs-ioctl.c

index 205a323ffc6ddd962499a3e0082d686d13c1a103..3551a737181b2ef94ce344fb333d886df73e9a46 100644 (file)
@@ -308,8 +308,8 @@ static int bch2_ioc_goingdown(struct bch_fs *c, u32 __user *arg)
        return ret;
 }
 
-static long __bch2_ioctl_subvolume_create(struct bch_fs *c, struct file *filp,
-                                         struct bch_ioctl_subvolume arg)
+static long bch2_ioctl_subvolume_create(struct bch_fs *c, struct file *filp,
+                                       struct bch_ioctl_subvolume arg)
 {
        struct inode *dir;
        struct bch_inode_info *inode;
@@ -406,9 +406,12 @@ retry:
            !arg.src_ptr)
                snapshot_src.subvol = inode_inum(to_bch_ei(dir)).subvol;
 
+       down_write(&c->snapshot_create_lock);
        inode = __bch2_create(file_mnt_idmap(filp), to_bch_ei(dir),
                              dst_dentry, arg.mode|S_IFDIR,
                              0, snapshot_src, create_flags);
+       up_write(&c->snapshot_create_lock);
+
        error = PTR_ERR_OR_ZERO(inode);
        if (error)
                goto err3;
@@ -429,16 +432,6 @@ err1:
        return error;
 }
 
-static long bch2_ioctl_subvolume_create(struct bch_fs *c, struct file *filp,
-                                       struct bch_ioctl_subvolume arg)
-{
-       down_write(&c->snapshot_create_lock);
-       long ret = __bch2_ioctl_subvolume_create(c, filp, arg);
-       up_write(&c->snapshot_create_lock);
-
-       return ret;
-}
-
 static long bch2_ioctl_subvolume_destroy(struct bch_fs *c, struct file *filp,
                                struct bch_ioctl_subvolume arg)
 {