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

Anand Jain
2020-10-29T01:14:06Z

On 28/10/20 10:37 pm, Josef Bacik wrote:
> 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.
>
Will fix.

> 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,
>

That's true. I had the same challenges when I wrote this. Then later
it lead to have device groups, which is for next developments.

Thanks, Anand

> Josef