| Message ID | 44c756daa28122f0f51f52d154c1232a09e66872.1605284383.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,01/42] btrfs: allow error injection for btrfs_search_slot and btrfs_cow_block | expand |
On 13.11.20 г. 18:22 ч., Josef Bacik wrote: > While testing the error paths of relocation I hit the following lockdep > splat > > ====================================================== > WARNING: possible circular locking dependency detected > 5.10.0-rc2-btrfs-next-71 #1 Not tainted > ------------------------------------------------------ > find/324157 is trying to acquire lock: > ffff8ebc48d293a0 (btrfs-tree-01#2/3){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > > but task is already holding lock: > ffff8eb9932c5088 (btrfs-tree-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > > which lock already depends on the new lock. > > the existing dependency chain (in reverse order) is: > > -> #1 (btrfs-tree-00){++++}-{3:3}: > lock_acquire+0xd8/0x490 > down_write_nested+0x44/0x120 > __btrfs_tree_lock+0x27/0x120 [btrfs] > btrfs_search_slot+0x2a3/0xc50 [btrfs] > btrfs_insert_empty_items+0x58/0xa0 [btrfs] > insert_with_overflow+0x44/0x110 [btrfs] > btrfs_insert_xattr_item+0xb8/0x1d0 [btrfs] > btrfs_setxattr+0xd6/0x4c0 [btrfs] > btrfs_setxattr_trans+0x68/0x100 [btrfs] > __vfs_setxattr+0x66/0x80 > __vfs_setxattr_noperm+0x70/0x200 > vfs_setxattr+0x6b/0x120 > setxattr+0x125/0x240 > path_setxattr+0xba/0xd0 > __x64_sys_setxattr+0x27/0x30 > do_syscall_64+0x33/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > -> #0 (btrfs-tree-01#2/3){++++}-{3:3}: > check_prev_add+0x91/0xc60 > __lock_acquire+0x1689/0x3130 > lock_acquire+0xd8/0x490 > down_read_nested+0x45/0x220 > __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > btrfs_next_old_leaf+0x27d/0x580 [btrfs] > btrfs_real_readdir+0x1e3/0x4b0 [btrfs] > iterate_dir+0x170/0x1c0 > __x64_sys_getdents64+0x83/0x140 > do_syscall_64+0x33/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > other info that might help us debug this: > > Possible unsafe locking scenario: > > CPU0 CPU1 > ---- ---- > lock(btrfs-tree-00); > lock(btrfs-tree-01#2/3); > lock(btrfs-tree-00); > lock(btrfs-tree-01#2/3); > > *** DEADLOCK *** > > 5 locks held by find/324157: > #0: ffff8ebc502c6e00 (&f->f_pos_lock){+.+.}-{3:3}, at: __fdget_pos+0x4d/0x60 > #1: ffff8eb97f689980 (&type->i_mutex_dir_key#10){++++}-{3:3}, at: iterate_dir+0x52/0x1c0 > #2: ffff8ebaec00ca58 (btrfs-tree-02#2){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > #3: ffff8eb98f986f78 (btrfs-tree-01#2){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > #4: ffff8eb9932c5088 (btrfs-tree-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > > stack backtrace: > CPU: 2 PID: 324157 Comm: find Not tainted 5.10.0-rc2-btrfs-next-71 #1 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014 > Call Trace: > dump_stack+0x8d/0xb5 > check_noncircular+0xff/0x110 > ? mark_lock.part.0+0x468/0xe90 > check_prev_add+0x91/0xc60 > __lock_acquire+0x1689/0x3130 > ? kvm_clock_read+0x14/0x30 > ? kvm_sched_clock_read+0x5/0x10 > lock_acquire+0xd8/0x490 > ? __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > down_read_nested+0x45/0x220 > ? __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > btrfs_next_old_leaf+0x27d/0x580 [btrfs] > btrfs_real_readdir+0x1e3/0x4b0 [btrfs] > iterate_dir+0x170/0x1c0 > __x64_sys_getdents64+0x83/0x140 > ? filldir+0x1d0/0x1d0 > do_syscall_64+0x33/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > This is thankfully straightforward to fix, simply release the path > before we setup the reloc_ctl. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> So you are changing btrfs_recover_balance yet nowhere in the stack traces provided does this functino persist, instead the problem seems to be due to the way btrfs_real_readdir does its locking. I'm confused. > --- > fs/btrfs/volumes.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index bb1aa96e1233..ece8bb62fcc1 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -4283,6 +4283,8 @@ int btrfs_recover_balance(struct btrfs_fs_info *fs_info) > btrfs_warn(fs_info, > "balance: cannot set exclusive op status, resume manually"); > > + btrfs_release_path(path); > + > mutex_lock(&fs_info->balance_mutex); > BUG_ON(fs_info->balance_ctl); > spin_lock(&fs_info->balance_lock); >
On Fri, Nov 13, 2020 at 4:25 PM Josef Bacik <josef@toxicpanda.com> wrote: > > While testing the error paths of relocation I hit the following lockdep > splat The lockdep splat has a kernel named exactly like mine: *-btrfs-next-71 :) > > ====================================================== > WARNING: possible circular locking dependency detected > 5.10.0-rc2-btrfs-next-71 #1 Not tainted > ------------------------------------------------------ > find/324157 is trying to acquire lock: > ffff8ebc48d293a0 (btrfs-tree-01#2/3){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > > but task is already holding lock: > ffff8eb9932c5088 (btrfs-tree-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > > which lock already depends on the new lock. > > the existing dependency chain (in reverse order) is: > > -> #1 (btrfs-tree-00){++++}-{3:3}: > lock_acquire+0xd8/0x490 > down_write_nested+0x44/0x120 > __btrfs_tree_lock+0x27/0x120 [btrfs] > btrfs_search_slot+0x2a3/0xc50 [btrfs] > btrfs_insert_empty_items+0x58/0xa0 [btrfs] > insert_with_overflow+0x44/0x110 [btrfs] > btrfs_insert_xattr_item+0xb8/0x1d0 [btrfs] > btrfs_setxattr+0xd6/0x4c0 [btrfs] > btrfs_setxattr_trans+0x68/0x100 [btrfs] > __vfs_setxattr+0x66/0x80 > __vfs_setxattr_noperm+0x70/0x200 > vfs_setxattr+0x6b/0x120 > setxattr+0x125/0x240 > path_setxattr+0xba/0xd0 > __x64_sys_setxattr+0x27/0x30 > do_syscall_64+0x33/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > -> #0 (btrfs-tree-01#2/3){++++}-{3:3}: > check_prev_add+0x91/0xc60 > __lock_acquire+0x1689/0x3130 > lock_acquire+0xd8/0x490 > down_read_nested+0x45/0x220 > __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > btrfs_next_old_leaf+0x27d/0x580 [btrfs] > btrfs_real_readdir+0x1e3/0x4b0 [btrfs] > iterate_dir+0x170/0x1c0 > __x64_sys_getdents64+0x83/0x140 > do_syscall_64+0x33/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > other info that might help us debug this: > > Possible unsafe locking scenario: > > CPU0 CPU1 > ---- ---- > lock(btrfs-tree-00); > lock(btrfs-tree-01#2/3); > lock(btrfs-tree-00); > lock(btrfs-tree-01#2/3); > > *** DEADLOCK *** > > 5 locks held by find/324157: > #0: ffff8ebc502c6e00 (&f->f_pos_lock){+.+.}-{3:3}, at: __fdget_pos+0x4d/0x60 > #1: ffff8eb97f689980 (&type->i_mutex_dir_key#10){++++}-{3:3}, at: iterate_dir+0x52/0x1c0 > #2: ffff8ebaec00ca58 (btrfs-tree-02#2){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > #3: ffff8eb98f986f78 (btrfs-tree-01#2){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > #4: ffff8eb9932c5088 (btrfs-tree-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > > stack backtrace: > CPU: 2 PID: 324157 Comm: find Not tainted 5.10.0-rc2-btrfs-next-71 #1 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014 > Call Trace: > dump_stack+0x8d/0xb5 > check_noncircular+0xff/0x110 > ? mark_lock.part.0+0x468/0xe90 > check_prev_add+0x91/0xc60 > __lock_acquire+0x1689/0x3130 > ? kvm_clock_read+0x14/0x30 > ? kvm_sched_clock_read+0x5/0x10 > lock_acquire+0xd8/0x490 > ? __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > down_read_nested+0x45/0x220 > ? __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] > btrfs_next_old_leaf+0x27d/0x580 [btrfs] > btrfs_real_readdir+0x1e3/0x4b0 [btrfs] > iterate_dir+0x170/0x1c0 > __x64_sys_getdents64+0x83/0x140 > ? filldir+0x1d0/0x1d0 > do_syscall_64+0x33/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > This is thankfully straightforward to fix, simply release the path > before we setup the reloc_ctl. Ok, so that splat is exactly what I reported not long ago and is already fixed by: https://lore.kernel.org/linux-btrfs/36b861f262858990f84eda72da6bb2e6762c41b7.1604697895.git.josef@toxicpanda.com/#r Which is the splat that happened in one of my test boxes. So, have you pasted the wrong splat? Did it happen with any existing test case from fstests, if so, which? That one I reported was with btrfs/187 (worth mentioning in the changelog). Thanks. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/volumes.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index bb1aa96e1233..ece8bb62fcc1 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -4283,6 +4283,8 @@ int btrfs_recover_balance(struct btrfs_fs_info *fs_info) > btrfs_warn(fs_info, > "balance: cannot set exclusive op status, resume manually"); > > + btrfs_release_path(path); > + > mutex_lock(&fs_info->balance_mutex); > BUG_ON(fs_info->balance_ctl); > spin_lock(&fs_info->balance_lock); > -- > 2.26.2 >
On 11/24/20 11:56 AM, Filipe Manana wrote: > On Fri, Nov 13, 2020 at 4:25 PM Josef Bacik <josef@toxicpanda.com> wrote: >> >> While testing the error paths of relocation I hit the following lockdep >> splat > > The lockdep splat has a kernel named exactly like mine: *-btrfs-next-71 :) > >> >> ====================================================== >> WARNING: possible circular locking dependency detected >> 5.10.0-rc2-btrfs-next-71 #1 Not tainted >> ------------------------------------------------------ >> find/324157 is trying to acquire lock: >> ffff8ebc48d293a0 (btrfs-tree-01#2/3){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] >> >> but task is already holding lock: >> ffff8eb9932c5088 (btrfs-tree-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] >> >> which lock already depends on the new lock. >> >> the existing dependency chain (in reverse order) is: >> >> -> #1 (btrfs-tree-00){++++}-{3:3}: >> lock_acquire+0xd8/0x490 >> down_write_nested+0x44/0x120 >> __btrfs_tree_lock+0x27/0x120 [btrfs] >> btrfs_search_slot+0x2a3/0xc50 [btrfs] >> btrfs_insert_empty_items+0x58/0xa0 [btrfs] >> insert_with_overflow+0x44/0x110 [btrfs] >> btrfs_insert_xattr_item+0xb8/0x1d0 [btrfs] >> btrfs_setxattr+0xd6/0x4c0 [btrfs] >> btrfs_setxattr_trans+0x68/0x100 [btrfs] >> __vfs_setxattr+0x66/0x80 >> __vfs_setxattr_noperm+0x70/0x200 >> vfs_setxattr+0x6b/0x120 >> setxattr+0x125/0x240 >> path_setxattr+0xba/0xd0 >> __x64_sys_setxattr+0x27/0x30 >> do_syscall_64+0x33/0x80 >> entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> >> -> #0 (btrfs-tree-01#2/3){++++}-{3:3}: >> check_prev_add+0x91/0xc60 >> __lock_acquire+0x1689/0x3130 >> lock_acquire+0xd8/0x490 >> down_read_nested+0x45/0x220 >> __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] >> btrfs_next_old_leaf+0x27d/0x580 [btrfs] >> btrfs_real_readdir+0x1e3/0x4b0 [btrfs] >> iterate_dir+0x170/0x1c0 >> __x64_sys_getdents64+0x83/0x140 >> do_syscall_64+0x33/0x80 >> entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> >> other info that might help us debug this: >> >> Possible unsafe locking scenario: >> >> CPU0 CPU1 >> ---- ---- >> lock(btrfs-tree-00); >> lock(btrfs-tree-01#2/3); >> lock(btrfs-tree-00); >> lock(btrfs-tree-01#2/3); >> >> *** DEADLOCK *** >> >> 5 locks held by find/324157: >> #0: ffff8ebc502c6e00 (&f->f_pos_lock){+.+.}-{3:3}, at: __fdget_pos+0x4d/0x60 >> #1: ffff8eb97f689980 (&type->i_mutex_dir_key#10){++++}-{3:3}, at: iterate_dir+0x52/0x1c0 >> #2: ffff8ebaec00ca58 (btrfs-tree-02#2){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] >> #3: ffff8eb98f986f78 (btrfs-tree-01#2){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] >> #4: ffff8eb9932c5088 (btrfs-tree-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] >> >> stack backtrace: >> CPU: 2 PID: 324157 Comm: find Not tainted 5.10.0-rc2-btrfs-next-71 #1 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014 >> Call Trace: >> dump_stack+0x8d/0xb5 >> check_noncircular+0xff/0x110 >> ? mark_lock.part.0+0x468/0xe90 >> check_prev_add+0x91/0xc60 >> __lock_acquire+0x1689/0x3130 >> ? kvm_clock_read+0x14/0x30 >> ? kvm_sched_clock_read+0x5/0x10 >> lock_acquire+0xd8/0x490 >> ? __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] >> down_read_nested+0x45/0x220 >> ? __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] >> __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] >> btrfs_next_old_leaf+0x27d/0x580 [btrfs] >> btrfs_real_readdir+0x1e3/0x4b0 [btrfs] >> iterate_dir+0x170/0x1c0 >> __x64_sys_getdents64+0x83/0x140 >> ? filldir+0x1d0/0x1d0 >> do_syscall_64+0x33/0x80 >> entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> >> This is thankfully straightforward to fix, simply release the path >> before we setup the reloc_ctl. > > Ok, so that splat is exactly what I reported not long ago and is > already fixed by: > > https://lore.kernel.org/linux-btrfs/36b861f262858990f84eda72da6bb2e6762c41b7.1604697895.git.josef@toxicpanda.com/#r > > Which is the splat that happened in one of my test boxes. > > So, have you pasted the wrong splat? > Did it happen with any existing test case from fstests, if so, which? > That one I reported was with btrfs/187 (worth mentioning in the > changelog). Lol I pasted the wrong splat, I forgot to scp it from the vm that had the problem, so I just pulled the last one I had fixed. It didn't happen with an xfstests testcase, it happened while doing error injection testing. I'll try and reproduce so I get the real splat, my bad. Thanks, Josef
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index bb1aa96e1233..ece8bb62fcc1 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -4283,6 +4283,8 @@ int btrfs_recover_balance(struct btrfs_fs_info *fs_info) btrfs_warn(fs_info, "balance: cannot set exclusive op status, resume manually"); + btrfs_release_path(path); + mutex_lock(&fs_info->balance_mutex); BUG_ON(fs_info->balance_ctl); spin_lock(&fs_info->balance_lock);
While testing the error paths of relocation I hit the following lockdep splat ====================================================== WARNING: possible circular locking dependency detected 5.10.0-rc2-btrfs-next-71 #1 Not tainted ------------------------------------------------------ find/324157 is trying to acquire lock: ffff8ebc48d293a0 (btrfs-tree-01#2/3){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] but task is already holding lock: ffff8eb9932c5088 (btrfs-tree-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (btrfs-tree-00){++++}-{3:3}: lock_acquire+0xd8/0x490 down_write_nested+0x44/0x120 __btrfs_tree_lock+0x27/0x120 [btrfs] btrfs_search_slot+0x2a3/0xc50 [btrfs] btrfs_insert_empty_items+0x58/0xa0 [btrfs] insert_with_overflow+0x44/0x110 [btrfs] btrfs_insert_xattr_item+0xb8/0x1d0 [btrfs] btrfs_setxattr+0xd6/0x4c0 [btrfs] btrfs_setxattr_trans+0x68/0x100 [btrfs] __vfs_setxattr+0x66/0x80 __vfs_setxattr_noperm+0x70/0x200 vfs_setxattr+0x6b/0x120 setxattr+0x125/0x240 path_setxattr+0xba/0xd0 __x64_sys_setxattr+0x27/0x30 do_syscall_64+0x33/0x80 entry_SYSCALL_64_after_hwframe+0x44/0xa9 -> #0 (btrfs-tree-01#2/3){++++}-{3:3}: check_prev_add+0x91/0xc60 __lock_acquire+0x1689/0x3130 lock_acquire+0xd8/0x490 down_read_nested+0x45/0x220 __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] btrfs_next_old_leaf+0x27d/0x580 [btrfs] btrfs_real_readdir+0x1e3/0x4b0 [btrfs] iterate_dir+0x170/0x1c0 __x64_sys_getdents64+0x83/0x140 do_syscall_64+0x33/0x80 entry_SYSCALL_64_after_hwframe+0x44/0xa9 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(btrfs-tree-00); lock(btrfs-tree-01#2/3); lock(btrfs-tree-00); lock(btrfs-tree-01#2/3); *** DEADLOCK *** 5 locks held by find/324157: #0: ffff8ebc502c6e00 (&f->f_pos_lock){+.+.}-{3:3}, at: __fdget_pos+0x4d/0x60 #1: ffff8eb97f689980 (&type->i_mutex_dir_key#10){++++}-{3:3}, at: iterate_dir+0x52/0x1c0 #2: ffff8ebaec00ca58 (btrfs-tree-02#2){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] #3: ffff8eb98f986f78 (btrfs-tree-01#2){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] #4: ffff8eb9932c5088 (btrfs-tree-00){++++}-{3:3}, at: __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] stack backtrace: CPU: 2 PID: 324157 Comm: find Not tainted 5.10.0-rc2-btrfs-next-71 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014 Call Trace: dump_stack+0x8d/0xb5 check_noncircular+0xff/0x110 ? mark_lock.part.0+0x468/0xe90 check_prev_add+0x91/0xc60 __lock_acquire+0x1689/0x3130 ? kvm_clock_read+0x14/0x30 ? kvm_sched_clock_read+0x5/0x10 lock_acquire+0xd8/0x490 ? __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] down_read_nested+0x45/0x220 ? __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] __btrfs_tree_read_lock+0x32/0x1a0 [btrfs] btrfs_next_old_leaf+0x27d/0x580 [btrfs] btrfs_real_readdir+0x1e3/0x4b0 [btrfs] iterate_dir+0x170/0x1c0 __x64_sys_getdents64+0x83/0x140 ? filldir+0x1d0/0x1d0 do_syscall_64+0x33/0x80 entry_SYSCALL_64_after_hwframe+0x44/0xa9 This is thankfully straightforward to fix, simply release the path before we setup the reloc_ctl. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/volumes.c | 2 ++ 1 file changed, 2 insertions(+)