Re: [PATCH RFC 3/3] btrfs: use latest_bdev in btrfs_show_devname
From: David Sterba <hidden>
Date: 2021-08-20 11:00:56
On Fri, Aug 20, 2021 at 02:18:14AM +0800, Anand Jain wrote:
latest_bdev is updated according to the changes to the device list. That means we could use the latest_bdev to show the device name in /proc/self/mounts. So this patch makes that change. Signed-off-by: Anand Jain <redacted> --- RFC because 1. latest_bdev might not be the lowest devid but, we showed the lowest devid in /proc/self/mount. 2. The device's path is not shown now but, previously we did. So does these break ABI? Maybe yes for 2 howabout for 1 above?
The path needs to be preserved, that would break a lot of things..
quoted hunk ↗ jump to hunk
fs/btrfs/super.c | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-)diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 1f9dd1a4faa3..4ad3fe174c41 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c@@ -2464,30 +2464,11 @@ 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; + char name[BDEVNAME_SIZE]; - /* - * 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; - } + seq_escape(m, bdevname(fs_info->fs_devices->latest_bdev, name), + " \t\n\\");
No protection at all? So what if latest_bdev or latest_dev gets updated in parallel with devicre remove and there's a window where the pointer is invalid but still is accessed. The whole point of RCU section here is to prevent that.
- if (first_dev) - seq_escape(m, rcu_str_deref(first_dev->name), " \t\n\\"); - else - WARN_ON(1); - rcu_read_unlock(); return 0; } -- 2.31.1