diff mbox series

[v7,3/5] btrfs: create read policy sysfs attribute, pid

Message ID 1586173871-5559-4-git-send-email-anand.jain@oracle.com (mailing list archive)
State New, archived
Headers show
Series readmirror feature (sysfs and in-memory only approach; with new read_policy device) | expand

Commit Message

Anand Jain April 6, 2020, 11:51 a.m. UTC 
Add

 /sys/fs/btrfs/UUID/read_policy

attribute so that the read policy for the raid1 and raid10 chunks can be
tuned.

When this attribute is read, it shall show all available policies, with
active policy being with in [ ]. The read_policy attribute can be written
using one of the items listed in there.

For example:
  $cat /sys/fs/btrfs/UUID/read_policy
  [pid]
  $echo pid > /sys/fs/btrfs/UUID/read_policy

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v5: 
  Title rename: old: btrfs: sysfs, add read_policy attribute
  Uses the btrfs_strmatch() helper (BTRFS_READ_POLICY_NAME_MAX dropped).
  Use the table for the policy names.
  Rename len to ret.
  Use a simple logic to prefix space in btrfs_read_policy_show()
  Reviewed-by: Josef Bacik <josef@toxicpanda.com> dropped.

v4:-
v3: rename [by_pid] to [pid]
v2: v2: check input len before strip and kstrdup

 fs/btrfs/sysfs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

Comments

Johannes Thumshirn May 19, 2020, 10:07 a.m. UTC | #1 
On 06/04/2020 13:51, Anand Jain wrote:
> +static ssize_t btrfs_read_policy_store(struct kobject *kobj,
> +				       struct kobj_attribute *a,
> +				       const char *buf, size_t len)
> +{
> +	int i;
> +	struct btrfs_fs_devices *fs_devices = to_fs_devs(kobj);
> +
> +	for (i = 0; i < BTRFS_NR_READ_POLICY; i++) {
> +		if (btrfs_strmatch(buf, btrfs_read_policy_name[i]) == 0) {
> +			if (i != fs_devices->read_policy) {
> +				fs_devices->read_policy = i;
> +				btrfs_info(fs_devices->fs_info,
> +					   "read policy set to '%s'",
> +					   btrfs_read_policy_name[i]);
> +			}
> +			return len;
> +		}
> +	}

Naive question, what's the advantage over sysfs_match_string()?
Anand Jain May 20, 2020, 8:54 a.m. UTC | #2 
On 19/5/20 6:07 pm, Johannes Thumshirn wrote:
> On 06/04/2020 13:51, Anand Jain wrote:
>> +static ssize_t btrfs_read_policy_store(struct kobject *kobj,
>> +				       struct kobj_attribute *a,
>> +				       const char *buf, size_t len)
>> +{
>> +	int i;
>> +	struct btrfs_fs_devices *fs_devices = to_fs_devs(kobj);
>> +
>> +	for (i = 0; i < BTRFS_NR_READ_POLICY; i++) {
>> +		if (btrfs_strmatch(buf, btrfs_read_policy_name[i]) == 0) {
>> +			if (i != fs_devices->read_policy) {
>> +				fs_devices->read_policy = i;
>> +				btrfs_info(fs_devices->fs_info,
>> +					   "read policy set to '%s'",
>> +					   btrfs_read_policy_name[i]);
>> +			}
>> +			return len;
>> +		}
>> +	}
> 
> Naive question, what's the advantage over sysfs_match_string()?

We strip both leading and trailing whitespaces, its not
possible to do the same with sysfs_match_string().

For example:
echo ==== These should pass ======
run_nocheck "echo ' pid' > $sysfs_path/read_policy"
run_nocheck "echo ' pid ' > $sysfs_path/read_policy"
run_nocheck "echo -n ' pid ' > $sysfs_path/read_policy"
run_nocheck "echo -n ' pid' > $sysfs_path/read_policy"
run_nocheck "echo 'pid ' > $sysfs_path/read_policy"
run_nocheck "echo -n 'pid ' > $sysfs_path/read_policy"
run_nocheck "echo -n pid > $sysfs_path/read_policy"
run_nocheck "echo pid > $sysfs_path/read_policy"

Thanks, Anand
Johannes Thumshirn May 20, 2020, 8:55 a.m. UTC | #3 
On 20/05/2020 10:54, Anand Jain wrote:
> On 19/5/20 6:07 pm, Johannes Thumshirn wrote:
>> On 06/04/2020 13:51, Anand Jain wrote:
>>> +static ssize_t btrfs_read_policy_store(struct kobject *kobj,
>>> +				       struct kobj_attribute *a,
>>> +				       const char *buf, size_t len)
>>> +{
>>> +	int i;
>>> +	struct btrfs_fs_devices *fs_devices = to_fs_devs(kobj);
>>> +
>>> +	for (i = 0; i < BTRFS_NR_READ_POLICY; i++) {
>>> +		if (btrfs_strmatch(buf, btrfs_read_policy_name[i]) == 0) {
>>> +			if (i != fs_devices->read_policy) {
>>> +				fs_devices->read_policy = i;
>>> +				btrfs_info(fs_devices->fs_info,
>>> +					   "read policy set to '%s'",
>>> +					   btrfs_read_policy_name[i]);
>>> +			}
>>> +			return len;
>>> +		}
>>> +	}
>>
>> Naive question, what's the advantage over sysfs_match_string()?
> 
> We strip both leading and trailing whitespaces, its not
> possible to do the same with sysfs_match_string().
> 
> For example:
> echo ==== These should pass ======
> run_nocheck "echo ' pid' > $sysfs_path/read_policy"
> run_nocheck "echo ' pid ' > $sysfs_path/read_policy"
> run_nocheck "echo -n ' pid ' > $sysfs_path/read_policy"
> run_nocheck "echo -n ' pid' > $sysfs_path/read_policy"
> run_nocheck "echo 'pid ' > $sysfs_path/read_policy"
> run_nocheck "echo -n 'pid ' > $sysfs_path/read_policy"
> run_nocheck "echo -n pid > $sysfs_path/read_policy"
> run_nocheck "echo pid > $sysfs_path/read_policy"

Ah ok
diff mbox series

Patch

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 7bb68cef98ab..c9a8850b186a 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -832,6 +832,54 @@  static int btrfs_strmatch(const char *given, const char *golden)
 	return -EINVAL;
 }
 
+static const char* const btrfs_read_policy_name[] = { "pid" };
+
+static ssize_t btrfs_read_policy_show(struct kobject *kobj,
+				      struct kobj_attribute *a, char *buf)
+{
+	int i;
+	ssize_t ret = 0;
+	struct btrfs_fs_devices *fs_devices = to_fs_devs(kobj);
+
+	for (i = 0; i < BTRFS_NR_READ_POLICY; i++) {
+		if (fs_devices->read_policy == i)
+			ret += snprintf(buf + ret, PAGE_SIZE - ret, "%s[%s]",
+					(ret == 0 ? "" : " "),
+					btrfs_read_policy_name[i]);
+		else
+			ret += snprintf(buf + ret, PAGE_SIZE - ret, "%s%s",
+					(ret == 0 ? "" : " "),
+					btrfs_read_policy_name[i]);
+	}
+
+	ret += snprintf(buf + ret, PAGE_SIZE - ret, "\n");
+
+	return ret;
+}
+
+static ssize_t btrfs_read_policy_store(struct kobject *kobj,
+				       struct kobj_attribute *a,
+				       const char *buf, size_t len)
+{
+	int i;
+	struct btrfs_fs_devices *fs_devices = to_fs_devs(kobj);
+
+	for (i = 0; i < BTRFS_NR_READ_POLICY; i++) {
+		if (btrfs_strmatch(buf, btrfs_read_policy_name[i]) == 0) {
+			if (i != fs_devices->read_policy) {
+				fs_devices->read_policy = i;
+				btrfs_info(fs_devices->fs_info,
+					   "read policy set to '%s'",
+					   btrfs_read_policy_name[i]);
+			}
+			return len;
+		}
+	}
+
+	return -EINVAL;
+}
+BTRFS_ATTR_RW(, read_policy, btrfs_read_policy_show, btrfs_read_policy_store);
+
 static const struct attribute *btrfs_attrs[] = {
 	BTRFS_ATTR_PTR(, label),
 	BTRFS_ATTR_PTR(, nodesize),
@@ -840,6 +888,7 @@  static const struct attribute *btrfs_attrs[] = {
 	BTRFS_ATTR_PTR(, quota_override),
 	BTRFS_ATTR_PTR(, metadata_uuid),
 	BTRFS_ATTR_PTR(, checksum),
+	BTRFS_ATTR_PTR(, read_policy),
 	NULL,
 };