[PATCH] btrfs: hold the uuid_mutex for all close_fs_devices calls

[PATCH] btrfs: hold the uuid_mutex for all close_fs_devices calls

[Josef Bacik]

My recent change to not take the device_list_mutex for closing devices
added a lockdep_assert_held(&uuid_mutex) to close_fs_devices.  I then
went and verified all calls had that, except I overlooked
btrfs_close_devices() where we close seed devices.  Fix this by holding
the uuid_mutex for this entire operation.

20cc6d129252 ("btrfs: do not hold device_list_mutex when closing devices")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/volumes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6f489245eec6..3f8bd1af29eb 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1183,13 +1183,13 @@ void btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
 	close_fs_devices(fs_devices);
 	if (!fs_devices->opened)
 		list_splice_init(&fs_devices->seed_list, &list);
-	mutex_unlock(&uuid_mutex);
 
 	list_for_each_entry_safe(fs_devices, tmp, &list, seed_list) {
 		close_fs_devices(fs_devices);
 		list_del(&fs_devices->seed_list);
 		free_fs_devices(fs_devices);
 	}
+	mutex_unlock(&uuid_mutex);
 }
 
 static int open_fs_devices(struct btrfs_fs_devices *fs_devices,
-- 
2.24.1

Re: [PATCH] btrfs: hold the uuid_mutex for all close_fs_devices calls

[Nikolay Borisov]

On 28.08.20 г. 17:24 ч., Josef Bacik wrote:
> My recent change to not take the device_list_mutex for closing devices
> added a lockdep_assert_held(&uuid_mutex) to close_fs_devices.  I then
> went and verified all calls had that, except I overlooked
> btrfs_close_devices() where we close seed devices.  Fix this by holding
> the uuid_mutex for this entire operation.
> 
> 20cc6d129252 ("btrfs: do not hold device_list_mutex when closing devices")
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>


> ---
>  fs/btrfs/volumes.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6f489245eec6..3f8bd1af29eb 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1183,13 +1183,13 @@ void btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
>  	close_fs_devices(fs_devices);
>  	if (!fs_devices->opened)
>  		list_splice_init(&fs_devices->seed_list, &list);
> -	mutex_unlock(&uuid_mutex);
>  
>  	list_for_each_entry_safe(fs_devices, tmp, &list, seed_list) {
>  		close_fs_devices(fs_devices);
>  		list_del(&fs_devices->seed_list);
>  		free_fs_devices(fs_devices);
>  	}
> +	mutex_unlock(&uuid_mutex);
>  }
>  
>  static int open_fs_devices(struct btrfs_fs_devices *fs_devices,
> 

Re: [PATCH] btrfs: hold the uuid_mutex for all close_fs_devices calls

[David Sterba]

On Fri, Aug 28, 2020 at 10:24:57AM -0400, Josef Bacik wrote:
> My recent change to not take the device_list_mutex for closing devices
> added a lockdep_assert_held(&uuid_mutex) to close_fs_devices.  I then
> went and verified all calls had that, except I overlooked
> btrfs_close_devices() where we close seed devices.  Fix this by holding
> the uuid_mutex for this entire operation.
> 
> 20cc6d129252 ("btrfs: do not hold device_list_mutex when closing devices")

This is still in misc-next so I won't apply it as a separate patch.
You can eithere send a v2 of the patch or send it as an "improper" patch
with reference to the patch that needs to be updated with the diff.

Re: [PATCH] btrfs: hold the uuid_mutex for all close_fs_devices calls

[Anand Jain]

On 28/8/20 10:24 pm, Josef Bacik wrote:
> My recent change to not take the device_list_mutex for closing devices
> added a lockdep_assert_held(&uuid_mutex) to close_fs_devices.  I then
> went and verified all calls had that, except I overlooked
> btrfs_close_devices() where we close seed devices.  Fix this by holding
> the uuid_mutex for this entire operation.
> 
> 20cc6d129252 ("btrfs: do not hold device_list_mutex when closing devices")
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>   fs/btrfs/volumes.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6f489245eec6..3f8bd1af29eb 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1183,13 +1183,13 @@ void btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
>   	close_fs_devices(fs_devices);
>   	if (!fs_devices->opened)
>   		list_splice_init(&fs_devices->seed_list, &list);
> -	mutex_unlock(&uuid_mutex);
>   
>   	list_for_each_entry_safe(fs_devices, tmp, &list, seed_list) {
>   		close_fs_devices(fs_devices);
>   		list_del(&fs_devices->seed_list);
>   		free_fs_devices(fs_devices);
>   	}
> +	mutex_unlock(&uuid_mutex);
>   }
>   
>   static int open_fs_devices(struct btrfs_fs_devices *fs_devices,
> 

Reviewed-by: Anand Jain <anand.jain@oracle.com>
Thanks, Anand