Re: [PATCH 1/2] btrfs: init device stats for seed devices

Anand Jain
2020-09-18T21:47:23Z

Seed read errors should remain persistent.
A similar update to btrfs_run_dev_stats() is required?

On 19/9/20 4:44 am, Josef Bacik wrote:
> We recently started recording device stats across the fleet, and noticed
> a large increase in messages such as this
>
> BTRFS warning (device dm-0): get dev_stats failed, not yet valid
>
> on our tiers that use seed devices for their root devices. This is
> because we do not initialize the device stats for any seed devices if we
> have a sprout device and mount using that sprout device. The basic
> steps for reproducing are
>
> mkfs seed device
> mount seed device
> fill seed device
> umount seed device
> btrfstune -S 1 seed device
> mount seed device
> btrfs device add -f sprout device /mnt/wherever
> umount /mnt/wherever
> mount sprout device /mnt/wherever
> btrfs device stats /mnt/wherever
>
> This will fail with the above message in dmesg.
>
> Fix this by iterating over the fs_devices->seed if they exist in
> btrfs_init_dev_stats. This fixed the problem and properly reports the
> stats for both devices.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
> fs/btrfs/volumes.c | 88 +++++++++++++++++++++++++---------------------
> 1 file changed, 48 insertions(+), 40 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6c7c8819cb31..c0cea9f5fdbc 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -7223,61 +7223,69 @@ static void btrfs_set_dev_stats_value(struct extent_buffer *eb,
> sizeof(val));
> }
>
> -int btrfs_init_dev_stats(struct btrfs_fs_info *fs_info)
> +static void __btrfs_init_dev_stats(struct btrfs_fs_info *fs_info,
> + struct btrfs_device *device,
> + struct btrfs_path *path)
> {
> - struct btrfs_key key;
> struct btrfs_root *dev_root = fs_info->dev_root;
> - struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
> + struct btrfs_dev_stats_item *ptr;
> struct extent_buffer *eb;
> - int slot;
> - int ret = 0;
> + struct btrfs_key key;
> + int item_size;
> + int i, ret, slot;
> +
> + key.objectid = BTRFS_DEV_STATS_OBJECTID;
> + key.type = BTRFS_PERSISTENT_ITEM_KEY;
> + key.offset = device->devid;
> + ret = btrfs_search_slot(NULL, dev_root, &key, path, 0, 0);
> + if (ret) {
> + for (i = 0; i < BTRFS_DEV_STAT_VALUES_MAX; i++)
> + btrfs_dev_stat_set(device, i, 0);
> + device->dev_stats_valid = 1;
> + btrfs_release_path(path);
> + return;
> + }
> + slot = path->slots[0];
> + eb = path->nodes[0];
> + item_size = btrfs_item_size_nr(eb, slot);
> +
> + ptr = btrfs_item_ptr(eb, slot,
> + struct btrfs_dev_stats_item);
> +
> + for (i = 0; i < BTRFS_DEV_STAT_VALUES_MAX; i++) {
> + if (item_size >= (1 + i) * sizeof(__le64))
> + btrfs_dev_stat_set(device, i,
> + btrfs_dev_stats_value(eb, ptr, i));
> + else
> + btrfs_dev_stat_set(device, i, 0);
> + }
> +
> + device->dev_stats_valid = 1;
> + btrfs_dev_stat_print_on_load(device);
> + btrfs_release_path(path);
> +}
> +
> +int btrfs_init_dev_stats(struct btrfs_fs_info *fs_info)
> +{
> + struct btrfs_fs_devices *fs_devices = fs_info->fs_devices, *seed_devs;
> struct btrfs_device *device;
> struct btrfs_path *path = NULL;
> - int i;
>
> path = btrfs_alloc_path();
> if (!path)
> return -ENOMEM;
>
> mutex_lock(&fs_devices->device_list_mutex);
> - list_for_each_entry(device, &fs_devices->devices, dev_list) {
> - int item_size;
> - struct btrfs_dev_stats_item *ptr;
> -
> - key.objectid = BTRFS_DEV_STATS_OBJECTID;
> - key.type = BTRFS_PERSISTENT_ITEM_KEY;
> - key.offset = device->devid;
> - ret = btrfs_search_slot(NULL, dev_root, &key, path, 0, 0);
> - if (ret) {
> - for (i = 0; i < BTRFS_DEV_STAT_VALUES_MAX; i++)
> - btrfs_dev_stat_set(device, i, 0);
> - device->dev_stats_valid = 1;
> - btrfs_release_path(path);
> - continue;
> - }
> - slot = path->slots[0];
> - eb = path->nodes[0];
> - item_size = btrfs_item_size_nr(eb, slot);
> -
> - ptr = btrfs_item_ptr(eb, slot,
> - struct btrfs_dev_stats_item);
> -
> - for (i = 0; i < BTRFS_DEV_STAT_VALUES_MAX; i++) {
> - if (item_size >= (1 + i) * sizeof(__le64))
> - btrfs_dev_stat_set(device, i,
> - btrfs_dev_stats_value(eb, ptr, i));
> - else
> - btrfs_dev_stat_set(device, i, 0);
> - }
> -
> - device->dev_stats_valid = 1;
> - btrfs_dev_stat_print_on_load(device);
> - btrfs_release_path(path);
> + list_for_each_entry(device, &fs_devices->devices, dev_list)
> + __btrfs_init_dev_stats(fs_info, device, path);
> + list_for_each_entry(seed_devs, &fs_devices->seed_list, seed_list) {
> + list_for_each_entry(device, &seed_devs->devices, dev_list)
> + __btrfs_init_dev_stats(fs_info, device, path);
> }
> mutex_unlock(&fs_devices->device_list_mutex);
>
> btrfs_free_path(path);
> - return ret < 0 ? ret : 0;
> + return 0;
> }
>
> static int update_dev_stat_item(struct btrfs_trans_handle *trans,
>