* [PATCH] Revert "btrfs: fix a possible umount deadlock"
@ 2018-06-28 8:48 Nikolay Borisov
2018-06-28 21:53 ` kbuild test robot
2018-06-28 22:49 ` kbuild test robot
0 siblings, 2 replies; 3+ messages in thread
From: Nikolay Borisov @ 2018-06-28 8:48 UTC (permalink / raw)
To: anand.jain; +Cc: linux-btrfs, Nikolay Borisov
Since commit 88c14590cdd6 ("btrfs: use RCU in btrfs_show_devname for
device list traversal") btrfs_show_devname no longer takes
device_list_mutex. As such the deadlock that 0ccd05285e7f ("btrfs: fix
a possible umount deadlock") aimed to fix no longer exists. So remove the
extra code this commit added. No functional changes.
This reverts commit 0ccd05285e7f5a8e297e1d6dfc41e7c65757d6fa.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
Anand,
I would like to use the opportunity to ask you about the peculiar
sequence needed to close btrfs devices. Why do we first replace the closed
device with a copy in btrfs_close_one_device, then dispose of the copied
devices in btrfs_close_devices IFF we had fs_devices->seed not being NULL?
fs/btrfs/volumes.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5bd6f3a40f9c..70b0ed2ba4df 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1004,7 +1004,7 @@ static void btrfs_close_bdev(struct btrfs_device *device)
blkdev_put(device->bdev, device->mode);
}
-static void btrfs_prepare_close_one_device(struct btrfs_device *device)
+static void btrfs_close_one_device(struct btrfs_device *device)
{
struct btrfs_fs_devices *fs_devices = device->fs_devices;
struct btrfs_device *new_device;
@@ -1022,6 +1022,8 @@ static void btrfs_prepare_close_one_device(struct btrfs_device *device)
if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
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 */
@@ -1035,39 +1037,23 @@ static void btrfs_prepare_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 close_fs_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_prepare_close_one_device(device);
- list_add(&device->dev_list, &pending_put);
+ btrfs_close_one_device(device);
}
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_rcu);
- }
-
WARN_ON(fs_devices->open_devices);
WARN_ON(fs_devices->rw_devices);
fs_devices->opened = 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Revert "btrfs: fix a possible umount deadlock"
2018-06-28 8:48 [PATCH] Revert "btrfs: fix a possible umount deadlock" Nikolay Borisov
@ 2018-06-28 21:53 ` kbuild test robot
2018-06-28 22:49 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2018-06-28 21:53 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: kbuild-all, anand.jain, linux-btrfs, Nikolay Borisov
[-- Attachment #1: Type: text/plain, Size: 2461 bytes --]
Hi Nikolay,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v4.18-rc2]
[also build test ERROR on next-20180628]
[cannot apply to btrfs/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nikolay-Borisov/Revert-btrfs-fix-a-possible-umount-deadlock/20180629-044154
config: i386-randconfig-a0-201825 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
fs//btrfs/volumes.c: In function 'btrfs_close_one_device':
>> fs//btrfs/volumes.c:1041:25: error: 'free_device' undeclared (first use in this function)
call_rcu(&device->rcu, free_device);
^
fs//btrfs/volumes.c:1041:25: note: each undeclared identifier is reported only once for each function it appears in
vim +/free_device +1041 fs//btrfs/volumes.c
1006
1007 static void btrfs_close_one_device(struct btrfs_device *device)
1008 {
1009 struct btrfs_fs_devices *fs_devices = device->fs_devices;
1010 struct btrfs_device *new_device;
1011 struct rcu_string *name;
1012
1013 if (device->bdev)
1014 fs_devices->open_devices--;
1015
1016 if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
1017 device->devid != BTRFS_DEV_REPLACE_DEVID) {
1018 list_del_init(&device->dev_alloc_list);
1019 fs_devices->rw_devices--;
1020 }
1021
1022 if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
1023 fs_devices->missing_devices--;
1024
1025 btrfs_close_bdev(device);
1026
1027 new_device = btrfs_alloc_device(NULL, &device->devid,
1028 device->uuid);
1029 BUG_ON(IS_ERR(new_device)); /* -ENOMEM */
1030
1031 /* Safe because we are under uuid_mutex */
1032 if (device->name) {
1033 name = rcu_string_strdup(device->name->str, GFP_NOFS);
1034 BUG_ON(!name); /* -ENOMEM */
1035 rcu_assign_pointer(new_device->name, name);
1036 }
1037
1038 list_replace_rcu(&device->dev_list, &new_device->dev_list);
1039 new_device->fs_devices = device->fs_devices;
1040
> 1041 call_rcu(&device->rcu, free_device);
1042 }
1043
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27668 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Revert "btrfs: fix a possible umount deadlock"
2018-06-28 8:48 [PATCH] Revert "btrfs: fix a possible umount deadlock" Nikolay Borisov
2018-06-28 21:53 ` kbuild test robot
@ 2018-06-28 22:49 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2018-06-28 22:49 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: kbuild-all, anand.jain, linux-btrfs, Nikolay Borisov
[-- Attachment #1: Type: text/plain, Size: 2525 bytes --]
Hi Nikolay,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v4.18-rc2]
[also build test ERROR on next-20180628]
[cannot apply to btrfs/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nikolay-Borisov/Revert-btrfs-fix-a-possible-umount-deadlock/20180629-044154
config: x86_64-randconfig-x015-201825 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
fs/btrfs/volumes.c: In function 'btrfs_close_one_device':
>> fs/btrfs/volumes.c:1041:25: error: 'free_device' undeclared (first use in this function); did you mean 'new_device'?
call_rcu(&device->rcu, free_device);
^~~~~~~~~~~
new_device
fs/btrfs/volumes.c:1041:25: note: each undeclared identifier is reported only once for each function it appears in
vim +1041 fs/btrfs/volumes.c
1006
1007 static void btrfs_close_one_device(struct btrfs_device *device)
1008 {
1009 struct btrfs_fs_devices *fs_devices = device->fs_devices;
1010 struct btrfs_device *new_device;
1011 struct rcu_string *name;
1012
1013 if (device->bdev)
1014 fs_devices->open_devices--;
1015
1016 if (test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) &&
1017 device->devid != BTRFS_DEV_REPLACE_DEVID) {
1018 list_del_init(&device->dev_alloc_list);
1019 fs_devices->rw_devices--;
1020 }
1021
1022 if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
1023 fs_devices->missing_devices--;
1024
1025 btrfs_close_bdev(device);
1026
1027 new_device = btrfs_alloc_device(NULL, &device->devid,
1028 device->uuid);
1029 BUG_ON(IS_ERR(new_device)); /* -ENOMEM */
1030
1031 /* Safe because we are under uuid_mutex */
1032 if (device->name) {
1033 name = rcu_string_strdup(device->name->str, GFP_NOFS);
1034 BUG_ON(!name); /* -ENOMEM */
1035 rcu_assign_pointer(new_device->name, name);
1036 }
1037
1038 list_replace_rcu(&device->dev_list, &new_device->dev_list);
1039 new_device->fs_devices = device->fs_devices;
1040
> 1041 call_rcu(&device->rcu, free_device);
1042 }
1043
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27637 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-06-28 22:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-28 8:48 [PATCH] Revert "btrfs: fix a possible umount deadlock" Nikolay Borisov
2018-06-28 21:53 ` kbuild test robot
2018-06-28 22:49 ` kbuild test robot
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.