Re: [PATCH 2/4] btrfs: introduce new device-state read_preferred

Josef Bacik
2020-10-28T21:41:14Z

On 10/28/20 9:26 AM, Anand Jain wrote:
> This is a preparatory patch and introduces a new device flag
> 'read_preferred', RW-able using sysfs interface.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> fs/btrfs/sysfs.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/btrfs/volumes.h | 1 +
> 2 files changed, 56 insertions(+)
>
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 88cbf7b2edf0..52b4c9bef673 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -1413,11 +1413,66 @@ static ssize_t btrfs_devinfo_writeable_show(struct kobject *kobj,
> }
> BTRFS_ATTR(devid, writeable, btrfs_devinfo_writeable_show);
>
> +static ssize_t btrfs_devinfo_read_pref_show(struct kobject *kobj,
> + struct kobj_attribute *a, char *buf)
> +{
> + int val;
> + struct btrfs_device *device = container_of(kobj, struct btrfs_device,
> + devid_kobj);
> +
> + val = !!test_bit(BTRFS_DEV_STATE_READ_PREFERRED, &device->dev_state);
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", val);
> +}
> +
> +static ssize_t btrfs_devinfo_read_pref_store(struct kobject *kobj,
> + struct kobj_attribute *a,
> + const char *buf, size_t len)
> +{
> + int ret;
> + unsigned long val;
> + struct btrfs_device *device;
> +
> + ret = kstrtoul(skip_spaces(buf), 0, &val);
> + if (ret)
> + return ret;
> +
> + if (val != 0 && val != 1)
> + return -EINVAL;
> +
> + /*
> + * lock is not required, the btrfs_device struct can't be freed while
> + * its kobject btrfs_device::devid_kobj is still open.
> + */
> + device = container_of(kobj, struct btrfs_device, devid_kobj);
> +
> + if (val &&
> + ! test_bit(BTRFS_DEV_STATE_READ_PREFERRED, &device->dev_state)) {
> +

No extra space between the ! test_bit, and you don't need these extra newlines.

I sort of wonder what happens if we have multiple SSD's with multiple slow
disks, we may want to load balance between the SSD's and set the two SSD's to
preferred. But typing it out made me think its not really related to this
change, so don't worry too much about it, just thoughts for the future. Thanks,

Josef