[PATCH v3 0/4] btrf_show_devname related fixes

[PATCH v3 0/4] btrf_show_devname related fixes

[Anand Jain]

These fixes are inspired by the bug report and its discussions in the
mailing list subject
 btrfs: traverse seed devices if fs_devices::devices is empty in show_devname

v3:
 Fix rcu_lock in the patch 3/4

Anand Jain (4):
  btrfs: consolidate device_list_mutex in prepare_sprout to its parent
  btrfs: save latest btrfs_device instead of its block_device in
    fs_devices
  btrfs: use latest_dev in btrfs_show_devname
  btrfs: update latest_dev when we sprout

 fs/btrfs/disk-io.c   |  6 +++---
 fs/btrfs/extent_io.c |  2 +-
 fs/btrfs/inode.c     |  2 +-
 fs/btrfs/procfs.c    |  6 +++---
 fs/btrfs/super.c     | 26 ++++----------------------
 fs/btrfs/volumes.c   | 19 +++++++++++--------
 fs/btrfs/volumes.h   |  2 +-
 7 files changed, 24 insertions(+), 39 deletions(-)

-- 
2.31.1

[PATCH v3 1/4] btrfs: consolidate device_list_mutex in prepare_sprout to its parent

[Anand Jain]

btrfs_prepare_sprout() moves seed devices into its own struct fs_devices,
so that its parent function btrfs_init_new_device() can add the new sprout
device to fs_info->fs_devices.

Both btrfs_prepare_sprout() and btrfs_init_new_device() needs
device_list_mutex. But they are holding it sequentially, thus creates a
small window to an opportunity to race. Close this opportunity and hold
device_list_mutex common to both btrfs_init_new_device() and
btrfs_prepare_sprout().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
RFC because I haven't identified the other thread which could race with
this, but still does this cleanup makes sense?

v2: fix the missing mutex_unlock in the error return
v3: -

 fs/btrfs/volumes.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index e7295ec3865e..51cf68785782 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2366,6 +2366,8 @@ static int btrfs_prepare_sprout(struct btrfs_fs_info *fs_info)
 	u64 super_flags;
 
 	lockdep_assert_held(&uuid_mutex);
+	lockdep_assert_held(&fs_devices->device_list_mutex);
+
 	if (!fs_devices->seeding)
 		return -EINVAL;
 
@@ -2397,7 +2399,6 @@ static int btrfs_prepare_sprout(struct btrfs_fs_info *fs_info)
 	INIT_LIST_HEAD(&seed_devices->alloc_list);
 	mutex_init(&seed_devices->device_list_mutex);
 
-	mutex_lock(&fs_devices->device_list_mutex);
 	list_splice_init_rcu(&fs_devices->devices, &seed_devices->devices,
 			      synchronize_rcu);
 	list_for_each_entry(device, &seed_devices->devices, dev_list)
@@ -2413,7 +2414,6 @@ static int btrfs_prepare_sprout(struct btrfs_fs_info *fs_info)
 	generate_random_uuid(fs_devices->fsid);
 	memcpy(fs_devices->metadata_uuid, fs_devices->fsid, BTRFS_FSID_SIZE);
 	memcpy(disk_super->fsid, fs_devices->fsid, BTRFS_FSID_SIZE);
-	mutex_unlock(&fs_devices->device_list_mutex);
 
 	super_flags = btrfs_super_flags(disk_super) &
 		      ~BTRFS_SUPER_FLAG_SEEDING;
@@ -2588,10 +2588,12 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	device->dev_stats_valid = 1;
 	set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
 
+	mutex_lock(&fs_devices->device_list_mutex);
 	if (seeding_dev) {
 		btrfs_clear_sb_rdonly(sb);
 		ret = btrfs_prepare_sprout(fs_info);
 		if (ret) {
+			mutex_unlock(&fs_devices->device_list_mutex);
 			btrfs_abort_transaction(trans, ret);
 			goto error_trans;
 		}
@@ -2599,7 +2601,6 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 
 	device->fs_devices = fs_devices;
 
-	mutex_lock(&fs_devices->device_list_mutex);
 	mutex_lock(&fs_info->chunk_mutex);
 	list_add_rcu(&device->dev_list, &fs_devices->devices);
 	list_add(&device->dev_alloc_list, &fs_devices->alloc_list);
-- 
2.31.1

[PATCH RFC v4 1/4] btrfs: consolidate device_list_mutex in prepare_sprout to its parent

[Anand Jain]

btrfs_prepare_sprout() moves seed devices into its own struct fs_devices,
so that its parent function btrfs_init_new_device() can add the new sprout
device to fs_info->fs_devices.

Both btrfs_prepare_sprout() and btrfs_init_new_device() needs
device_list_mutex. But they are holding it sequentially, thus creates a
small window to an opportunity to race. Close this opportunity and hold
device_list_mutex common to both btrfs_init_new_device() and
btrfs_prepare_sprout().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
RFC because I haven't identified the other thread which could race with
this (if any), but still does this cleanup makes sense?

btrfs_prepare_sprout() calls clone_fs_devices() which holds
device_list_mutex. So this patch also needs the following
cleanup patch in the ML, that removed device_list_mutex from
clone_fs_devices().
 [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs

v2: fix the missing mutex_unlock in the error return
v3: -
v4: -

 fs/btrfs/volumes.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index e7295ec3865e..51cf68785782 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2366,6 +2366,8 @@ static int btrfs_prepare_sprout(struct btrfs_fs_info *fs_info)
 	u64 super_flags;
 
 	lockdep_assert_held(&uuid_mutex);
+	lockdep_assert_held(&fs_devices->device_list_mutex);
+
 	if (!fs_devices->seeding)
 		return -EINVAL;
 
@@ -2397,7 +2399,6 @@ static int btrfs_prepare_sprout(struct btrfs_fs_info *fs_info)
 	INIT_LIST_HEAD(&seed_devices->alloc_list);
 	mutex_init(&seed_devices->device_list_mutex);
 
-	mutex_lock(&fs_devices->device_list_mutex);
 	list_splice_init_rcu(&fs_devices->devices, &seed_devices->devices,
 			      synchronize_rcu);
 	list_for_each_entry(device, &seed_devices->devices, dev_list)
@@ -2413,7 +2414,6 @@ static int btrfs_prepare_sprout(struct btrfs_fs_info *fs_info)
 	generate_random_uuid(fs_devices->fsid);
 	memcpy(fs_devices->metadata_uuid, fs_devices->fsid, BTRFS_FSID_SIZE);
 	memcpy(disk_super->fsid, fs_devices->fsid, BTRFS_FSID_SIZE);
-	mutex_unlock(&fs_devices->device_list_mutex);
 
 	super_flags = btrfs_super_flags(disk_super) &
 		      ~BTRFS_SUPER_FLAG_SEEDING;
@@ -2588,10 +2588,12 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	device->dev_stats_valid = 1;
 	set_blocksize(device->bdev, BTRFS_BDEV_BLOCKSIZE);
 
+	mutex_lock(&fs_devices->device_list_mutex);
 	if (seeding_dev) {
 		btrfs_clear_sb_rdonly(sb);
 		ret = btrfs_prepare_sprout(fs_info);
 		if (ret) {
+			mutex_unlock(&fs_devices->device_list_mutex);
 			btrfs_abort_transaction(trans, ret);
 			goto error_trans;
 		}
@@ -2599,7 +2601,6 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 
 	device->fs_devices = fs_devices;
 
-	mutex_lock(&fs_devices->device_list_mutex);
 	mutex_lock(&fs_info->chunk_mutex);
 	list_add_rcu(&device->dev_list, &fs_devices->devices);
 	list_add(&device->dev_alloc_list, &fs_devices->alloc_list);
-- 
2.31.1

[PATCH v3 2/4] btrfs: save latest btrfs_device instead of its block_device in fs_devices

[Anand Jain]

In preparation to fix a bug in btrfs_show_devname(), save the device with
the largest generation in fs_info instead of just its bdev. So that
btrfs_show_devname() can read device's name.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: born
v3: -
 fs/btrfs/disk-io.c   |  6 +++---
 fs/btrfs/extent_io.c |  2 +-
 fs/btrfs/inode.c     |  2 +-
 fs/btrfs/procfs.c    |  6 +++---
 fs/btrfs/super.c     |  2 +-
 fs/btrfs/volumes.c   | 10 +++++-----
 fs/btrfs/volumes.h   |  2 +-
 7 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1052437cec64..c0d2c093b874 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3228,12 +3228,12 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	mapping_set_gfp_mask(fs_info->btree_inode->i_mapping, GFP_NOFS);
 	btrfs_init_btree_inode(fs_info);
 
-	invalidate_bdev(fs_devices->latest_bdev);
+	invalidate_bdev(fs_devices->latest_dev->bdev);
 
 	/*
 	 * Read super block and check the signature bytes only
 	 */
-	disk_super = btrfs_read_dev_super(fs_devices->latest_bdev);
+	disk_super = btrfs_read_dev_super(fs_devices->latest_dev->bdev);
 	if (IS_ERR(disk_super)) {
 		err = PTR_ERR(disk_super);
 		goto fail_alloc;
@@ -3466,7 +3466,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	 * below in btrfs_init_dev_replace().
 	 */
 	btrfs_free_extra_devids(fs_devices);
-	if (!fs_devices->latest_bdev) {
+	if (!fs_devices->latest_dev->bdev) {
 		btrfs_err(fs_info, "failed to read devices");
 		goto fail_tree_roots;
 	}
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index aaddd7225348..edf0162c9020 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3327,7 +3327,7 @@ static int alloc_new_bio(struct btrfs_inode *inode,
 	if (wbc) {
 		struct block_device *bdev;
 
-		bdev = fs_info->fs_devices->latest_bdev;
+		bdev = fs_info->fs_devices->latest_dev->bdev;
 		bio_set_dev(bio, bdev);
 		wbc_init_bio(wbc, bio);
 	}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2aa9646bce56..ceedcd54e6d2 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -7961,7 +7961,7 @@ static int btrfs_dio_iomap_begin(struct inode *inode, loff_t start,
 		iomap->type = IOMAP_MAPPED;
 	}
 	iomap->offset = start;
-	iomap->bdev = fs_info->fs_devices->latest_bdev;
+	iomap->bdev = fs_info->fs_devices->latest_dev->bdev;
 	iomap->length = len;
 
 	if (write && btrfs_use_zone_append(BTRFS_I(inode), em->block_start))
diff --git a/fs/btrfs/procfs.c b/fs/btrfs/procfs.c
index 30eaeca07aeb..2c3bb474c28f 100644
--- a/fs/btrfs/procfs.c
+++ b/fs/btrfs/procfs.c
@@ -291,9 +291,9 @@ void btrfs_print_fsinfo(struct seq_file *seq)
 					bdevname(fs_info->sb->s_bdev, b) :
 					"null");
 		BTRFS_SEQ_PRINT2("\tlatest_bdev:\t\t%s\n",
-				fs_devices->latest_bdev ?
-					bdevname(fs_devices->latest_bdev, b) :
-					"null");
+				  fs_devices->latest_dev->bdev ?
+				  bdevname(fs_devices->latest_dev->bdev, b) :
+				  "null");
 
 		fs_state_to_str(fs_info, fs_str);
 		BTRFS_SEQ_PRINT2("\tfs_state:\t\t%s\n", fs_str);
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 1f9dd1a4faa3..64ecbdb50c1a 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1706,7 +1706,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
 		goto error_close_devices;
 	}
 
-	bdev = fs_devices->latest_bdev;
+	bdev = fs_devices->latest_dev->bdev;
 	s = sget(fs_type, btrfs_test_super, btrfs_set_super, flags | SB_NOSEC,
 		 fs_info);
 	if (IS_ERR(s)) {
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 51cf68785782..958503c8a854 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1091,7 +1091,7 @@ void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices)
 	list_for_each_entry(seed_dev, &fs_devices->seed_list, seed_list)
 		__btrfs_free_extra_devids(seed_dev, &latest_dev);
 
-	fs_devices->latest_bdev = latest_dev->bdev;
+	fs_devices->latest_dev = latest_dev;
 
 	mutex_unlock(&uuid_mutex);
 }
@@ -1206,7 +1206,7 @@ static int open_fs_devices(struct btrfs_fs_devices *fs_devices,
 		return -EINVAL;
 
 	fs_devices->opened = 1;
-	fs_devices->latest_bdev = latest_dev->bdev;
+	fs_devices->latest_dev = latest_dev;
 	fs_devices->total_rw_bytes = 0;
 	fs_devices->chunk_alloc_policy = BTRFS_CHUNK_ALLOC_REGULAR;
 	fs_devices->read_policy = BTRFS_READ_POLICY_PID;
@@ -1968,7 +1968,7 @@ static struct btrfs_device * btrfs_find_next_active_device(
 }
 
 /*
- * Helper function to check if the given device is part of s_bdev / latest_bdev
+ * Helper function to check if the given device is part of s_bdev / latest_dev
  * and replace it with the provided or the next active device, in the context
  * where this function called, there should be always be another device (or
  * this_dev) which is active.
@@ -1987,8 +1987,8 @@ void __cold btrfs_assign_next_active_device(struct btrfs_device *device,
 			(fs_info->sb->s_bdev == device->bdev))
 		fs_info->sb->s_bdev = next_device->bdev;
 
-	if (fs_info->fs_devices->latest_bdev == device->bdev)
-		fs_info->fs_devices->latest_bdev = next_device->bdev;
+	if (fs_info->fs_devices->latest_dev->bdev == device->bdev)
+		fs_info->fs_devices->latest_dev = next_device;
 }
 
 /*
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 4c941b4dd269..150b4cd8f81f 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -246,7 +246,7 @@ struct btrfs_fs_devices {
 	/* Highest generation number of seen devices */
 	u64 latest_generation;
 
-	struct block_device *latest_bdev;
+	struct btrfs_device *latest_dev;
 
 	/* all of the devices in the FS, protected by a mutex
 	 * so we can safely walk it to write out the supers without
-- 
2.31.1

Re: [PATCH v3 2/4] btrfs: save latest btrfs_device instead of its block_device in fs_devices

[Su Yue]

On Fri 20 Aug 2021 at 19:28, Anand Jain <anand.jain@oracle.com> 
wrote:

> In preparation to fix a bug in btrfs_show_devname(), save the 
> device with
> the largest generation in fs_info instead of just its bdev. So 
> that
> btrfs_show_devname() can read device's name.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v2: born
> v3: -
>  fs/btrfs/disk-io.c   |  6 +++---
>  fs/btrfs/extent_io.c |  2 +-
>  fs/btrfs/inode.c     |  2 +-
>  fs/btrfs/procfs.c    |  6 +++---
>

Try to test the patchset but...
I can't find procfs in Linus/master, kdave/misc-next, v5.14-rc6 
even
through Google search.

--
Su

>  fs/btrfs/super.c     |  2 +-
>  fs/btrfs/volumes.c   | 10 +++++-----
>  fs/btrfs/volumes.h   |  2 +-
>  7 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 1052437cec64..c0d2c093b874 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3228,12 +3228,12 @@ int __cold open_ctree(struct super_block 
> *sb, struct btrfs_fs_devices *fs_device
>  	mapping_set_gfp_mask(fs_info->btree_inode->i_mapping, 
>  GFP_NOFS);
>  	btrfs_init_btree_inode(fs_info);
>
> -	invalidate_bdev(fs_devices->latest_bdev);
> +	invalidate_bdev(fs_devices->latest_dev->bdev);
>
>  	/*
>  	 * Read super block and check the signature bytes only
>  	 */
> -	disk_super = btrfs_read_dev_super(fs_devices->latest_bdev);
> +	disk_super = 
> btrfs_read_dev_super(fs_devices->latest_dev->bdev);
>  	if (IS_ERR(disk_super)) {
>  		err = PTR_ERR(disk_super);
>  		goto fail_alloc;
> @@ -3466,7 +3466,7 @@ int __cold open_ctree(struct super_block 
> *sb, struct btrfs_fs_devices *fs_device
>  	 * below in btrfs_init_dev_replace().
>  	 */
>  	btrfs_free_extra_devids(fs_devices);
> -	if (!fs_devices->latest_bdev) {
> +	if (!fs_devices->latest_dev->bdev) {
>  		btrfs_err(fs_info, "failed to read devices");
>  		goto fail_tree_roots;
>  	}
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index aaddd7225348..edf0162c9020 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -3327,7 +3327,7 @@ static int alloc_new_bio(struct 
> btrfs_inode *inode,
>  	if (wbc) {
>  		struct block_device *bdev;
>
> -		bdev = fs_info->fs_devices->latest_bdev;
> +		bdev = fs_info->fs_devices->latest_dev->bdev;
>  		bio_set_dev(bio, bdev);
>  		wbc_init_bio(wbc, bio);
>  	}
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 2aa9646bce56..ceedcd54e6d2 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7961,7 +7961,7 @@ static int btrfs_dio_iomap_begin(struct 
> inode *inode, loff_t start,
>  		iomap->type = IOMAP_MAPPED;
>  	}
>  	iomap->offset = start;
> -	iomap->bdev = fs_info->fs_devices->latest_bdev;
> +	iomap->bdev = fs_info->fs_devices->latest_dev->bdev;
>  	iomap->length = len;
>
>  	if (write && btrfs_use_zone_append(BTRFS_I(inode), 
>  em->block_start))
> diff --git a/fs/btrfs/procfs.c b/fs/btrfs/procfs.c
> index 30eaeca07aeb..2c3bb474c28f 100644
> --- a/fs/btrfs/procfs.c
> +++ b/fs/btrfs/procfs.c
> @@ -291,9 +291,9 @@ void btrfs_print_fsinfo(struct seq_file 
> *seq)
>  					bdevname(fs_info->sb->s_bdev, b) :
>  					"null");
>  		BTRFS_SEQ_PRINT2("\tlatest_bdev:\t\t%s\n",
> -				fs_devices->latest_bdev ?
> -					bdevname(fs_devices->latest_bdev, b) :
> -					"null");
> +				  fs_devices->latest_dev->bdev ?
> +				  bdevname(fs_devices->latest_dev->bdev, b) :
> +				  "null");
>
>  		fs_state_to_str(fs_info, fs_str);
>  		BTRFS_SEQ_PRINT2("\tfs_state:\t\t%s\n", fs_str);
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 1f9dd1a4faa3..64ecbdb50c1a 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1706,7 +1706,7 @@ static struct dentry 
> *btrfs_mount_root(struct file_system_type *fs_type,
>  		goto error_close_devices;
>  	}
>
> -	bdev = fs_devices->latest_bdev;
> +	bdev = fs_devices->latest_dev->bdev;
>  	s = sget(fs_type, btrfs_test_super, btrfs_set_super, flags | 
>  SB_NOSEC,
>  		 fs_info);
>  	if (IS_ERR(s)) {
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 51cf68785782..958503c8a854 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1091,7 +1091,7 @@ void btrfs_free_extra_devids(struct 
> btrfs_fs_devices *fs_devices)
>  	list_for_each_entry(seed_dev, &fs_devices->seed_list, 
>  seed_list)
>  		__btrfs_free_extra_devids(seed_dev, &latest_dev);
>
> -	fs_devices->latest_bdev = latest_dev->bdev;
> +	fs_devices->latest_dev = latest_dev;
>
>  	mutex_unlock(&uuid_mutex);
>  }
> @@ -1206,7 +1206,7 @@ static int open_fs_devices(struct 
> btrfs_fs_devices *fs_devices,
>  		return -EINVAL;
>
>  	fs_devices->opened = 1;
> -	fs_devices->latest_bdev = latest_dev->bdev;
> +	fs_devices->latest_dev = latest_dev;
>  	fs_devices->total_rw_bytes = 0;
>  	fs_devices->chunk_alloc_policy = BTRFS_CHUNK_ALLOC_REGULAR;
>  	fs_devices->read_policy = BTRFS_READ_POLICY_PID;
> @@ -1968,7 +1968,7 @@ static struct btrfs_device * 
> btrfs_find_next_active_device(
>  }
>
>  /*
> - * Helper function to check if the given device is part of 
> s_bdev / latest_bdev
> + * Helper function to check if the given device is part of 
> s_bdev / latest_dev
>   * and replace it with the provided or the next active device, 
>   in the context
>   * where this function called, there should be always be 
>   another device (or
>   * this_dev) which is active.
> @@ -1987,8 +1987,8 @@ void __cold 
> btrfs_assign_next_active_device(struct btrfs_device *device,
>  			(fs_info->sb->s_bdev == device->bdev))
>  		fs_info->sb->s_bdev = next_device->bdev;
>
> -	if (fs_info->fs_devices->latest_bdev == device->bdev)
> -		fs_info->fs_devices->latest_bdev = next_device->bdev;
> +	if (fs_info->fs_devices->latest_dev->bdev == device->bdev)
> +		fs_info->fs_devices->latest_dev = next_device;
>  }
>
>  /*
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 4c941b4dd269..150b4cd8f81f 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -246,7 +246,7 @@ struct btrfs_fs_devices {
>  	/* Highest generation number of seen devices */
>  	u64 latest_generation;
>
> -	struct block_device *latest_bdev;
> +	struct btrfs_device *latest_dev;
>
>  	/* all of the devices in the FS, protected by a mutex
>  	 * so we can safely walk it to write out the supers without

Re: [PATCH v3 2/4] btrfs: save latest btrfs_device instead of its block_device in fs_devices

[Anand Jain]

On 21/8/21 10:46 pm, Su Yue wrote:
> 
> On Fri 20 Aug 2021 at 19:28, Anand Jain <anand.jain@oracle.com> wrote:
> 
>> In preparation to fix a bug in btrfs_show_devname(), save the device with
>> the largest generation in fs_info instead of just its bdev. So that
>> btrfs_show_devname() can read device's name.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> v2: born
>> v3: -
>>  fs/btrfs/disk-io.c   |  6 +++---
>>  fs/btrfs/extent_io.c |  2 +-
>>  fs/btrfs/inode.c     |  2 +-
>>  fs/btrfs/procfs.c    |  6 +++---

  Oops. It got in my boilerplate codes.
  I will fix and send v4.

Thanks, Anand

[PATCH v3 3/4] btrfs: use latest_dev in btrfs_show_devname

[Anand Jain]

latest_dev is updated according to the changes to the device list.
That means we could use the latest_dev->name to show the device name in
/proc/self/mounts. So this patch makes that change.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
RFC because,
With this patch, /proc/self/mounts might not show the lowest devid
device as we did before.  We show the device that has the greatest
generation and, we used it to build the tree. Are we ok with this change
and, it won't affect the ABI? IMO it should be ok.
 
v2 use latest_dev so that device path is also shown
v3 add missing rcu_lock in show_devname

 fs/btrfs/super.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 64ecbdb50c1a..61682a143bf3 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2464,30 +2464,12 @@ static int btrfs_unfreeze(struct super_block *sb)
 static int btrfs_show_devname(struct seq_file *m, struct dentry *root)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(root->d_sb);
-	struct btrfs_device *dev, *first_dev = NULL;
 
-	/*
-	 * Lightweight locking of the devices. We should not need
-	 * device_list_mutex here as we only read the device data and the list
-	 * is protected by RCU.  Even if a device is deleted during the list
-	 * traversals, we'll get valid data, the freeing callback will wait at
-	 * least until the rcu_read_unlock.
-	 */
 	rcu_read_lock();
-	list_for_each_entry_rcu(dev, &fs_info->fs_devices->devices, dev_list) {
-		if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state))
-			continue;
-		if (!dev->name)
-			continue;
-		if (!first_dev || dev->devid < first_dev->devid)
-			first_dev = dev;
-	}
-
-	if (first_dev)
-		seq_escape(m, rcu_str_deref(first_dev->name), " \t\n\\");
-	else
-		WARN_ON(1);
+	seq_escape(m, rcu_str_deref(fs_info->fs_devices->latest_dev->name),
+		   " \t\n\\");
 	rcu_read_unlock();
+
 	return 0;
 }
 
-- 
2.31.1

[PATCH RFC v4 3/4] btrfs: use latest_dev in btrfs_show_devname

[Anand Jain]

latest_dev is updated according to the changes to the device list.
That means we could use the latest_dev->name to show the device name in
/proc/self/mounts. So this patch makes that change.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
RFC because,
With this patch, /proc/self/mounts might not show the lowest devid
device as we did before.  We show the device that has the greatest
generation and, we used it to build the tree. Are we ok with this change
and, it won't affect the ABI? IMO it should be ok.
 
v2 use latest_dev so that device path is also shown
v3 add missing rcu_lock in show_devname
v4 -

 fs/btrfs/super.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 64ecbdb50c1a..61682a143bf3 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2464,30 +2464,12 @@ static int btrfs_unfreeze(struct super_block *sb)
 static int btrfs_show_devname(struct seq_file *m, struct dentry *root)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(root->d_sb);
-	struct btrfs_device *dev, *first_dev = NULL;
 
-	/*
-	 * Lightweight locking of the devices. We should not need
-	 * device_list_mutex here as we only read the device data and the list
-	 * is protected by RCU.  Even if a device is deleted during the list
-	 * traversals, we'll get valid data, the freeing callback will wait at
-	 * least until the rcu_read_unlock.
-	 */
 	rcu_read_lock();
-	list_for_each_entry_rcu(dev, &fs_info->fs_devices->devices, dev_list) {
-		if (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state))
-			continue;
-		if (!dev->name)
-			continue;
-		if (!first_dev || dev->devid < first_dev->devid)
-			first_dev = dev;
-	}
-
-	if (first_dev)
-		seq_escape(m, rcu_str_deref(first_dev->name), " \t\n\\");
-	else
-		WARN_ON(1);
+	seq_escape(m, rcu_str_deref(fs_info->fs_devices->latest_dev->name),
+		   " \t\n\\");
 	rcu_read_unlock();
+
 	return 0;
 }
 
-- 
2.31.1

[PATCH v3 4/4] btrfs: update latest_dev when we sprout

[Anand Jain]

When we add a device to the seed filesystem (sprouting) it is a new
filesystem (and fsid) on the device added. Update the latest_device so
that /proc/self/mounts shows the correct device.

For example:
 $ btrfstune -S1 /dev/vg/scratch1
 $ mount /dev/vg/scratch1 /btrfs
 mount: /btrfs: WARNING: device write-protected, mounted read-only.

 $ cat /proc/self/mounts | grep btrfs
 /dev/mapper/vg-scratch1 /btrfs btrfs ro,relatime,space_cache,subvolid=5,subvol=/ 0 0

 $ btrfs dev add -f /dev/vg/scratch0 /btrfs

Before:
 $ cat /proc/self/mounts | grep btrfs
 /dev/mapper/vg-scratch1 /btrfs btrfs ro,relatime,space_cache,subvolid=5,subvol=/ 0 0

After:
 $ cat /proc/self/mounts | grep btrfs
 /dev/mapper/vg-scratch0 /btrfs btrfs ro,relatime,space_cache,subvolid=5,subvol=/ 0 0

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: born
v3: -

 fs/btrfs/volumes.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 958503c8a854..1d1204547e72 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2597,6 +2597,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 			btrfs_abort_transaction(trans, ret);
 			goto error_trans;
 		}
+		btrfs_assign_next_active_device(fs_info->fs_devices->latest_dev,
+						device);
 	}
 
 	device->fs_devices = fs_devices;
-- 
2.31.1

[PATCH v4 4/4] btrfs: update latest_dev when we sprout

[Anand Jain]

When we add a device to the seed filesystem (sprouting) it is a new
filesystem (and fsid) on the device added. Update the latest_device so
that /proc/self/mounts shows the correct device.

For example:
 $ btrfstune -S1 /dev/vg/scratch1
 $ mount /dev/vg/scratch1 /btrfs
 mount: /btrfs: WARNING: device write-protected, mounted read-only.

 $ cat /proc/self/mounts | grep btrfs
 /dev/mapper/vg-scratch1 /btrfs btrfs ro,relatime,space_cache,subvolid=5,subvol=/ 0 0

 $ btrfs dev add -f /dev/vg/scratch0 /btrfs

Before:
 $ cat /proc/self/mounts | grep btrfs
 /dev/mapper/vg-scratch1 /btrfs btrfs ro,relatime,space_cache,subvolid=5,subvol=/ 0 0

After:
 $ cat /proc/self/mounts | grep btrfs
 /dev/mapper/vg-scratch0 /btrfs btrfs ro,relatime,space_cache,subvolid=5,subvol=/ 0 0

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: born
v3: -
v4: -

 fs/btrfs/volumes.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 958503c8a854..1d1204547e72 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2597,6 +2597,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 			btrfs_abort_transaction(trans, ret);
 			goto error_trans;
 		}
+		btrfs_assign_next_active_device(fs_info->fs_devices->latest_dev,
+						device);
 	}
 
 	device->fs_devices = fs_devices;
-- 
2.31.1

Re: [PATCH v3 0/4] btrf_show_devname related fixes

[Anand Jain]

Please consider patches 1/4 and 3/4 as RFC.
The reason for the RFC is in the individual patches.

Thanks.

On 20/08/2021 19:28, Anand Jain wrote:
> These fixes are inspired by the bug report and its discussions in the
> mailing list subject
>   btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
> 
> v3:
>   Fix rcu_lock in the patch 3/4
> 
> Anand Jain (4):
>    btrfs: consolidate device_list_mutex in prepare_sprout to its parent
>    btrfs: save latest btrfs_device instead of its block_device in
>      fs_devices
>    btrfs: use latest_dev in btrfs_show_devname
>    btrfs: update latest_dev when we sprout
> 
>   fs/btrfs/disk-io.c   |  6 +++---
>   fs/btrfs/extent_io.c |  2 +-
>   fs/btrfs/inode.c     |  2 +-
>   fs/btrfs/procfs.c    |  6 +++---
>   fs/btrfs/super.c     | 26 ++++----------------------
>   fs/btrfs/volumes.c   | 19 +++++++++++--------
>   fs/btrfs/volumes.h   |  2 +-
>   7 files changed, 24 insertions(+), 39 deletions(-)
> 

[PATCH v4 0/4] btrf_show_devname related fixes

[Anand Jain]

These fixes are inspired by the bug report and its discussions in the
mailing list subject
 btrfs: traverse seed devices if fs_devices::devices is empty in show_devname

And depends on the patch
 [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
in the ML

v4:
 Fix unrelated changes in 2/4
v3:
 Fix rcu_lock in the patch 3/4

Anand Jain (4):
  btrfs: consolidate device_list_mutex in prepare_sprout to its parent
  btrfs: save latest btrfs_device instead of its block_device in
    fs_devices
  btrfs: use latest_dev in btrfs_show_devname
  btrfs: update latest_dev when we sprout

 fs/btrfs/disk-io.c   |  6 +++---
 fs/btrfs/extent_io.c |  2 +-
 fs/btrfs/inode.c     |  2 +-
 fs/btrfs/procfs.c    |  6 +++---
 fs/btrfs/super.c     | 26 ++++----------------------
 fs/btrfs/volumes.c   | 19 +++++++++++--------
 fs/btrfs/volumes.h   |  2 +-
 7 files changed, 24 insertions(+), 39 deletions(-)

-- 
2.31.1

[PATCH v4 2/4] btrfs: save latest btrfs_device instead of its block_device in fs_devices

[Anand Jain]

In preparation to fix a bug in btrfs_show_devname(), save the device with
the largest generation in fs_info instead of just its bdev. So that
btrfs_show_devname() can read device's name.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: born
v3: -
v4: remove changes that do not belong to this patch

 fs/btrfs/disk-io.c   |  6 +++---
 fs/btrfs/extent_io.c |  2 +-
 fs/btrfs/inode.c     |  2 +-
 fs/btrfs/super.c     |  2 +-
 fs/btrfs/volumes.c   | 10 +++++-----
 fs/btrfs/volumes.h   |  2 +-
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1052437cec64..c0d2c093b874 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3228,12 +3228,12 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	mapping_set_gfp_mask(fs_info->btree_inode->i_mapping, GFP_NOFS);
 	btrfs_init_btree_inode(fs_info);
 
-	invalidate_bdev(fs_devices->latest_bdev);
+	invalidate_bdev(fs_devices->latest_dev->bdev);
 
 	/*
 	 * Read super block and check the signature bytes only
 	 */
-	disk_super = btrfs_read_dev_super(fs_devices->latest_bdev);
+	disk_super = btrfs_read_dev_super(fs_devices->latest_dev->bdev);
 	if (IS_ERR(disk_super)) {
 		err = PTR_ERR(disk_super);
 		goto fail_alloc;
@@ -3466,7 +3466,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	 * below in btrfs_init_dev_replace().
 	 */
 	btrfs_free_extra_devids(fs_devices);
-	if (!fs_devices->latest_bdev) {
+	if (!fs_devices->latest_dev->bdev) {
 		btrfs_err(fs_info, "failed to read devices");
 		goto fail_tree_roots;
 	}
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index aaddd7225348..edf0162c9020 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3327,7 +3327,7 @@ static int alloc_new_bio(struct btrfs_inode *inode,
 	if (wbc) {
 		struct block_device *bdev;
 
-		bdev = fs_info->fs_devices->latest_bdev;
+		bdev = fs_info->fs_devices->latest_dev->bdev;
 		bio_set_dev(bio, bdev);
 		wbc_init_bio(wbc, bio);
 	}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2aa9646bce56..ceedcd54e6d2 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -7961,7 +7961,7 @@ static int btrfs_dio_iomap_begin(struct inode *inode, loff_t start,
 		iomap->type = IOMAP_MAPPED;
 	}
 	iomap->offset = start;
-	iomap->bdev = fs_info->fs_devices->latest_bdev;
+	iomap->bdev = fs_info->fs_devices->latest_dev->bdev;
 	iomap->length = len;
 
 	if (write && btrfs_use_zone_append(BTRFS_I(inode), em->block_start))
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 1f9dd1a4faa3..64ecbdb50c1a 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1706,7 +1706,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
 		goto error_close_devices;
 	}
 
-	bdev = fs_devices->latest_bdev;
+	bdev = fs_devices->latest_dev->bdev;
 	s = sget(fs_type, btrfs_test_super, btrfs_set_super, flags | SB_NOSEC,
 		 fs_info);
 	if (IS_ERR(s)) {
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 51cf68785782..958503c8a854 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1091,7 +1091,7 @@ void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices)
 	list_for_each_entry(seed_dev, &fs_devices->seed_list, seed_list)
 		__btrfs_free_extra_devids(seed_dev, &latest_dev);
 
-	fs_devices->latest_bdev = latest_dev->bdev;
+	fs_devices->latest_dev = latest_dev;
 
 	mutex_unlock(&uuid_mutex);
 }
@@ -1206,7 +1206,7 @@ static int open_fs_devices(struct btrfs_fs_devices *fs_devices,
 		return -EINVAL;
 
 	fs_devices->opened = 1;
-	fs_devices->latest_bdev = latest_dev->bdev;
+	fs_devices->latest_dev = latest_dev;
 	fs_devices->total_rw_bytes = 0;
 	fs_devices->chunk_alloc_policy = BTRFS_CHUNK_ALLOC_REGULAR;
 	fs_devices->read_policy = BTRFS_READ_POLICY_PID;
@@ -1968,7 +1968,7 @@ static struct btrfs_device * btrfs_find_next_active_device(
 }
 
 /*
- * Helper function to check if the given device is part of s_bdev / latest_bdev
+ * Helper function to check if the given device is part of s_bdev / latest_dev
  * and replace it with the provided or the next active device, in the context
  * where this function called, there should be always be another device (or
  * this_dev) which is active.
@@ -1987,8 +1987,8 @@ void __cold btrfs_assign_next_active_device(struct btrfs_device *device,
 			(fs_info->sb->s_bdev == device->bdev))
 		fs_info->sb->s_bdev = next_device->bdev;
 
-	if (fs_info->fs_devices->latest_bdev == device->bdev)
-		fs_info->fs_devices->latest_bdev = next_device->bdev;
+	if (fs_info->fs_devices->latest_dev->bdev == device->bdev)
+		fs_info->fs_devices->latest_dev = next_device;
 }
 
 /*
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 4c941b4dd269..150b4cd8f81f 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -246,7 +246,7 @@ struct btrfs_fs_devices {
 	/* Highest generation number of seen devices */
 	u64 latest_generation;
 
-	struct block_device *latest_bdev;
+	struct btrfs_device *latest_dev;
 
 	/* all of the devices in the FS, protected by a mutex
 	 * so we can safely walk it to write out the supers without
-- 
2.31.1

Re: [PATCH v3 0/4] btrf_show_devname related fixes

[David Sterba]

On Fri, Aug 20, 2021 at 07:28:38PM +0800, Anand Jain wrote:
> These fixes are inspired by the bug report and its discussions in the
> mailing list subject
>  btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
> 
> v3:
>  Fix rcu_lock in the patch 3/4

Please next time send a separate patch with another revision, the
threading is a total mess, look

https://lore.kernel.org/linux-btrfs/7171ca39-8f57-4646-face-70d3d23fbf02@oracle.com/T/#t

Re: [PATCH v4 0/4] btrf_show_devname related fixes

[David Sterba]

On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
> These fixes are inspired by the bug report and its discussions in the
> mailing list subject
>  btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
> 
> And depends on the patch
>  [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
> in the ML
> 
> v4:
>  Fix unrelated changes in 2/4
> v3:
>  Fix rcu_lock in the patch 3/4
> 
> Anand Jain (4):
>   btrfs: consolidate device_list_mutex in prepare_sprout to its parent
>   btrfs: save latest btrfs_device instead of its block_device in
>     fs_devices
>   btrfs: use latest_dev in btrfs_show_devname
>   btrfs: update latest_dev when we sprout

Patchset survived one round of fstests and I haven't seen the lockdep
warning in btrfs/020 that's caused by some unrelated work in loop device
driver. There's a series from Josef to fix it by shuffling locking, so
it would be interesting to see where's the difference.

Re: [PATCH v4 0/4] btrf_show_devname related fixes

[Anand Jain]

On 24/08/2021 03:46, David Sterba wrote:
> On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
>> These fixes are inspired by the bug report and its discussions in the
>> mailing list subject
>>   btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
>>
>> And depends on the patch
>>   [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
>> in the ML
>>
>> v4:
>>   Fix unrelated changes in 2/4
>> v3:
>>   Fix rcu_lock in the patch 3/4
>>
>> Anand Jain (4):
>>    btrfs: consolidate device_list_mutex in prepare_sprout to its parent
>>    btrfs: save latest btrfs_device instead of its block_device in
>>      fs_devices
>>    btrfs: use latest_dev in btrfs_show_devname
>>    btrfs: update latest_dev when we sprout
> 
> Patchset survived one round of fstests and I haven't seen the lockdep
> warning in btrfs/020 that's caused by some unrelated work in loop device
> driver.


> There's a series from Josef to fix it by shuffling locking,

  Hm. Is it a recent patch? I can't find.

  Is it about device_list_mutex (as in cleanup patch 1 above) or 
btrfs_show_devname() (which patches 2-4 fixes)?
  Yeah, patch 1 is unrelated to patches 2-4 will separate them send v5.



> so
> it would be interesting to see where's the difference.

Re: [PATCH v4 0/4] btrf_show_devname related fixes

[David Sterba]

On Tue, Aug 24, 2021 at 08:28:09AM +0800, Anand Jain wrote:
> 
> 
> On 24/08/2021 03:46, David Sterba wrote:
> > On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
> >> These fixes are inspired by the bug report and its discussions in the
> >> mailing list subject
> >>   btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
> >>
> >> And depends on the patch
> >>   [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
> >> in the ML
> >>
> >> v4:
> >>   Fix unrelated changes in 2/4
> >> v3:
> >>   Fix rcu_lock in the patch 3/4
> >>
> >> Anand Jain (4):
> >>    btrfs: consolidate device_list_mutex in prepare_sprout to its parent
> >>    btrfs: save latest btrfs_device instead of its block_device in
> >>      fs_devices
> >>    btrfs: use latest_dev in btrfs_show_devname
> >>    btrfs: update latest_dev when we sprout
> > 
> > Patchset survived one round of fstests and I haven't seen the lockdep
> > warning in btrfs/020 that's caused by some unrelated work in loop device
> > driver.
> 
> 
> > There's a series from Josef to fix it by shuffling locking,
> 
>   Hm. Is it a recent patch? I can't find.

https://lore.kernel.org/linux-btrfs/cover.1627419595.git.josef@toxicpanda.com/

>   Is it about device_list_mutex (as in cleanup patch 1 above) or 
> btrfs_show_devname() (which patches 2-4 fixes)?

Relatd to device locking, so device list mutex and also uuid mutex IIRC.

Re: [PATCH v4 0/4] btrf_show_devname related fixes

[Anand Jain]

On 25/08/2021 00:11, David Sterba wrote:
> On Tue, Aug 24, 2021 at 08:28:09AM +0800, Anand Jain wrote:
>>
>>
>> On 24/08/2021 03:46, David Sterba wrote:
>>> On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
>>>> These fixes are inspired by the bug report and its discussions in the
>>>> mailing list subject
>>>>    btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
>>>>
>>>> And depends on the patch
>>>>    [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
>>>> in the ML
>>>>
>>>> v4:
>>>>    Fix unrelated changes in 2/4
>>>> v3:
>>>>    Fix rcu_lock in the patch 3/4
>>>>
>>>> Anand Jain (4):
>>>>     btrfs: consolidate device_list_mutex in prepare_sprout to its parent
>>>>     btrfs: save latest btrfs_device instead of its block_device in
>>>>       fs_devices
>>>>     btrfs: use latest_dev in btrfs_show_devname
>>>>     btrfs: update latest_dev when we sprout
>>>
>>> Patchset survived one round of fstests and I haven't seen the lockdep
>>> warning in btrfs/020 that's caused by some unrelated work in loop device
>>> driver.
>>
>>
>>> There's a series from Josef to fix it by shuffling locking,
>>
>>    Hm. Is it a recent patch? I can't find.
> 
> https://lore.kernel.org/linux-btrfs/cover.1627419595.git.josef@toxicpanda.com/

Thx.
It is the same fix I sent ~3months before in the ML[1].
If you want, I can refresh [1] and include 1/4 (in this patchset)
into a separate patchset.

[1]
https://patchwork.kernel.org/project/linux-btrfs/patch/23a8830f3be500995e74b45f18862e67c0634c3d.1614793362.git.anand.jain@oracle.com/

> 
>>    Is it about device_list_mutex (as in cleanup patch 1 above) or
>> btrfs_show_devname() (which patches 2-4 fixes)?
> 
> Relatd to device locking, so device list mutex and also uuid mutex IIRC.
> 

Re: [PATCH v4 0/4] btrf_show_devname related fixes

[David Sterba]

On Wed, Aug 25, 2021 at 10:13:52AM +0800, Anand Jain wrote:
> On 25/08/2021 00:11, David Sterba wrote:
> > On Tue, Aug 24, 2021 at 08:28:09AM +0800, Anand Jain wrote:
> >>
> >>
> >> On 24/08/2021 03:46, David Sterba wrote:
> >>> On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
> >>>> These fixes are inspired by the bug report and its discussions in the
> >>>> mailing list subject
> >>>>    btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
> >>>>
> >>>> And depends on the patch
> >>>>    [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
> >>>> in the ML
> >>>>
> >>>> v4:
> >>>>    Fix unrelated changes in 2/4
> >>>> v3:
> >>>>    Fix rcu_lock in the patch 3/4
> >>>>
> >>>> Anand Jain (4):
> >>>>     btrfs: consolidate device_list_mutex in prepare_sprout to its parent
> >>>>     btrfs: save latest btrfs_device instead of its block_device in
> >>>>       fs_devices
> >>>>     btrfs: use latest_dev in btrfs_show_devname
> >>>>     btrfs: update latest_dev when we sprout
> >>>
> >>> Patchset survived one round of fstests and I haven't seen the lockdep
> >>> warning in btrfs/020 that's caused by some unrelated work in loop device
> >>> driver.
> >>
> >>
> >>> There's a series from Josef to fix it by shuffling locking,
> >>
> >>    Hm. Is it a recent patch? I can't find.
> > 
> > https://lore.kernel.org/linux-btrfs/cover.1627419595.git.josef@toxicpanda.com/
> 
> Thx.
> It is the same fix I sent ~3months before in the ML[1].

Well, I've seen the patch and pings but sometimes the timing is not good
and there's something else I have to finish before looking into new
area. And device locking is something that needs full attention. The
bugs/warnings in question weren't that important.

But anyway now the 5.15 branch is out and there are several patches
regarding the devices so I'm going to spend time on that.  Also because
the recent updates to loop devices started to trigger lockdep warnings
that make testing harder and we're missing lockdep in many tests.

Re: [PATCH v4 0/4] btrf_show_devname related fixes

[Anand Jain]

On 31/08/2021 23:40, David Sterba wrote:
> On Wed, Aug 25, 2021 at 10:13:52AM +0800, Anand Jain wrote:
>> On 25/08/2021 00:11, David Sterba wrote:
>>> On Tue, Aug 24, 2021 at 08:28:09AM +0800, Anand Jain wrote:
>>>>
>>>>
>>>> On 24/08/2021 03:46, David Sterba wrote:
>>>>> On Mon, Aug 23, 2021 at 07:31:38PM +0800, Anand Jain wrote:
>>>>>> These fixes are inspired by the bug report and its discussions in the
>>>>>> mailing list subject
>>>>>>     btrfs: traverse seed devices if fs_devices::devices is empty in show_devname
>>>>>>
>>>>>> And depends on the patch
>>>>>>     [PATCH v2] btrfs: fix lockdep warning while mounting sprout fs
>>>>>> in the ML
>>>>>>
>>>>>> v4:
>>>>>>     Fix unrelated changes in 2/4
>>>>>> v3:
>>>>>>     Fix rcu_lock in the patch 3/4
>>>>>>
>>>>>> Anand Jain (4):
>>>>>>      btrfs: consolidate device_list_mutex in prepare_sprout to its parent
>>>>>>      btrfs: save latest btrfs_device instead of its block_device in
>>>>>>        fs_devices
>>>>>>      btrfs: use latest_dev in btrfs_show_devname
>>>>>>      btrfs: update latest_dev when we sprout
>>>>>
>>>>> Patchset survived one round of fstests and I haven't seen the lockdep
>>>>> warning in btrfs/020 that's caused by some unrelated work in loop device
>>>>> driver.
>>>>
>>>>
>>>>> There's a series from Josef to fix it by shuffling locking,
>>>>
>>>>     Hm. Is it a recent patch? I can't find.
>>>
>>> https://lore.kernel.org/linux-btrfs/cover.1627419595.git.josef@toxicpanda.com/
>>
>> Thx.
>> It is the same fix I sent ~3months before in the ML[1].
> 
> Well, I've seen the patch and pings but sometimes the timing is not good
> and there's something else I have to finish before looking into new
> area. And device locking is something that needs full attention. The
> bugs/warnings in question weren't that important.

  Ah. Thanks for taking the time to explain.

> But anyway now the 5.15 branch is out and there are several patches
> regarding the devices so I'm going to spend time on that.

  Thx. That includes read_policy patchset as well? Let me know so that I
  can refresh/resend.

>  Also because
> the recent updates to loop devices started to trigger lockdep warnings
> that make testing harder and we're missing lockdep in many tests.

  I am reviewing the patch 1 and 2 in Josef list.

Re: [PATCH v4 0/4] btrf_show_devname related fixes

[David Sterba]

On Wed, Sep 01, 2021 at 04:18:41PM +0800, Anand Jain wrote:
> > But anyway now the 5.15 branch is out and there are several patches
> > regarding the devices so I'm going to spend time on that.
> 
>   Thx. That includes read_policy patchset as well? Let me know so that I
>   can refresh/resend.

Yeah we should finalize that, we have all the building blocks ready.
What's remaining is proper benchmarking, IIRC we haven't found a good
default policy yet.

> 
> >  Also because
> > the recent updates to loop devices started to trigger lockdep warnings
> > that make testing harder and we're missing lockdep in many tests.
> 
>   I am reviewing the patch 1 and 2 in Josef list.

Thanks, reviewing the whole batch should be easier now.