* [PATCH] btrfs: fix a possible umount deadlock
@ 2016-09-09 8:31 Anand Jain
2016-09-09 12:53 ` David Sterba
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Anand Jain @ 2016-09-09 8:31 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, clm, idryomov
btrfs_show_devname() is using the device_list_mutex, sometimes
a call to blkdev_put() leads vfs calling into this func. So
call blkdev_put() outside of device_list_mutex, as of now.
[ 983.284212] ======================================================
[ 983.290401] [ INFO: possible circular locking dependency detected ]
[ 983.296677] 4.8.0-rc5-ceph-00023-g1b39cec2 #1 Not tainted
[ 983.302081] -------------------------------------------------------
[ 983.308357] umount/21720 is trying to acquire lock:
[ 983.313243] (&bdev->bd_mutex){+.+.+.}, at: [<ffffffff9128ec51>] blkdev_put+0x31/0x150
[ 983.321264]
[ 983.321264] but task is already holding lock:
[ 983.327101] (&fs_devs->device_list_mutex){+.+...}, at: [<ffffffffc033d6f6>] __btrfs_close_devices+0x46/0x200 [btrfs]
[ 983.337839]
[ 983.337839] which lock already depends on the new lock.
[ 983.337839]
[ 983.346024]
[ 983.346024] the existing dependency chain (in reverse order) is:
[ 983.353512]
-> #4 (&fs_devs->device_list_mutex){+.+...}:
[ 983.359096] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.365143] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.371521] [<ffffffffc02d8116>] btrfs_show_devname+0x36/0x1f0 [btrfs]
[ 983.378710] [<ffffffff9129523e>] show_vfsmnt+0x4e/0x150
[ 983.384593] [<ffffffff9126ffc7>] m_show+0x17/0x20
[ 983.389957] [<ffffffff91276405>] seq_read+0x2b5/0x3b0
[ 983.395669] [<ffffffff9124c808>] __vfs_read+0x28/0x100
[ 983.401464] [<ffffffff9124eb3b>] vfs_read+0xab/0x150
[ 983.407080] [<ffffffff9124ec32>] SyS_read+0x52/0xb0
[ 983.412609] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 983.419617]
-> #3 (namespace_sem){++++++}:
[ 983.424024] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.430074] [<ffffffff918239e9>] down_write+0x49/0x80
[ 983.435785] [<ffffffff91272457>] lock_mount+0x67/0x1c0
[ 983.441582] [<ffffffff91272ab2>] do_add_mount+0x32/0xf0
[ 983.447458] [<ffffffff9127363a>] finish_automount+0x5a/0xc0
[ 983.453682] [<ffffffff91259513>] follow_managed+0x1b3/0x2a0
[ 983.459912] [<ffffffff9125b750>] lookup_fast+0x300/0x350
[ 983.465875] [<ffffffff9125d6e7>] path_openat+0x3a7/0xaa0
[ 983.471846] [<ffffffff9125ef75>] do_filp_open+0x85/0xe0
[ 983.477731] [<ffffffff9124c41c>] do_sys_open+0x14c/0x1f0
[ 983.483702] [<ffffffff9124c4de>] SyS_open+0x1e/0x20
[ 983.489240] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 983.496254]
-> #2 (&sb->s_type->i_mutex_key#3){+.+.+.}:
[ 983.501798] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.507855] [<ffffffff918239e9>] down_write+0x49/0x80
[ 983.513558] [<ffffffff91366237>] start_creating+0x87/0x100
[ 983.519703] [<ffffffff91366647>] debugfs_create_dir+0x17/0x100
[ 983.526195] [<ffffffff911df153>] bdi_register+0x93/0x210
[ 983.532165] [<ffffffff911df313>] bdi_register_owner+0x43/0x70
[ 983.538570] [<ffffffff914080fb>] device_add_disk+0x1fb/0x450
[ 983.544888] [<ffffffff91580226>] loop_add+0x1e6/0x290
[ 983.550596] [<ffffffff91fec358>] loop_init+0x10b/0x14f
[ 983.556394] [<ffffffff91002207>] do_one_initcall+0xa7/0x180
[ 983.562618] [<ffffffff91f932e0>] kernel_init_freeable+0x1cc/0x266
[ 983.569370] [<ffffffff918174be>] kernel_init+0xe/0x100
[ 983.575166] [<ffffffff9182620f>] ret_from_fork+0x1f/0x40
[ 983.581131]
-> #1 (loop_index_mutex){+.+.+.}:
[ 983.585801] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.591858] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.598256] [<ffffffff9157ed3f>] lo_open+0x1f/0x60
[ 983.603704] [<ffffffff9128eec3>] __blkdev_get+0x123/0x400
[ 983.609757] [<ffffffff9128f4ea>] blkdev_get+0x34a/0x350
[ 983.615639] [<ffffffff9128f554>] blkdev_open+0x64/0x80
[ 983.621428] [<ffffffff9124aff6>] do_dentry_open+0x1c6/0x2d0
[ 983.627651] [<ffffffff9124c029>] vfs_open+0x69/0x80
[ 983.633181] [<ffffffff9125db74>] path_openat+0x834/0xaa0
[ 983.639152] [<ffffffff9125ef75>] do_filp_open+0x85/0xe0
[ 983.645035] [<ffffffff9124c41c>] do_sys_open+0x14c/0x1f0
[ 983.650999] [<ffffffff9124c4de>] SyS_open+0x1e/0x20
[ 983.656535] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 983.663541]
-> #0 (&bdev->bd_mutex){+.+.+.}:
[ 983.668107] [<ffffffff910def43>] __lock_acquire+0x1003/0x17b0
[ 983.674510] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.680561] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.686967] [<ffffffff9128ec51>] blkdev_put+0x31/0x150
[ 983.692761] [<ffffffffc033481f>] btrfs_close_bdev+0x4f/0x60 [btrfs]
[ 983.699699] [<ffffffffc033d77b>] __btrfs_close_devices+0xcb/0x200 [btrfs]
[ 983.707178] [<ffffffffc033d8db>] btrfs_close_devices+0x2b/0xa0 [btrfs]
[ 983.714380] [<ffffffffc03081c5>] close_ctree+0x265/0x340 [btrfs]
[ 983.721061] [<ffffffffc02d7959>] btrfs_put_super+0x19/0x20 [btrfs]
[ 983.727908] [<ffffffff91250e2f>] generic_shutdown_super+0x6f/0x100
[ 983.734744] [<ffffffff91250f56>] kill_anon_super+0x16/0x30
[ 983.740888] [<ffffffffc02da97e>] btrfs_kill_super+0x1e/0x130 [btrfs]
[ 983.747909] [<ffffffff91250fe9>] deactivate_locked_super+0x49/0x80
[ 983.754745] [<ffffffff912515fd>] deactivate_super+0x5d/0x70
[ 983.760977] [<ffffffff91270a1c>] cleanup_mnt+0x5c/0x80
[ 983.766773] [<ffffffff91270a92>] __cleanup_mnt+0x12/0x20
[ 983.772738] [<ffffffff910aa2fe>] task_work_run+0x7e/0xc0
[ 983.778708] [<ffffffff91081b5a>] exit_to_usermode_loop+0x7e/0xb4
[ 983.785373] [<ffffffff910039eb>] syscall_return_slowpath+0xbb/0xd0
[ 983.792212] [<ffffffff9182605c>] entry_SYSCALL_64_fastpath+0xbf/0xc1
[ 983.799225]
[ 983.799225] other info that might help us debug this:
[ 983.799225]
[ 983.807291] Chain exists of:
&bdev->bd_mutex --> namespace_sem --> &fs_devs->device_list_mutex
[ 983.816521] Possible unsafe locking scenario:
[ 983.816521]
[ 983.822489] CPU0 CPU1
[ 983.827043] ---- ----
[ 983.831599] lock(&fs_devs->device_list_mutex);
[ 983.836289] lock(namespace_sem);
[ 983.842268] lock(&fs_devs->device_list_mutex);
[ 983.849478] lock(&bdev->bd_mutex);
[ 983.853127]
[ 983.853127] *** DEADLOCK ***
[ 983.853127]
[ 983.859113] 3 locks held by umount/21720:
[ 983.863145] #0: (&type->s_umount_key#35){++++..}, at: [<ffffffff912515f5>] deactivate_super+0x55/0x70
[ 983.872713] #1: (uuid_mutex){+.+.+.}, at: [<ffffffffc033d8d3>] btrfs_close_devices+0x23/0xa0 [btrfs]
[ 983.882206] #2: (&fs_devs->device_list_mutex){+.+...}, at: [<ffffffffc033d6f6>] __btrfs_close_devices+0x46/0x200 [btrfs]
[ 983.893422]
[ 983.893422] stack backtrace:
[ 983.897824] CPU: 6 PID: 21720 Comm: umount Not tainted 4.8.0-rc5-ceph-00023-g1b39cec2 #1
[ 983.905958] Hardware name: Supermicro SYS-5018R-WR/X10SRW-F, BIOS 1.0c 09/07/2015
[ 983.913492] 0000000000000000 ffff8c8a53c17a38 ffffffff91429521 ffffffff9260f4f0
[ 983.921018] ffffffff92642760 ffff8c8a53c17a88 ffffffff911b2b04 0000000000000050
[ 983.928542] ffffffff9237d620 ffff8c8a5294aee0 ffff8c8a5294aeb8 ffff8c8a5294aee0
[ 983.936072] Call Trace:
[ 983.938545] [<ffffffff91429521>] dump_stack+0x85/0xc4
[ 983.943715] [<ffffffff911b2b04>] print_circular_bug+0x1fb/0x20c
[ 983.949748] [<ffffffff910def43>] __lock_acquire+0x1003/0x17b0
[ 983.955613] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.961123] [<ffffffff9128ec51>] ? blkdev_put+0x31/0x150
[ 983.966550] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.972407] [<ffffffff9128ec51>] ? blkdev_put+0x31/0x150
[ 983.977832] [<ffffffff9128ec51>] blkdev_put+0x31/0x150
[ 983.983101] [<ffffffffc033481f>] btrfs_close_bdev+0x4f/0x60 [btrfs]
[ 983.989500] [<ffffffffc033d77b>] __btrfs_close_devices+0xcb/0x200 [btrfs]
[ 983.996415] [<ffffffffc033d8db>] btrfs_close_devices+0x2b/0xa0 [btrfs]
[ 984.003068] [<ffffffffc03081c5>] close_ctree+0x265/0x340 [btrfs]
[ 984.009189] [<ffffffff9126cc5e>] ? evict_inodes+0x15e/0x170
[ 984.014881] [<ffffffffc02d7959>] btrfs_put_super+0x19/0x20 [btrfs]
[ 984.021176] [<ffffffff91250e2f>] generic_shutdown_super+0x6f/0x100
[ 984.027476] [<ffffffff91250f56>] kill_anon_super+0x16/0x30
[ 984.033082] [<ffffffffc02da97e>] btrfs_kill_super+0x1e/0x130 [btrfs]
[ 984.039548] [<ffffffff91250fe9>] deactivate_locked_super+0x49/0x80
[ 984.045839] [<ffffffff912515fd>] deactivate_super+0x5d/0x70
[ 984.051525] [<ffffffff91270a1c>] cleanup_mnt+0x5c/0x80
[ 984.056774] [<ffffffff91270a92>] __cleanup_mnt+0x12/0x20
[ 984.062201] [<ffffffff910aa2fe>] task_work_run+0x7e/0xc0
[ 984.067625] [<ffffffff91081b5a>] exit_to_usermode_loop+0x7e/0xb4
[ 984.073747] [<ffffffff910039eb>] syscall_return_slowpath+0xbb/0xd0
[ 984.080038] [<ffffffff9182605c>] entry_SYSCALL_64_fastpath+0xbf/0xc1
Reported-by: Ilya Dryomov <idryomov@gmail.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/volumes.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index eb7557fec345..29d889ef23cd 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -877,8 +877,6 @@ static void btrfs_close_one_device(struct btrfs_device *device)
if (device->missing)
fs_devices->missing_devices--;
- btrfs_close_bdev(device);
-
new_device = btrfs_alloc_device(NULL, &device->devid,
device->uuid);
BUG_ON(IS_ERR(new_device)); /* -ENOMEM */
@@ -892,13 +890,13 @@ static void btrfs_close_one_device(struct btrfs_device *device)
list_replace_rcu(&device->dev_list, &new_device->dev_list);
new_device->fs_devices = device->fs_devices;
-
- call_rcu(&device->rcu, free_device);
}
static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
{
struct btrfs_device *device, *tmp;
+ static LIST_HEAD(pending_put);
+ INIT_LIST_HEAD(&pending_put);
if (--fs_devices->opened > 0)
return 0;
@@ -906,9 +904,24 @@ static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
mutex_lock(&fs_devices->device_list_mutex);
list_for_each_entry_safe(device, tmp, &fs_devices->devices, dev_list) {
btrfs_close_one_device(device);
+ list_add(&device->dev_list, &pending_put);
}
mutex_unlock(&fs_devices->device_list_mutex);
+ /*
+ * btrfs_show_devname() is using the device_list_mutex,
+ * sometimes a call to blkdev_put() leads vfs calling
+ * into this func. So do put outside of device_list_mutex,
+ * as of now.
+ */
+ while (!list_empty(&pending_put)) {
+ device = list_entry(pending_put.next,
+ struct btrfs_device, dev_list);
+ list_del(&device->dev_list);
+ btrfs_close_bdev(device);
+ call_rcu(&device->rcu, free_device);
+ }
+
WARN_ON(fs_devices->open_devices);
WARN_ON(fs_devices->rw_devices);
fs_devices->opened = 0;
--
2.7.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] btrfs: fix a possible umount deadlock
2016-09-09 8:31 [PATCH] btrfs: fix a possible umount deadlock Anand Jain
@ 2016-09-09 12:53 ` David Sterba
2016-09-09 23:08 ` Anand Jain
2016-09-09 23:03 ` [PATCH v2] " Anand Jain
2016-09-22 4:56 ` [PATCH v3] " Anand Jain
2 siblings, 1 reply; 10+ messages in thread
From: David Sterba @ 2016-09-09 12:53 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, dsterba, clm, idryomov
On Fri, Sep 09, 2016 at 04:31:04PM +0800, Anand Jain wrote:
> static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
> {
> struct btrfs_device *device, *tmp;
> + static LIST_HEAD(pending_put);
Why is it static?
> + INIT_LIST_HEAD(&pending_put);
>
> if (--fs_devices->opened > 0)
> return 0;
> @@ -906,9 +904,24 @@ static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
> mutex_lock(&fs_devices->device_list_mutex);
> list_for_each_entry_safe(device, tmp, &fs_devices->devices, dev_list) {
> btrfs_close_one_device(device);
> + list_add(&device->dev_list, &pending_put);
> }
> mutex_unlock(&fs_devices->device_list_mutex);
>
> + /*
> + * btrfs_show_devname() is using the device_list_mutex,
> + * sometimes a call to blkdev_put() leads vfs calling
> + * into this func. So do put outside of device_list_mutex,
> + * as of now.
> + */
> + while (!list_empty(&pending_put)) {
> + device = list_entry(pending_put.next,
> + struct btrfs_device, dev_list);
> + list_del(&device->dev_list);
> + btrfs_close_bdev(device);
> + call_rcu(&device->rcu, free_device);
> + }
> +
> WARN_ON(fs_devices->open_devices);
> WARN_ON(fs_devices->rw_devices);
> fs_devices->opened = 0;
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] btrfs: fix a possible umount deadlock
@ 2016-09-09 23:03 ` Anand Jain
2016-09-21 14:26 ` David Sterba
0 siblings, 1 reply; 10+ messages in thread
From: Anand Jain @ 2016-09-09 23:03 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, idryomov
btrfs_show_devname() is using the device_list_mutex, sometimes
a call to blkdev_put() leads vfs calling into this func. So
call blkdev_put() outside of device_list_mutex, as of now.
[ 983.284212] ======================================================
[ 983.290401] [ INFO: possible circular locking dependency detected ]
[ 983.296677] 4.8.0-rc5-ceph-00023-g1b39cec2 #1 Not tainted
[ 983.302081] -------------------------------------------------------
[ 983.308357] umount/21720 is trying to acquire lock:
[ 983.313243] (&bdev->bd_mutex){+.+.+.}, at: [<ffffffff9128ec51>] blkdev_put+0x31/0x150
[ 983.321264]
[ 983.321264] but task is already holding lock:
[ 983.327101] (&fs_devs->device_list_mutex){+.+...}, at: [<ffffffffc033d6f6>] __btrfs_close_devices+0x46/0x200 [btrfs]
[ 983.337839]
[ 983.337839] which lock already depends on the new lock.
[ 983.337839]
[ 983.346024]
[ 983.346024] the existing dependency chain (in reverse order) is:
[ 983.353512]
-> #4 (&fs_devs->device_list_mutex){+.+...}:
[ 983.359096] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.365143] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.371521] [<ffffffffc02d8116>] btrfs_show_devname+0x36/0x1f0 [btrfs]
[ 983.378710] [<ffffffff9129523e>] show_vfsmnt+0x4e/0x150
[ 983.384593] [<ffffffff9126ffc7>] m_show+0x17/0x20
[ 983.389957] [<ffffffff91276405>] seq_read+0x2b5/0x3b0
[ 983.395669] [<ffffffff9124c808>] __vfs_read+0x28/0x100
[ 983.401464] [<ffffffff9124eb3b>] vfs_read+0xab/0x150
[ 983.407080] [<ffffffff9124ec32>] SyS_read+0x52/0xb0
[ 983.412609] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 983.419617]
-> #3 (namespace_sem){++++++}:
[ 983.424024] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.430074] [<ffffffff918239e9>] down_write+0x49/0x80
[ 983.435785] [<ffffffff91272457>] lock_mount+0x67/0x1c0
[ 983.441582] [<ffffffff91272ab2>] do_add_mount+0x32/0xf0
[ 983.447458] [<ffffffff9127363a>] finish_automount+0x5a/0xc0
[ 983.453682] [<ffffffff91259513>] follow_managed+0x1b3/0x2a0
[ 983.459912] [<ffffffff9125b750>] lookup_fast+0x300/0x350
[ 983.465875] [<ffffffff9125d6e7>] path_openat+0x3a7/0xaa0
[ 983.471846] [<ffffffff9125ef75>] do_filp_open+0x85/0xe0
[ 983.477731] [<ffffffff9124c41c>] do_sys_open+0x14c/0x1f0
[ 983.483702] [<ffffffff9124c4de>] SyS_open+0x1e/0x20
[ 983.489240] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 983.496254]
-> #2 (&sb->s_type->i_mutex_key#3){+.+.+.}:
[ 983.501798] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.507855] [<ffffffff918239e9>] down_write+0x49/0x80
[ 983.513558] [<ffffffff91366237>] start_creating+0x87/0x100
[ 983.519703] [<ffffffff91366647>] debugfs_create_dir+0x17/0x100
[ 983.526195] [<ffffffff911df153>] bdi_register+0x93/0x210
[ 983.532165] [<ffffffff911df313>] bdi_register_owner+0x43/0x70
[ 983.538570] [<ffffffff914080fb>] device_add_disk+0x1fb/0x450
[ 983.544888] [<ffffffff91580226>] loop_add+0x1e6/0x290
[ 983.550596] [<ffffffff91fec358>] loop_init+0x10b/0x14f
[ 983.556394] [<ffffffff91002207>] do_one_initcall+0xa7/0x180
[ 983.562618] [<ffffffff91f932e0>] kernel_init_freeable+0x1cc/0x266
[ 983.569370] [<ffffffff918174be>] kernel_init+0xe/0x100
[ 983.575166] [<ffffffff9182620f>] ret_from_fork+0x1f/0x40
[ 983.581131]
-> #1 (loop_index_mutex){+.+.+.}:
[ 983.585801] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.591858] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.598256] [<ffffffff9157ed3f>] lo_open+0x1f/0x60
[ 983.603704] [<ffffffff9128eec3>] __blkdev_get+0x123/0x400
[ 983.609757] [<ffffffff9128f4ea>] blkdev_get+0x34a/0x350
[ 983.615639] [<ffffffff9128f554>] blkdev_open+0x64/0x80
[ 983.621428] [<ffffffff9124aff6>] do_dentry_open+0x1c6/0x2d0
[ 983.627651] [<ffffffff9124c029>] vfs_open+0x69/0x80
[ 983.633181] [<ffffffff9125db74>] path_openat+0x834/0xaa0
[ 983.639152] [<ffffffff9125ef75>] do_filp_open+0x85/0xe0
[ 983.645035] [<ffffffff9124c41c>] do_sys_open+0x14c/0x1f0
[ 983.650999] [<ffffffff9124c4de>] SyS_open+0x1e/0x20
[ 983.656535] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 983.663541]
-> #0 (&bdev->bd_mutex){+.+.+.}:
[ 983.668107] [<ffffffff910def43>] __lock_acquire+0x1003/0x17b0
[ 983.674510] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.680561] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.686967] [<ffffffff9128ec51>] blkdev_put+0x31/0x150
[ 983.692761] [<ffffffffc033481f>] btrfs_close_bdev+0x4f/0x60 [btrfs]
[ 983.699699] [<ffffffffc033d77b>] __btrfs_close_devices+0xcb/0x200 [btrfs]
[ 983.707178] [<ffffffffc033d8db>] btrfs_close_devices+0x2b/0xa0 [btrfs]
[ 983.714380] [<ffffffffc03081c5>] close_ctree+0x265/0x340 [btrfs]
[ 983.721061] [<ffffffffc02d7959>] btrfs_put_super+0x19/0x20 [btrfs]
[ 983.727908] [<ffffffff91250e2f>] generic_shutdown_super+0x6f/0x100
[ 983.734744] [<ffffffff91250f56>] kill_anon_super+0x16/0x30
[ 983.740888] [<ffffffffc02da97e>] btrfs_kill_super+0x1e/0x130 [btrfs]
[ 983.747909] [<ffffffff91250fe9>] deactivate_locked_super+0x49/0x80
[ 983.754745] [<ffffffff912515fd>] deactivate_super+0x5d/0x70
[ 983.760977] [<ffffffff91270a1c>] cleanup_mnt+0x5c/0x80
[ 983.766773] [<ffffffff91270a92>] __cleanup_mnt+0x12/0x20
[ 983.772738] [<ffffffff910aa2fe>] task_work_run+0x7e/0xc0
[ 983.778708] [<ffffffff91081b5a>] exit_to_usermode_loop+0x7e/0xb4
[ 983.785373] [<ffffffff910039eb>] syscall_return_slowpath+0xbb/0xd0
[ 983.792212] [<ffffffff9182605c>] entry_SYSCALL_64_fastpath+0xbf/0xc1
[ 983.799225]
[ 983.799225] other info that might help us debug this:
[ 983.799225]
[ 983.807291] Chain exists of:
&bdev->bd_mutex --> namespace_sem --> &fs_devs->device_list_mutex
[ 983.816521] Possible unsafe locking scenario:
[ 983.816521]
[ 983.822489] CPU0 CPU1
[ 983.827043] ---- ----
[ 983.831599] lock(&fs_devs->device_list_mutex);
[ 983.836289] lock(namespace_sem);
[ 983.842268] lock(&fs_devs->device_list_mutex);
[ 983.849478] lock(&bdev->bd_mutex);
[ 983.853127]
[ 983.853127] *** DEADLOCK ***
[ 983.853127]
[ 983.859113] 3 locks held by umount/21720:
[ 983.863145] #0: (&type->s_umount_key#35){++++..}, at: [<ffffffff912515f5>] deactivate_super+0x55/0x70
[ 983.872713] #1: (uuid_mutex){+.+.+.}, at: [<ffffffffc033d8d3>] btrfs_close_devices+0x23/0xa0 [btrfs]
[ 983.882206] #2: (&fs_devs->device_list_mutex){+.+...}, at: [<ffffffffc033d6f6>] __btrfs_close_devices+0x46/0x200 [btrfs]
[ 983.893422]
[ 983.893422] stack backtrace:
[ 983.897824] CPU: 6 PID: 21720 Comm: umount Not tainted 4.8.0-rc5-ceph-00023-g1b39cec2 #1
[ 983.905958] Hardware name: Supermicro SYS-5018R-WR/X10SRW-F, BIOS 1.0c 09/07/2015
[ 983.913492] 0000000000000000 ffff8c8a53c17a38 ffffffff91429521 ffffffff9260f4f0
[ 983.921018] ffffffff92642760 ffff8c8a53c17a88 ffffffff911b2b04 0000000000000050
[ 983.928542] ffffffff9237d620 ffff8c8a5294aee0 ffff8c8a5294aeb8 ffff8c8a5294aee0
[ 983.936072] Call Trace:
[ 983.938545] [<ffffffff91429521>] dump_stack+0x85/0xc4
[ 983.943715] [<ffffffff911b2b04>] print_circular_bug+0x1fb/0x20c
[ 983.949748] [<ffffffff910def43>] __lock_acquire+0x1003/0x17b0
[ 983.955613] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.961123] [<ffffffff9128ec51>] ? blkdev_put+0x31/0x150
[ 983.966550] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.972407] [<ffffffff9128ec51>] ? blkdev_put+0x31/0x150
[ 983.977832] [<ffffffff9128ec51>] blkdev_put+0x31/0x150
[ 983.983101] [<ffffffffc033481f>] btrfs_close_bdev+0x4f/0x60 [btrfs]
[ 983.989500] [<ffffffffc033d77b>] __btrfs_close_devices+0xcb/0x200 [btrfs]
[ 983.996415] [<ffffffffc033d8db>] btrfs_close_devices+0x2b/0xa0 [btrfs]
[ 984.003068] [<ffffffffc03081c5>] close_ctree+0x265/0x340 [btrfs]
[ 984.009189] [<ffffffff9126cc5e>] ? evict_inodes+0x15e/0x170
[ 984.014881] [<ffffffffc02d7959>] btrfs_put_super+0x19/0x20 [btrfs]
[ 984.021176] [<ffffffff91250e2f>] generic_shutdown_super+0x6f/0x100
[ 984.027476] [<ffffffff91250f56>] kill_anon_super+0x16/0x30
[ 984.033082] [<ffffffffc02da97e>] btrfs_kill_super+0x1e/0x130 [btrfs]
[ 984.039548] [<ffffffff91250fe9>] deactivate_locked_super+0x49/0x80
[ 984.045839] [<ffffffff912515fd>] deactivate_super+0x5d/0x70
[ 984.051525] [<ffffffff91270a1c>] cleanup_mnt+0x5c/0x80
[ 984.056774] [<ffffffff91270a92>] __cleanup_mnt+0x12/0x20
[ 984.062201] [<ffffffff910aa2fe>] task_work_run+0x7e/0xc0
[ 984.067625] [<ffffffff91081b5a>] exit_to_usermode_loop+0x7e/0xb4
[ 984.073747] [<ffffffff910039eb>] syscall_return_slowpath+0xbb/0xd0
[ 984.080038] [<ffffffff9182605c>] entry_SYSCALL_64_fastpath+0xbf/0xc1
Reported-by: Ilya Dryomov <idryomov@gmail.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v1->v2: Fix typo, remove static thanks Kdave.
fs/btrfs/volumes.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 1ce584893d1b..66dc3eeca6d0 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -877,8 +877,6 @@ static void btrfs_close_one_device(struct btrfs_device *device)
if (device->missing)
fs_devices->missing_devices--;
- btrfs_close_bdev(device);
-
new_device = btrfs_alloc_device(NULL, &device->devid,
device->uuid);
BUG_ON(IS_ERR(new_device)); /* -ENOMEM */
@@ -892,13 +890,13 @@ static void btrfs_close_one_device(struct btrfs_device *device)
list_replace_rcu(&device->dev_list, &new_device->dev_list);
new_device->fs_devices = device->fs_devices;
-
- call_rcu(&device->rcu, free_device);
}
static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
{
struct btrfs_device *device, *tmp;
+ LIST_HEAD(pending_put);
+ INIT_LIST_HEAD(&pending_put);
if (--fs_devices->opened > 0)
return 0;
@@ -906,9 +904,24 @@ static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
mutex_lock(&fs_devices->device_list_mutex);
list_for_each_entry_safe(device, tmp, &fs_devices->devices, dev_list) {
btrfs_close_one_device(device);
+ list_add(&device->dev_list, &pending_put);
}
mutex_unlock(&fs_devices->device_list_mutex);
+ /*
+ * btrfs_show_devname() is using the device_list_mutex,
+ * sometimes a call to blkdev_put() leads vfs calling
+ * into this func. So do put outside of device_list_mutex,
+ * as of now.
+ */
+ while (!list_empty(&pending_put)) {
+ device = list_entry(pending_put.next,
+ struct btrfs_device, dev_list);
+ list_del(&device->dev_list);
+ btrfs_close_bdev(device);
+ call_rcu(&device->rcu, free_device);
+ }
+
WARN_ON(fs_devices->open_devices);
WARN_ON(fs_devices->rw_devices);
fs_devices->opened = 0;
--
2.7.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] btrfs: fix a possible umount deadlock
2016-09-09 12:53 ` David Sterba
@ 2016-09-09 23:08 ` Anand Jain
0 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2016-09-09 23:08 UTC (permalink / raw)
To: dsterba, linux-btrfs, clm, idryomov
On 09/09/2016 08:53 PM, David Sterba wrote:
> On Fri, Sep 09, 2016 at 04:31:04PM +0800, Anand Jain wrote:
>> static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
>> {
>> struct btrfs_device *device, *tmp;
>> + static LIST_HEAD(pending_put);
>
> Why is it static?
sorry my mistake its typo. v2 is sent out.
Thanks, Anand
>> + INIT_LIST_HEAD(&pending_put);
>>
>> if (--fs_devices->opened > 0)
>> return 0;
>> @@ -906,9 +904,24 @@ static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
>> mutex_lock(&fs_devices->device_list_mutex);
>> list_for_each_entry_safe(device, tmp, &fs_devices->devices, dev_list) {
>> btrfs_close_one_device(device);
>> + list_add(&device->dev_list, &pending_put);
>> }
>> mutex_unlock(&fs_devices->device_list_mutex);
>>
>> + /*
>> + * btrfs_show_devname() is using the device_list_mutex,
>> + * sometimes a call to blkdev_put() leads vfs calling
>> + * into this func. So do put outside of device_list_mutex,
>> + * as of now.
>> + */
>> + while (!list_empty(&pending_put)) {
>> + device = list_entry(pending_put.next,
>> + struct btrfs_device, dev_list);
>> + list_del(&device->dev_list);
>> + btrfs_close_bdev(device);
>> + call_rcu(&device->rcu, free_device);
>> + }
>> +
>> WARN_ON(fs_devices->open_devices);
>> WARN_ON(fs_devices->rw_devices);
>> fs_devices->opened = 0;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] btrfs: fix a possible umount deadlock
2016-09-09 23:03 ` [PATCH v2] " Anand Jain
@ 2016-09-21 14:26 ` David Sterba
2016-09-22 4:59 ` Anand Jain
0 siblings, 1 reply; 10+ messages in thread
From: David Sterba @ 2016-09-21 14:26 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, dsterba, idryomov
On Sat, Sep 10, 2016 at 07:03:38AM +0800, Anand Jain wrote:
> static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
> {
> struct btrfs_device *device, *tmp;
> + LIST_HEAD(pending_put);
> + INIT_LIST_HEAD(&pending_put);
LIST_HEAD declares and initializes the list to an empty one, not
necessary to call INIT_LIST_HEAD.
>
> if (--fs_devices->opened > 0)
> return 0;
> @@ -906,9 +904,24 @@ static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
> mutex_lock(&fs_devices->device_list_mutex);
> list_for_each_entry_safe(device, tmp, &fs_devices->devices, dev_list) {
> btrfs_close_one_device(device);
> + list_add(&device->dev_list, &pending_put);
> }
> mutex_unlock(&fs_devices->device_list_mutex);
>
> + /*
> + * btrfs_show_devname() is using the device_list_mutex,
> + * sometimes a call to blkdev_put() leads vfs calling
> + * into this func. So do put outside of device_list_mutex,
> + * as of now.
> + */
> + while (!list_empty(&pending_put)) {
> + device = list_entry(pending_put.next,
Could be list_first_entry
> + struct btrfs_device, dev_list);
> + list_del(&device->dev_list);
> + btrfs_close_bdev(device);
> + call_rcu(&device->rcu, free_device);
> + }
> +
> WARN_ON(fs_devices->open_devices);
> WARN_ON(fs_devices->rw_devices);
> fs_devices->opened = 0;
After this patch, the function btrfs_close_one_device no longer does
what it says, ie. it does not close the device. I'd have to look closer
why it allocates a new structure and replaces it in the list, but this
looks weird.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3] btrfs: fix a possible umount deadlock
2016-09-09 8:31 [PATCH] btrfs: fix a possible umount deadlock Anand Jain
2016-09-09 12:53 ` David Sterba
2016-09-09 23:03 ` [PATCH v2] " Anand Jain
@ 2016-09-22 4:56 ` Anand Jain
2016-09-22 15:35 ` David Sterba
2016-09-22 16:24 ` Jeff Mahoney
2 siblings, 2 replies; 10+ messages in thread
From: Anand Jain @ 2016-09-22 4:56 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
btrfs_show_devname() is using the device_list_mutex, sometimes
a call to blkdev_put() leads vfs calling into this func. So
call blkdev_put() outside of device_list_mutex, as of now.
[ 983.284212] ======================================================
[ 983.290401] [ INFO: possible circular locking dependency detected ]
[ 983.296677] 4.8.0-rc5-ceph-00023-g1b39cec2 #1 Not tainted
[ 983.302081] -------------------------------------------------------
[ 983.308357] umount/21720 is trying to acquire lock:
[ 983.313243] (&bdev->bd_mutex){+.+.+.}, at: [<ffffffff9128ec51>] blkdev_put+0x31/0x150
[ 983.321264]
[ 983.321264] but task is already holding lock:
[ 983.327101] (&fs_devs->device_list_mutex){+.+...}, at: [<ffffffffc033d6f6>] __btrfs_close_devices+0x46/0x200 [btrfs]
[ 983.337839]
[ 983.337839] which lock already depends on the new lock.
[ 983.337839]
[ 983.346024]
[ 983.346024] the existing dependency chain (in reverse order) is:
[ 983.353512]
-> #4 (&fs_devs->device_list_mutex){+.+...}:
[ 983.359096] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.365143] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.371521] [<ffffffffc02d8116>] btrfs_show_devname+0x36/0x1f0 [btrfs]
[ 983.378710] [<ffffffff9129523e>] show_vfsmnt+0x4e/0x150
[ 983.384593] [<ffffffff9126ffc7>] m_show+0x17/0x20
[ 983.389957] [<ffffffff91276405>] seq_read+0x2b5/0x3b0
[ 983.395669] [<ffffffff9124c808>] __vfs_read+0x28/0x100
[ 983.401464] [<ffffffff9124eb3b>] vfs_read+0xab/0x150
[ 983.407080] [<ffffffff9124ec32>] SyS_read+0x52/0xb0
[ 983.412609] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 983.419617]
-> #3 (namespace_sem){++++++}:
[ 983.424024] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.430074] [<ffffffff918239e9>] down_write+0x49/0x80
[ 983.435785] [<ffffffff91272457>] lock_mount+0x67/0x1c0
[ 983.441582] [<ffffffff91272ab2>] do_add_mount+0x32/0xf0
[ 983.447458] [<ffffffff9127363a>] finish_automount+0x5a/0xc0
[ 983.453682] [<ffffffff91259513>] follow_managed+0x1b3/0x2a0
[ 983.459912] [<ffffffff9125b750>] lookup_fast+0x300/0x350
[ 983.465875] [<ffffffff9125d6e7>] path_openat+0x3a7/0xaa0
[ 983.471846] [<ffffffff9125ef75>] do_filp_open+0x85/0xe0
[ 983.477731] [<ffffffff9124c41c>] do_sys_open+0x14c/0x1f0
[ 983.483702] [<ffffffff9124c4de>] SyS_open+0x1e/0x20
[ 983.489240] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 983.496254]
-> #2 (&sb->s_type->i_mutex_key#3){+.+.+.}:
[ 983.501798] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.507855] [<ffffffff918239e9>] down_write+0x49/0x80
[ 983.513558] [<ffffffff91366237>] start_creating+0x87/0x100
[ 983.519703] [<ffffffff91366647>] debugfs_create_dir+0x17/0x100
[ 983.526195] [<ffffffff911df153>] bdi_register+0x93/0x210
[ 983.532165] [<ffffffff911df313>] bdi_register_owner+0x43/0x70
[ 983.538570] [<ffffffff914080fb>] device_add_disk+0x1fb/0x450
[ 983.544888] [<ffffffff91580226>] loop_add+0x1e6/0x290
[ 983.550596] [<ffffffff91fec358>] loop_init+0x10b/0x14f
[ 983.556394] [<ffffffff91002207>] do_one_initcall+0xa7/0x180
[ 983.562618] [<ffffffff91f932e0>] kernel_init_freeable+0x1cc/0x266
[ 983.569370] [<ffffffff918174be>] kernel_init+0xe/0x100
[ 983.575166] [<ffffffff9182620f>] ret_from_fork+0x1f/0x40
[ 983.581131]
-> #1 (loop_index_mutex){+.+.+.}:
[ 983.585801] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.591858] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.598256] [<ffffffff9157ed3f>] lo_open+0x1f/0x60
[ 983.603704] [<ffffffff9128eec3>] __blkdev_get+0x123/0x400
[ 983.609757] [<ffffffff9128f4ea>] blkdev_get+0x34a/0x350
[ 983.615639] [<ffffffff9128f554>] blkdev_open+0x64/0x80
[ 983.621428] [<ffffffff9124aff6>] do_dentry_open+0x1c6/0x2d0
[ 983.627651] [<ffffffff9124c029>] vfs_open+0x69/0x80
[ 983.633181] [<ffffffff9125db74>] path_openat+0x834/0xaa0
[ 983.639152] [<ffffffff9125ef75>] do_filp_open+0x85/0xe0
[ 983.645035] [<ffffffff9124c41c>] do_sys_open+0x14c/0x1f0
[ 983.650999] [<ffffffff9124c4de>] SyS_open+0x1e/0x20
[ 983.656535] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
[ 983.663541]
-> #0 (&bdev->bd_mutex){+.+.+.}:
[ 983.668107] [<ffffffff910def43>] __lock_acquire+0x1003/0x17b0
[ 983.674510] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.680561] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.686967] [<ffffffff9128ec51>] blkdev_put+0x31/0x150
[ 983.692761] [<ffffffffc033481f>] btrfs_close_bdev+0x4f/0x60 [btrfs]
[ 983.699699] [<ffffffffc033d77b>] __btrfs_close_devices+0xcb/0x200 [btrfs]
[ 983.707178] [<ffffffffc033d8db>] btrfs_close_devices+0x2b/0xa0 [btrfs]
[ 983.714380] [<ffffffffc03081c5>] close_ctree+0x265/0x340 [btrfs]
[ 983.721061] [<ffffffffc02d7959>] btrfs_put_super+0x19/0x20 [btrfs]
[ 983.727908] [<ffffffff91250e2f>] generic_shutdown_super+0x6f/0x100
[ 983.734744] [<ffffffff91250f56>] kill_anon_super+0x16/0x30
[ 983.740888] [<ffffffffc02da97e>] btrfs_kill_super+0x1e/0x130 [btrfs]
[ 983.747909] [<ffffffff91250fe9>] deactivate_locked_super+0x49/0x80
[ 983.754745] [<ffffffff912515fd>] deactivate_super+0x5d/0x70
[ 983.760977] [<ffffffff91270a1c>] cleanup_mnt+0x5c/0x80
[ 983.766773] [<ffffffff91270a92>] __cleanup_mnt+0x12/0x20
[ 983.772738] [<ffffffff910aa2fe>] task_work_run+0x7e/0xc0
[ 983.778708] [<ffffffff91081b5a>] exit_to_usermode_loop+0x7e/0xb4
[ 983.785373] [<ffffffff910039eb>] syscall_return_slowpath+0xbb/0xd0
[ 983.792212] [<ffffffff9182605c>] entry_SYSCALL_64_fastpath+0xbf/0xc1
[ 983.799225]
[ 983.799225] other info that might help us debug this:
[ 983.799225]
[ 983.807291] Chain exists of:
&bdev->bd_mutex --> namespace_sem --> &fs_devs->device_list_mutex
[ 983.816521] Possible unsafe locking scenario:
[ 983.816521]
[ 983.822489] CPU0 CPU1
[ 983.827043] ---- ----
[ 983.831599] lock(&fs_devs->device_list_mutex);
[ 983.836289] lock(namespace_sem);
[ 983.842268] lock(&fs_devs->device_list_mutex);
[ 983.849478] lock(&bdev->bd_mutex);
[ 983.853127]
[ 983.853127] *** DEADLOCK ***
[ 983.853127]
[ 983.859113] 3 locks held by umount/21720:
[ 983.863145] #0: (&type->s_umount_key#35){++++..}, at: [<ffffffff912515f5>] deactivate_super+0x55/0x70
[ 983.872713] #1: (uuid_mutex){+.+.+.}, at: [<ffffffffc033d8d3>] btrfs_close_devices+0x23/0xa0 [btrfs]
[ 983.882206] #2: (&fs_devs->device_list_mutex){+.+...}, at: [<ffffffffc033d6f6>] __btrfs_close_devices+0x46/0x200 [btrfs]
[ 983.893422]
[ 983.893422] stack backtrace:
[ 983.897824] CPU: 6 PID: 21720 Comm: umount Not tainted 4.8.0-rc5-ceph-00023-g1b39cec2 #1
[ 983.905958] Hardware name: Supermicro SYS-5018R-WR/X10SRW-F, BIOS 1.0c 09/07/2015
[ 983.913492] 0000000000000000 ffff8c8a53c17a38 ffffffff91429521 ffffffff9260f4f0
[ 983.921018] ffffffff92642760 ffff8c8a53c17a88 ffffffff911b2b04 0000000000000050
[ 983.928542] ffffffff9237d620 ffff8c8a5294aee0 ffff8c8a5294aeb8 ffff8c8a5294aee0
[ 983.936072] Call Trace:
[ 983.938545] [<ffffffff91429521>] dump_stack+0x85/0xc4
[ 983.943715] [<ffffffff911b2b04>] print_circular_bug+0x1fb/0x20c
[ 983.949748] [<ffffffff910def43>] __lock_acquire+0x1003/0x17b0
[ 983.955613] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
[ 983.961123] [<ffffffff9128ec51>] ? blkdev_put+0x31/0x150
[ 983.966550] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
[ 983.972407] [<ffffffff9128ec51>] ? blkdev_put+0x31/0x150
[ 983.977832] [<ffffffff9128ec51>] blkdev_put+0x31/0x150
[ 983.983101] [<ffffffffc033481f>] btrfs_close_bdev+0x4f/0x60 [btrfs]
[ 983.989500] [<ffffffffc033d77b>] __btrfs_close_devices+0xcb/0x200 [btrfs]
[ 983.996415] [<ffffffffc033d8db>] btrfs_close_devices+0x2b/0xa0 [btrfs]
[ 984.003068] [<ffffffffc03081c5>] close_ctree+0x265/0x340 [btrfs]
[ 984.009189] [<ffffffff9126cc5e>] ? evict_inodes+0x15e/0x170
[ 984.014881] [<ffffffffc02d7959>] btrfs_put_super+0x19/0x20 [btrfs]
[ 984.021176] [<ffffffff91250e2f>] generic_shutdown_super+0x6f/0x100
[ 984.027476] [<ffffffff91250f56>] kill_anon_super+0x16/0x30
[ 984.033082] [<ffffffffc02da97e>] btrfs_kill_super+0x1e/0x130 [btrfs]
[ 984.039548] [<ffffffff91250fe9>] deactivate_locked_super+0x49/0x80
[ 984.045839] [<ffffffff912515fd>] deactivate_super+0x5d/0x70
[ 984.051525] [<ffffffff91270a1c>] cleanup_mnt+0x5c/0x80
[ 984.056774] [<ffffffff91270a92>] __cleanup_mnt+0x12/0x20
[ 984.062201] [<ffffffff910aa2fe>] task_work_run+0x7e/0xc0
[ 984.067625] [<ffffffff91081b5a>] exit_to_usermode_loop+0x7e/0xb4
[ 984.073747] [<ffffffff910039eb>] syscall_return_slowpath+0xbb/0xd0
[ 984.080038] [<ffffffff9182605c>] entry_SYSCALL_64_fastpath+0xbf/0xc1
Reported-by: Ilya Dryomov <idryomov@gmail.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2->v3: rename btrfs_close_one_device() to btrfs_prepare_close_one_device()
use struct list_head instead of LIST_HEAD
use list_first_entry instead of list_entry
v1->v2: Fix typo, remove static thanks Kdave.
fs/btrfs/volumes.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 1ce584893d1b..0f4460e50cac 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -859,7 +859,7 @@ static void btrfs_close_bdev(struct btrfs_device *device)
blkdev_put(device->bdev, device->mode);
}
-static void btrfs_close_one_device(struct btrfs_device *device)
+static void btrfs_prepare_close_one_device(struct btrfs_device *device)
{
struct btrfs_fs_devices *fs_devices = device->fs_devices;
struct btrfs_device *new_device;
@@ -877,8 +877,6 @@ static void btrfs_close_one_device(struct btrfs_device *device)
if (device->missing)
fs_devices->missing_devices--;
- btrfs_close_bdev(device);
-
new_device = btrfs_alloc_device(NULL, &device->devid,
device->uuid);
BUG_ON(IS_ERR(new_device)); /* -ENOMEM */
@@ -892,23 +890,39 @@ static void btrfs_close_one_device(struct btrfs_device *device)
list_replace_rcu(&device->dev_list, &new_device->dev_list);
new_device->fs_devices = device->fs_devices;
-
- call_rcu(&device->rcu, free_device);
}
static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
{
struct btrfs_device *device, *tmp;
+ struct list_head pending_put;
+
+ INIT_LIST_HEAD(&pending_put);
if (--fs_devices->opened > 0)
return 0;
mutex_lock(&fs_devices->device_list_mutex);
list_for_each_entry_safe(device, tmp, &fs_devices->devices, dev_list) {
- btrfs_close_one_device(device);
+ btrfs_prepare_close_one_device(device);
+ list_add(&device->dev_list, &pending_put);
}
mutex_unlock(&fs_devices->device_list_mutex);
+ /*
+ * btrfs_show_devname() is using the device_list_mutex,
+ * sometimes call to blkdev_put() leads vfs calling
+ * into this func. So do put outside of device_list_mutex,
+ * as of now.
+ */
+ while (!list_empty(&pending_put)) {
+ device = list_first_entry(&pending_put,
+ struct btrfs_device, dev_list);
+ list_del(&device->dev_list);
+ btrfs_close_bdev(device);
+ call_rcu(&device->rcu, free_device);
+ }
+
WARN_ON(fs_devices->open_devices);
WARN_ON(fs_devices->rw_devices);
fs_devices->opened = 0;
--
2.7.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] btrfs: fix a possible umount deadlock
2016-09-21 14:26 ` David Sterba
@ 2016-09-22 4:59 ` Anand Jain
0 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2016-09-22 4:59 UTC (permalink / raw)
To: dsterba, linux-btrfs, idryomov
On 09/21/2016 10:26 PM, David Sterba wrote:
> On Sat, Sep 10, 2016 at 07:03:38AM +0800, Anand Jain wrote:
>> static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
>> {
>> struct btrfs_device *device, *tmp;
>> + LIST_HEAD(pending_put);
>> + INIT_LIST_HEAD(&pending_put);
>
> LIST_HEAD declares and initializes the list to an empty one, not
> necessary to call INIT_LIST_HEAD.
Will use struct list_head makes it with inline with rest of the code.
>>
>> if (--fs_devices->opened > 0)
>> return 0;
>> @@ -906,9 +904,24 @@ static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
>> mutex_lock(&fs_devices->device_list_mutex);
>> list_for_each_entry_safe(device, tmp, &fs_devices->devices, dev_list) {
>> btrfs_close_one_device(device);
>> + list_add(&device->dev_list, &pending_put);
>> }
>> mutex_unlock(&fs_devices->device_list_mutex);
>>
>> + /*
>> + * btrfs_show_devname() is using the device_list_mutex,
>> + * sometimes a call to blkdev_put() leads vfs calling
>> + * into this func. So do put outside of device_list_mutex,
>> + * as of now.
>> + */
>> + while (!list_empty(&pending_put)) {
>> + device = list_entry(pending_put.next,
>
> Could be list_first_entry
ok.
>> + struct btrfs_device, dev_list);
>> + list_del(&device->dev_list);
>> + btrfs_close_bdev(device);
>> + call_rcu(&device->rcu, free_device);
>> + }
>> +
>> WARN_ON(fs_devices->open_devices);
>> WARN_ON(fs_devices->rw_devices);
>> fs_devices->opened = 0;
>
> After this patch, the function btrfs_close_one_device no longer does
> what it says, ie. it does not close the device.
renamed to btrfs_prepare_close_one_device()
> I'd have to look closer
> why it allocates a new structure and replaces it in the list, but this
> looks weird.
I asked this in the ML quite sometime back. yeah reason is unknown.
So this reason the sysfs patches (in the ML) is probably got x2
complicated.
-Anand
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] btrfs: fix a possible umount deadlock
2016-09-22 4:56 ` [PATCH v3] " Anand Jain
@ 2016-09-22 15:35 ` David Sterba
2016-09-22 16:24 ` Jeff Mahoney
1 sibling, 0 replies; 10+ messages in thread
From: David Sterba @ 2016-09-22 15:35 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, dsterba
On Thu, Sep 22, 2016 at 12:56:13PM +0800, Anand Jain wrote:
> btrfs_show_devname() is using the device_list_mutex, sometimes
> a call to blkdev_put() leads vfs calling into this func. So
> call blkdev_put() outside of device_list_mutex, as of now.
>
[...]
>
> Reported-by: Ilya Dryomov <idryomov@gmail.com>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] btrfs: fix a possible umount deadlock
2016-09-22 4:56 ` [PATCH v3] " Anand Jain
2016-09-22 15:35 ` David Sterba
@ 2016-09-22 16:24 ` Jeff Mahoney
2016-10-03 16:20 ` David Sterba
1 sibling, 1 reply; 10+ messages in thread
From: Jeff Mahoney @ 2016-09-22 16:24 UTC (permalink / raw)
To: Anand Jain, linux-btrfs; +Cc: dsterba
[-- Attachment #1.1: Type: text/plain, Size: 12835 bytes --]
On 9/22/16 12:56 AM, Anand Jain wrote:
> btrfs_show_devname() is using the device_list_mutex, sometimes
> a call to blkdev_put() leads vfs calling into this func. So
> call blkdev_put() outside of device_list_mutex, as of now.
>
> [ 983.284212] ======================================================
> [ 983.290401] [ INFO: possible circular locking dependency detected ]
> [ 983.296677] 4.8.0-rc5-ceph-00023-g1b39cec2 #1 Not tainted
> [ 983.302081] -------------------------------------------------------
> [ 983.308357] umount/21720 is trying to acquire lock:
> [ 983.313243] (&bdev->bd_mutex){+.+.+.}, at: [<ffffffff9128ec51>] blkdev_put+0x31/0x150
> [ 983.321264]
> [ 983.321264] but task is already holding lock:
> [ 983.327101] (&fs_devs->device_list_mutex){+.+...}, at: [<ffffffffc033d6f6>] __btrfs_close_devices+0x46/0x200 [btrfs]
> [ 983.337839]
> [ 983.337839] which lock already depends on the new lock.
> [ 983.337839]
> [ 983.346024]
> [ 983.346024] the existing dependency chain (in reverse order) is:
> [ 983.353512]
> -> #4 (&fs_devs->device_list_mutex){+.+...}:
> [ 983.359096] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
> [ 983.365143] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
> [ 983.371521] [<ffffffffc02d8116>] btrfs_show_devname+0x36/0x1f0 [btrfs]
> [ 983.378710] [<ffffffff9129523e>] show_vfsmnt+0x4e/0x150
> [ 983.384593] [<ffffffff9126ffc7>] m_show+0x17/0x20
> [ 983.389957] [<ffffffff91276405>] seq_read+0x2b5/0x3b0
> [ 983.395669] [<ffffffff9124c808>] __vfs_read+0x28/0x100
> [ 983.401464] [<ffffffff9124eb3b>] vfs_read+0xab/0x150
> [ 983.407080] [<ffffffff9124ec32>] SyS_read+0x52/0xb0
> [ 983.412609] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
> [ 983.419617]
> -> #3 (namespace_sem){++++++}:
> [ 983.424024] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
> [ 983.430074] [<ffffffff918239e9>] down_write+0x49/0x80
> [ 983.435785] [<ffffffff91272457>] lock_mount+0x67/0x1c0
> [ 983.441582] [<ffffffff91272ab2>] do_add_mount+0x32/0xf0
> [ 983.447458] [<ffffffff9127363a>] finish_automount+0x5a/0xc0
> [ 983.453682] [<ffffffff91259513>] follow_managed+0x1b3/0x2a0
> [ 983.459912] [<ffffffff9125b750>] lookup_fast+0x300/0x350
> [ 983.465875] [<ffffffff9125d6e7>] path_openat+0x3a7/0xaa0
> [ 983.471846] [<ffffffff9125ef75>] do_filp_open+0x85/0xe0
> [ 983.477731] [<ffffffff9124c41c>] do_sys_open+0x14c/0x1f0
> [ 983.483702] [<ffffffff9124c4de>] SyS_open+0x1e/0x20
> [ 983.489240] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
> [ 983.496254]
> -> #2 (&sb->s_type->i_mutex_key#3){+.+.+.}:
> [ 983.501798] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
> [ 983.507855] [<ffffffff918239e9>] down_write+0x49/0x80
> [ 983.513558] [<ffffffff91366237>] start_creating+0x87/0x100
> [ 983.519703] [<ffffffff91366647>] debugfs_create_dir+0x17/0x100
> [ 983.526195] [<ffffffff911df153>] bdi_register+0x93/0x210
> [ 983.532165] [<ffffffff911df313>] bdi_register_owner+0x43/0x70
> [ 983.538570] [<ffffffff914080fb>] device_add_disk+0x1fb/0x450
> [ 983.544888] [<ffffffff91580226>] loop_add+0x1e6/0x290
> [ 983.550596] [<ffffffff91fec358>] loop_init+0x10b/0x14f
> [ 983.556394] [<ffffffff91002207>] do_one_initcall+0xa7/0x180
> [ 983.562618] [<ffffffff91f932e0>] kernel_init_freeable+0x1cc/0x266
> [ 983.569370] [<ffffffff918174be>] kernel_init+0xe/0x100
> [ 983.575166] [<ffffffff9182620f>] ret_from_fork+0x1f/0x40
> [ 983.581131]
> -> #1 (loop_index_mutex){+.+.+.}:
> [ 983.585801] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
> [ 983.591858] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
> [ 983.598256] [<ffffffff9157ed3f>] lo_open+0x1f/0x60
> [ 983.603704] [<ffffffff9128eec3>] __blkdev_get+0x123/0x400
> [ 983.609757] [<ffffffff9128f4ea>] blkdev_get+0x34a/0x350
> [ 983.615639] [<ffffffff9128f554>] blkdev_open+0x64/0x80
> [ 983.621428] [<ffffffff9124aff6>] do_dentry_open+0x1c6/0x2d0
> [ 983.627651] [<ffffffff9124c029>] vfs_open+0x69/0x80
> [ 983.633181] [<ffffffff9125db74>] path_openat+0x834/0xaa0
> [ 983.639152] [<ffffffff9125ef75>] do_filp_open+0x85/0xe0
> [ 983.645035] [<ffffffff9124c41c>] do_sys_open+0x14c/0x1f0
> [ 983.650999] [<ffffffff9124c4de>] SyS_open+0x1e/0x20
> [ 983.656535] [<ffffffff91825fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1
> [ 983.663541]
> -> #0 (&bdev->bd_mutex){+.+.+.}:
> [ 983.668107] [<ffffffff910def43>] __lock_acquire+0x1003/0x17b0
> [ 983.674510] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
> [ 983.680561] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
> [ 983.686967] [<ffffffff9128ec51>] blkdev_put+0x31/0x150
> [ 983.692761] [<ffffffffc033481f>] btrfs_close_bdev+0x4f/0x60 [btrfs]
> [ 983.699699] [<ffffffffc033d77b>] __btrfs_close_devices+0xcb/0x200 [btrfs]
> [ 983.707178] [<ffffffffc033d8db>] btrfs_close_devices+0x2b/0xa0 [btrfs]
> [ 983.714380] [<ffffffffc03081c5>] close_ctree+0x265/0x340 [btrfs]
> [ 983.721061] [<ffffffffc02d7959>] btrfs_put_super+0x19/0x20 [btrfs]
> [ 983.727908] [<ffffffff91250e2f>] generic_shutdown_super+0x6f/0x100
> [ 983.734744] [<ffffffff91250f56>] kill_anon_super+0x16/0x30
> [ 983.740888] [<ffffffffc02da97e>] btrfs_kill_super+0x1e/0x130 [btrfs]
> [ 983.747909] [<ffffffff91250fe9>] deactivate_locked_super+0x49/0x80
> [ 983.754745] [<ffffffff912515fd>] deactivate_super+0x5d/0x70
> [ 983.760977] [<ffffffff91270a1c>] cleanup_mnt+0x5c/0x80
> [ 983.766773] [<ffffffff91270a92>] __cleanup_mnt+0x12/0x20
> [ 983.772738] [<ffffffff910aa2fe>] task_work_run+0x7e/0xc0
> [ 983.778708] [<ffffffff91081b5a>] exit_to_usermode_loop+0x7e/0xb4
> [ 983.785373] [<ffffffff910039eb>] syscall_return_slowpath+0xbb/0xd0
> [ 983.792212] [<ffffffff9182605c>] entry_SYSCALL_64_fastpath+0xbf/0xc1
> [ 983.799225]
> [ 983.799225] other info that might help us debug this:
> [ 983.799225]
> [ 983.807291] Chain exists of:
> &bdev->bd_mutex --> namespace_sem --> &fs_devs->device_list_mutex
>
> [ 983.816521] Possible unsafe locking scenario:
> [ 983.816521]
> [ 983.822489] CPU0 CPU1
> [ 983.827043] ---- ----
> [ 983.831599] lock(&fs_devs->device_list_mutex);
> [ 983.836289] lock(namespace_sem);
> [ 983.842268] lock(&fs_devs->device_list_mutex);
> [ 983.849478] lock(&bdev->bd_mutex);
> [ 983.853127]
> [ 983.853127] *** DEADLOCK ***
> [ 983.853127]
> [ 983.859113] 3 locks held by umount/21720:
> [ 983.863145] #0: (&type->s_umount_key#35){++++..}, at: [<ffffffff912515f5>] deactivate_super+0x55/0x70
> [ 983.872713] #1: (uuid_mutex){+.+.+.}, at: [<ffffffffc033d8d3>] btrfs_close_devices+0x23/0xa0 [btrfs]
> [ 983.882206] #2: (&fs_devs->device_list_mutex){+.+...}, at: [<ffffffffc033d6f6>] __btrfs_close_devices+0x46/0x200 [btrfs]
> [ 983.893422]
> [ 983.893422] stack backtrace:
> [ 983.897824] CPU: 6 PID: 21720 Comm: umount Not tainted 4.8.0-rc5-ceph-00023-g1b39cec2 #1
> [ 983.905958] Hardware name: Supermicro SYS-5018R-WR/X10SRW-F, BIOS 1.0c 09/07/2015
> [ 983.913492] 0000000000000000 ffff8c8a53c17a38 ffffffff91429521 ffffffff9260f4f0
> [ 983.921018] ffffffff92642760 ffff8c8a53c17a88 ffffffff911b2b04 0000000000000050
> [ 983.928542] ffffffff9237d620 ffff8c8a5294aee0 ffff8c8a5294aeb8 ffff8c8a5294aee0
> [ 983.936072] Call Trace:
> [ 983.938545] [<ffffffff91429521>] dump_stack+0x85/0xc4
> [ 983.943715] [<ffffffff911b2b04>] print_circular_bug+0x1fb/0x20c
> [ 983.949748] [<ffffffff910def43>] __lock_acquire+0x1003/0x17b0
> [ 983.955613] [<ffffffff910dfd0c>] lock_acquire+0x1bc/0x1f0
> [ 983.961123] [<ffffffff9128ec51>] ? blkdev_put+0x31/0x150
> [ 983.966550] [<ffffffff91823125>] mutex_lock_nested+0x65/0x350
> [ 983.972407] [<ffffffff9128ec51>] ? blkdev_put+0x31/0x150
> [ 983.977832] [<ffffffff9128ec51>] blkdev_put+0x31/0x150
> [ 983.983101] [<ffffffffc033481f>] btrfs_close_bdev+0x4f/0x60 [btrfs]
> [ 983.989500] [<ffffffffc033d77b>] __btrfs_close_devices+0xcb/0x200 [btrfs]
> [ 983.996415] [<ffffffffc033d8db>] btrfs_close_devices+0x2b/0xa0 [btrfs]
> [ 984.003068] [<ffffffffc03081c5>] close_ctree+0x265/0x340 [btrfs]
> [ 984.009189] [<ffffffff9126cc5e>] ? evict_inodes+0x15e/0x170
> [ 984.014881] [<ffffffffc02d7959>] btrfs_put_super+0x19/0x20 [btrfs]
> [ 984.021176] [<ffffffff91250e2f>] generic_shutdown_super+0x6f/0x100
> [ 984.027476] [<ffffffff91250f56>] kill_anon_super+0x16/0x30
> [ 984.033082] [<ffffffffc02da97e>] btrfs_kill_super+0x1e/0x130 [btrfs]
> [ 984.039548] [<ffffffff91250fe9>] deactivate_locked_super+0x49/0x80
> [ 984.045839] [<ffffffff912515fd>] deactivate_super+0x5d/0x70
> [ 984.051525] [<ffffffff91270a1c>] cleanup_mnt+0x5c/0x80
> [ 984.056774] [<ffffffff91270a92>] __cleanup_mnt+0x12/0x20
> [ 984.062201] [<ffffffff910aa2fe>] task_work_run+0x7e/0xc0
> [ 984.067625] [<ffffffff91081b5a>] exit_to_usermode_loop+0x7e/0xb4
> [ 984.073747] [<ffffffff910039eb>] syscall_return_slowpath+0xbb/0xd0
> [ 984.080038] [<ffffffff9182605c>] entry_SYSCALL_64_fastpath+0xbf/0xc1
>
> Reported-by: Ilya Dryomov <idryomov@gmail.com>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v2->v3: rename btrfs_close_one_device() to btrfs_prepare_close_one_device()
> use struct list_head instead of LIST_HEAD
> use list_first_entry instead of list_entry
> v1->v2: Fix typo, remove static thanks Kdave.
>
> fs/btrfs/volumes.c | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 1ce584893d1b..0f4460e50cac 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -859,7 +859,7 @@ static void btrfs_close_bdev(struct btrfs_device *device)
> blkdev_put(device->bdev, device->mode);
> }
>
> -static void btrfs_close_one_device(struct btrfs_device *device)
> +static void btrfs_prepare_close_one_device(struct btrfs_device *device)
> {
> struct btrfs_fs_devices *fs_devices = device->fs_devices;
> struct btrfs_device *new_device;
> @@ -877,8 +877,6 @@ static void btrfs_close_one_device(struct btrfs_device *device)
> if (device->missing)
> fs_devices->missing_devices--;
>
> - btrfs_close_bdev(device);
> -
> new_device = btrfs_alloc_device(NULL, &device->devid,
> device->uuid);
> BUG_ON(IS_ERR(new_device)); /* -ENOMEM */
> @@ -892,23 +890,39 @@ static void btrfs_close_one_device(struct btrfs_device *device)
>
> list_replace_rcu(&device->dev_list, &new_device->dev_list);
> new_device->fs_devices = device->fs_devices;
> -
> - call_rcu(&device->rcu, free_device);
> }
>
> static int __btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
> {
> struct btrfs_device *device, *tmp;
> + struct list_head pending_put;
> +
> + INIT_LIST_HEAD(&pending_put);
>
> if (--fs_devices->opened > 0)
> return 0;
>
> mutex_lock(&fs_devices->device_list_mutex);
> list_for_each_entry_safe(device, tmp, &fs_devices->devices, dev_list) {
> - btrfs_close_one_device(device);
> + btrfs_prepare_close_one_device(device);
> + list_add(&device->dev_list, &pending_put);
> }
> mutex_unlock(&fs_devices->device_list_mutex);
>
> + /*
> + * btrfs_show_devname() is using the device_list_mutex,
> + * sometimes call to blkdev_put() leads vfs calling
> + * into this func. So do put outside of device_list_mutex,
> + * as of now.
> + */
> + while (!list_empty(&pending_put)) {
> + device = list_first_entry(&pending_put,
> + struct btrfs_device, dev_list);
> + list_del(&device->dev_list);
> + btrfs_close_bdev(device);
> + call_rcu(&device->rcu, free_device);
This isn't necessarily a comment on this code in particular, but what's
the reason for using call_rcu to defer the freeing instead of
synchronize_rcu and freeing the device directly via __free_device (if it
accepted a btrfs_device)? It's a pattern that's used throughout
volumes.c and it's old[1]. I'm not sure if it was intentional to defer
freeing since that was actually a functional change from the code it
replaced.
-Jeff
[1] 1f78160ce1b1b (Btrfs: using rcu lock in the reader side of devices list)
--
Jeff Mahoney
SUSE Labs
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 881 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] btrfs: fix a possible umount deadlock
2016-09-22 16:24 ` Jeff Mahoney
@ 2016-10-03 16:20 ` David Sterba
0 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2016-10-03 16:20 UTC (permalink / raw)
To: Jeff Mahoney; +Cc: Anand Jain, linux-btrfs
On Thu, Sep 22, 2016 at 12:24:07PM -0400, Jeff Mahoney wrote:
> This isn't necessarily a comment on this code in particular, but what's
> the reason for using call_rcu to defer the freeing instead of
> synchronize_rcu and freeing the device directly via __free_device (if it
> accepted a btrfs_device)? It's a pattern that's used throughout
> volumes.c and it's old[1]. I'm not sure if it was intentional to defer
> freeing since that was actually a functional change from the code it
> replaced.
It could be unintentional, the patch was supposed to relax the read-only
side, ie. use RCU instead of mutex. Deferred freeing does not seem to be
necessary. The device locking code needs cleanup/refactoring.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-10-03 16:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-09 8:31 [PATCH] btrfs: fix a possible umount deadlock Anand Jain
2016-09-09 12:53 ` David Sterba
2016-09-09 23:08 ` Anand Jain
2016-09-09 23:03 ` [PATCH v2] " Anand Jain
2016-09-21 14:26 ` David Sterba
2016-09-22 4:59 ` Anand Jain
2016-09-22 4:56 ` [PATCH v3] " Anand Jain
2016-09-22 15:35 ` David Sterba
2016-09-22 16:24 ` Jeff Mahoney
2016-10-03 16:20 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.