[PATCH] Revert "btrfs: fix a possible umount deadlock"

[PATCH] Revert "btrfs: fix a possible umount deadlock"

[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

Re: [PATCH] Revert "btrfs: fix a possible umount deadlock"

[kbuild test robot]

[-- 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 --]

Re: [PATCH] Revert "btrfs: fix a possible umount deadlock"

[kbuild test robot]

[-- 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 --]