On 14.07.20 г. 20:21 ч., Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@xxxxxxxx>
>
> There is a lockdep report when running xfstests btrfs/161 (seed + sprout
> devices) related to chunk_mutex:
>
For whatever reason this submission seems garbled. In any case my
proposal was to replace the chunk mutex with FS_EXCL_OP in
btrfs_read_chunk_tree which you haven't done. You simply remove the
chunk mutex.
> ======================================================
> WARNING: possible circular locking dependency detected
> 5.8.0-rc4-default #504 Not tainted
> ------------------------------------------------------
> mount/454 is trying to acquire lock:
> ffff8881133058e8 (&fs_devs->device_list_mutex){+.+.}-{3:3}, at: clone_fs_devices+0x4d/0x340
>
> but task is already holding lock:
> ffff888112010988 (&fs_info->chunk_mutex){+.+.}-{3:3}, at: btrfs_read_chunk_tree+0xcc/0x600
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (&fs_info->chunk_mutex){+.+.}-{3:3}:
> __mutex_lock+0x139/0x13e0
> btrfs_init_new_device+0x1ed1/0x3c50
> btrfs_ioctl+0x19b4/0x8130
> ksys_ioctl+0xa9/0xf0
> __x64_sys_ioctl+0x6f/0xb0
> do_syscall_64+0x50/0xd0
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> -> #0 (&fs_devs->device_list_mutex){+.+.}-{3:3}:
> __lock_acquire+0x2df6/0x4da0
> lock_acquire+0x176/0x820
> __mutex_lock+0x139/0x13e0
> clone_fs_devices+0x4d/0x340
> read_one_dev+0xa5c/0x13e0
> btrfs_read_chunk_tree+0x480/0x600
> open_ctree+0x244b/0x450d
> btrfs_mount_root.cold.80+0x10/0x129
> legacy_get_tree+0xff/0x1f0
> vfs_get_tree+0x83/0x250
> fc_mount+0xf/0x90
> vfs_kern_mount.part.47+0x5c/0x80
> btrfs_mount+0x215/0xbcf
> legacy_get_tree+0xff/0x1f0
> vfs_get_tree+0x83/0x250
> do_mount+0x106d/0x16f0
> __x64_sys_mount+0x162/0x1b0
> do_syscall_64+0x50/0xd0
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> other info that might help us debug this:
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(&fs_info->chunk_mutex);
> lock(&fs_devs->device_list_mutex);
> lock(&fs_info->chunk_mutex);
> lock(&fs_devs->device_list_mutex);
>
> *** DEADLOCK ***
>
> 3 locks held by mount/454:
> #0: ffff8881119340e8 (&type->s_umount_key#41/1){+.+.}-{3:3}, at: alloc_super.isra.21+0x183/0x990
> #1: ffffffff83b260d0 (uuid_mutex){+.+.}-{3:3}, at: btrfs_read_chunk_tree+0xb6/0x600
> #2: ffff888112010988 (&fs_info->chunk_mutex){+.+.}-{3:3}, at: btrfs_read_chunk_tree+0xcc/0x600
>
> stack backtrace:
> CPU: 3 PID: 454 Comm: mount Not tainted 5.8.0-rc4-default #504
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
> Call Trace:
> dump_stack+0x9d/0xe0
> check_noncircular+0x351/0x410
> ? print_circular_bug.isra.41+0x360/0x360
> ? mark_lock+0x12d/0x14d0
> ? rcu_read_unlock+0x40/0x40
> ? sched_clock+0x5/0x10
> ? sched_clock_cpu+0x18/0x170
> ? sched_clock_cpu+0x18/0x170
> __lock_acquire+0x2df6/0x4da0
> ? lockdep_hardirqs_on_prepare+0x540/0x540
> ? _raw_spin_unlock_irqrestore+0x3e/0x50
> ? trace_hardirqs_on+0x20/0x170
> ? stack_depot_save+0x260/0x470
> lock_acquire+0x176/0x820
> ? clone_fs_devices+0x4d/0x340
> ? btrfs_mount_root.cold.80+0x10/0x129
> ? rcu_read_unlock+0x40/0x40
> ? vfs_kern_mount.part.47+0x5c/0x80
> ? btrfs_mount+0x215/0xbcf
> ? legacy_get_tree+0xff/0x1f0
> ? vfs_get_tree+0x83/0x250
> ? do_mount+0x106d/0x16f0
> ? __x64_sys_mount+0x162/0x1b0
> ? do_syscall_64+0x50/0xd0
> __mutex_lock+0x139/0x13e0
> ? clone_fs_devices+0x4d/0x340
> ? clone_fs_devices+0x4d/0x340
> ? mark_held_locks+0xb7/0x120
> ? mutex_lock_io_nested+0x12a0/0x12a0
> ? rcu_read_lock_sched_held+0xa1/0xd0
> ? lockdep_init_map_waits+0x29f/0x810
> ? lockdep_init_map_waits+0x29f/0x810
> ? debug_mutex_init+0x31/0x60
> ? clone_fs_devices+0x4d/0x340
> clone_fs_devices+0x4d/0x340
> ? read_extent_buffer+0x15b/0x2a0
> read_one_dev+0xa5c/0x13e0
> ? split_leaf+0xef0/0xef0
> ? read_one_chunk+0xb20/0xb20
> ? btrfs_get_token_32+0x730/0x730
> ? memcpy+0x38/0x60
> ? read_extent_buffer+0x15b/0x2a0
> btrfs_read_chunk_tree+0x480/0x600
> ? btrfs_check_rw_degradable+0x340/0x340
> ? btrfs_root_node+0x2d/0x240
> ? memcpy+0x38/0x60
> ? read_extent_buffer+0x15b/0x2a0
> ? btrfs_root_node+0x2d/0x240
> open_ctree+0x244b/0x450d
> ? close_ctree+0x61c/0x61c
> ? sget+0x328/0x400
> btrfs_mount_root.cold.80+0x10/0x129
> ? parse_rescue_options+0x260/0x260
> ? rcu_read_lock_sched_held+0xa1/0xd0
> ? rcu_read_lock_bh_held+0xb0/0xb0
> ? kfree+0x2e2/0x330
> ? parse_rescue_options+0x260/0x260
> legacy_get_tree+0xff/0x1f0
> vfs_get_tree+0x83/0x250
> fc_mount+0xf/0x90
> vfs_kern_mount.part.47+0x5c/0x80
> btrfs_mount+0x215/0xbcf
> ? check_object+0xb3/0x2c0
> ? btrfs_get_subvol_name_from_objectid+0x7f0/0x7f0
> ? ___slab_alloc+0x4e5/0x570
> ? vfs_parse_fs_string+0xc0/0x100
> ? rcu_read_lock_sched_held+0xa1/0xd0
> ? rcu_read_lock_bh_held+0xb0/0xb0
> ? kfree+0x2e2/0x330
> ? btrfs_get_subvol_name_from_objectid+0x7f0/0x7f0
> ? legacy_get_tree+0xff/0x1f0
> legacy_get_tree+0xff/0x1f0
> vfs_get_tree+0x83/0x250
> do_mount+0x106d/0x16f0
> ? lock_downgrade+0x720/0x720
> ? copy_mount_string+0x20/0x20
> ? _copy_from_user+0xbe/0x100
> ? memdup_user+0x47/0x70
> __x64_sys_mount+0x162/0x1b0
> do_syscall_64+0x50/0xd0
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7f82dd1583ca
> Code: Bad RIP value.
> RSP: 002b:00007ffcf6935fc8 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
> RAX: ffffffffffffffda RBX: 0000562f10678500 RCX: 00007f82dd1583ca
> RDX: 0000562f1067f8e0 RSI: 0000562f1067b3f0 RDI: 0000562f106786e0
> RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> R10: 00000000c0ed0000 R11: 0000000000000202 R12: 0000562f106786e0
> R13: 0000562f1067f8e0 R14: 0000000000000000 R15: 00007f82dd6798a4
>
> In volumes.c there are comments stating that chunk_mutex is used to
> protect add/remove chunks, trim or add/remove devices. Since
> btrfs_read_chunk_tree is only called on mount, and trim and chunk alloc
> cannot happen at mount time, it's safe to remove this lock/unlock.
>
> Also, btrfs_ioctl_{add|rm}_dev calls set BTRFS_FS_EXCL_OP, which should
> be safe.
>
> Dropping the mutex lock/unlock solves the lockdep warning.
>
> Suggested-by: Nokolay Borisov <nborisov@xxxxxxxx>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@xxxxxxxx>
> ---
> fs/btrfs/volumes.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index c7a3d4d730a3..94cbdadd9d26 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -7033,7 +7033,6 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
> * otherwise we don't need it.
> */
> mutex_lock(&uuid_mutex);
> - mutex_lock(&fs_info->chunk_mutex);
>
> /*
> * Read all device items, and then all the chunk items. All
> @@ -7100,7 +7099,6 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
> }
> ret = 0;
> error:
> - mutex_unlock(&fs_info->chunk_mutex);
> mutex_unlock(&uuid_mutex);
>
> btrfs_free_path(path);
>
