| Message ID | 5d254bebd4afefa42e8c56ae1002354c04c7112c.1629780501.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrf_show_devname related fixes | expand |
On Tue, Aug 24, 2021 at 01:05:20PM +0800, Anand Jain wrote: > RFC because, > With this patch, /proc/self/mounts might not show the lowest devid > device as we did before. Instead we show the device that has the greatest > generation and, we used it to build the tree. IMO it is ok because > /proc/self/mounts should show a device the belongs to the fsid not, > necessarily the lowest devid device as devid is internal to btrfs. > IMO this won't affect the ABI? I tend to agree, the only thing that should be consistent that any number of mounts of the same filesystem (eg. by subvolume) should print the same device path. But given that fs_devices is shared then the same output is guaranteed. The time when the latest_bdev changes is after remove or replace, that's an intermediate state so the results may vary. And maybe when printing the device by which the fs was mounted is more correct, as it may be different from the lowest id and that could potentially be confusing. The commit 9c5085c14798 ("Btrfs: implement ->show_devname") adding the show_devname callback even mentions not showing the mount device as a drawback. The multi-device fs devices should be treated equally for the purpose of mount.
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; }
btrfs/238 reports warning as below. [1] ------------[ cut here ]------------ WARNING: CPU: 3 PID: 481 at fs/btrfs/super.c:2509 btrfs_show_devname+0x104/0x1e8 [btrfs] CPU: 2 PID: 1 Comm: systemd Tainted: G W O 5.14.0-rc1-custom #72 Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 Call trace: btrfs_show_devname+0x108/0x1b4 [btrfs] show_mountinfo+0x234/0x2c4 m_show+0x28/0x34 seq_read_iter+0x12c/0x3c4 vfs_read+0x29c/0x2c8 ksys_read+0x80/0xec __arm64_sys_read+0x28/0x34 invoke_syscall+0x50/0xf8 do_el0_svc+0x88/0x138 el0_svc+0x2c/0x8c el0t_64_sync_handler+0x84/0xe4 el0t_64_sync+0x198/0x19c ---[ end trace 3efd7e5950b8af05 ]--- Reason: While btrfs_prepare_sprout() moves the fs_devices::devices into fs_devices::seed_list, the btrfs_show_devname() searched for the devices and found none, leading to the warning as in [1] (above). Fix: 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. Reported-by: Su Yue <l@damenly.su> 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. Instead we show the device that has the greatest generation and, we used it to build the tree. IMO it is ok because /proc/self/mounts should show a device the belongs to the fsid not, necessarily the lowest devid device as devid is internal to btrfs. IMO this won't affect the ABI? fs/btrfs/super.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-)