[PATCH 0/3] readmirror feature

[PATCH 0/3] readmirror feature

[Anand Jain]

These patches are tested to be working fine.

Function call chain  __btrfs_map_block()->find_live_mirror() uses
thread pid to determine the %mirror_num when the mirror_num=0.

This patch introduces a framework so that we can add policies to determine
the %mirror_num. And adds the devid as the readmirror policy.

The property is stored as an extented attributes of root inode
(BTRFS_FS_TREE_OBJECTID).
User provided devid list is validated against the fs_devices::dev_list.

 For example:
   Usage:
     btrfs property set <mnt> readmirror devid<n>[,<m>...]
     btrfs property set <mnt> readmirror ""

   mkfs.btrfs -fq -draid1 -mraid1 /dev/sd[b-d] && mount /dev/sdb /btrfs
   btrfs prop set /btrfs readmirror devid1,2
   btrfs prop get /btrfs readmirror
    readmirror=devid1,2
   getfattr -n btrfs.readmirror --absolute-names /btrfs
    btrfs.readmirror="devid1,2"
   btrfs prop set /btrfs readmirror ""
   getfattr -n btrfs.readmirror --absolute-names /btrfs
    /btrfs: btrfs.readmirror: No such attribute
   btrfs prop get /btrfs readmirror

RFC->v1:
  Drops pid as one of the readmirror policy choices and as usual remains
  as default. And when the devid is reset the readmirror policy falls back
  to pid.
  Drops the mount -o readmirror idea, it can be added at a later point of
  time.
  Property now accepts more than 1 devid as readmirror device. As shown
  in the example above.

Anand Jain (3):
  btrfs: add inode pointer to prop_handler::validate()
  btrfs: add readmirror property framework
  btrfs: add readmirror devid property

 fs/btrfs/props.c   | 120 +++++++++++++++++++++++++++++++++++++++++++--
 fs/btrfs/props.h   |   4 +-
 fs/btrfs/volumes.c |  25 +++++++++-
 fs/btrfs/volumes.h |   8 +++
 fs/btrfs/xattr.c   |   2 +-
 5 files changed, 150 insertions(+), 9 deletions(-)

Anand Jain (2):
  btrfs-progs: add helper to create xattr name
  btrfs-progs: add readmirror policy

 props.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 68 insertions(+), 7 deletions(-)
-- 
2.20.1 (Apple Git-117)

[PATCH 1/3] btrfs: add inode pointer to prop_handler::validate()

[Anand Jain]

In preparation to add the readmirror property, pass inode in the
prop_handler::validate() function vector, so that it can fetch
corresponding fs_devices.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/props.c | 12 +++++++-----
 fs/btrfs/props.h |  4 ++--
 fs/btrfs/xattr.c |  2 +-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index ca2716917e37..5c1fccc32b43 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -17,7 +17,7 @@ static DEFINE_HASHTABLE(prop_handlers_ht, BTRFS_PROP_HANDLERS_HT_BITS);
 struct prop_handler {
 	struct hlist_node node;
 	const char *xattr_name;
-	int (*validate)(const char *value, size_t len);
+	int (*validate)(struct inode *inode, const char *value, size_t len);
 	int (*apply)(struct inode *inode, const char *value, size_t len);
 	const char *(*extract)(struct inode *inode);
 	int inheritable;
@@ -55,7 +55,8 @@ find_prop_handler(const char *name,
 	return NULL;
 }
 
-int btrfs_validate_prop(const char *name, const char *value, size_t value_len)
+int btrfs_validate_prop(struct inode *inode, const char *name,
+			const char *value, size_t value_len)
 {
 	const struct prop_handler *handler;
 
@@ -69,7 +70,7 @@ int btrfs_validate_prop(const char *name, const char *value, size_t value_len)
 	if (value_len == 0)
 		return 0;
 
-	return handler->validate(value, value_len);
+	return handler->validate(inode, value, value_len);
 }
 
 int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
@@ -252,7 +253,8 @@ int btrfs_load_inode_props(struct inode *inode, struct btrfs_path *path)
 	return ret;
 }
 
-static int prop_compression_validate(const char *value, size_t len)
+static int prop_compression_validate(struct inode *inode, const char *value,
+				     size_t len)
 {
 	if (!value)
 		return 0;
@@ -353,7 +355,7 @@ static int inherit_props(struct btrfs_trans_handle *trans,
 		 * This is not strictly necessary as the property should be
 		 * valid, but in case it isn't, don't propagate it futher.
 		 */
-		ret = h->validate(value, strlen(value));
+		ret = h->validate(inode, value, strlen(value));
 		if (ret)
 			continue;
 
diff --git a/fs/btrfs/props.h b/fs/btrfs/props.h
index 40b2c65b518c..5b1e39136f7f 100644
--- a/fs/btrfs/props.h
+++ b/fs/btrfs/props.h
@@ -13,8 +13,8 @@ void __init btrfs_props_init(void);
 int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
 		   const char *name, const char *value, size_t value_len,
 		   int flags);
-int btrfs_validate_prop(const char *name, const char *value, size_t value_len);
-
+int btrfs_validate_prop(struct inode *inode, const char *name,
+			const char *value, size_t value_len);
 int btrfs_load_inode_props(struct inode *inode, struct btrfs_path *path);
 
 int btrfs_inode_inherit_props(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index 46ad7b5cf209..4e4f0edd4968 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -378,7 +378,7 @@ static int btrfs_xattr_handler_set_prop(const struct xattr_handler *handler,
 	struct btrfs_root *root = BTRFS_I(inode)->root;
 
 	name = xattr_full_name(handler, name);
-	ret = btrfs_validate_prop(name, value, size);
+	ret = btrfs_validate_prop(inode, name, value, size);
 	if (ret)
 		return ret;
 
-- 
2.20.1 (Apple Git-117)

Re: [PATCH 0/3] readmirror feature

[Anand Jain]

> On 11 May 2019, at 11:24 PM, Steven Davies <btrfs-list@steev.me.uk> wrote:
> 
> Tested-by: Steven Davies <btrfs-list@steev.me.uk>
> 
> Series (inc. btrfs-progs changes) tested working as expected against current btrfs-devel and btrfs-progs/devel.
> 
> Am I correct in thinking that if more than one devid is assigned as a readmirror, the lowest available devid will be preferred?
> 

As of now, it's just don’t do that kind of approach and it picks stripe 0 as read mirror device. And a disk is stripe 0 if it has largest disk free space as the time of chunk allocation. So it's kind of hard to predict which disk gets picked.

I have plans to write a patch to maintain the consistent allocation order.

> Also I think it would be nice to be able to set this property at mkfs time.
> 

Yeah. It certainly makes sense in case of mixed device types such as ssd and hdd or hdd/ssd and iscsi.

Thanks, Anand

> I'd love to see this feature make it upstream.
> 
> -- 
> Steven Davies
> 
> On 25/04/2019 12:59, Anand Jain wrote:
>> These patches are tested to be working fine.
>> Function call chain  __btrfs_map_block()->find_live_mirror() uses
>> thread pid to determine the %mirror_num when the mirror_num=0.
>> This patch introduces a framework so that we can add policies to determine
>> the %mirror_num. And adds the devid as the readmirror policy.
>> The property is stored as an extented attributes of root inode
>> (BTRFS_FS_TREE_OBJECTID).
>> User provided devid list is validated against the fs_devices::dev_list.
>> For example:
>>   Usage:
>>     btrfs property set <mnt> readmirror devid<n>[,<m>...]
>>     btrfs property set <mnt> readmirror ""
>>   mkfs.btrfs -fq -draid1 -mraid1 /dev/sd[b-d] && mount /dev/sdb /btrfs
>>   btrfs prop set /btrfs readmirror devid1,2
>>   btrfs prop get /btrfs readmirror
>>    readmirror=devid1,2
>>   getfattr -n btrfs.readmirror --absolute-names /btrfs
>>    btrfs.readmirror="devid1,2"
>>   btrfs prop set /btrfs readmirror ""
>>   getfattr -n btrfs.readmirror --absolute-names /btrfs
>>    /btrfs: btrfs.readmirror: No such attribute
>>   btrfs prop get /btrfs readmirror
>> RFC->v1:
>>  Drops pid as one of the readmirror policy choices and as usual remains
>>  as default. And when the devid is reset the readmirror policy falls back
>>  to pid.
>>  Drops the mount -o readmirror idea, it can be added at a later point of
>>  time.
>>  Property now accepts more than 1 devid as readmirror device. As shown
>>  in the example above.
>> Anand Jain (3):
>>  btrfs: add inode pointer to prop_handler::validate()
>>  btrfs: add readmirror property framework
>>  btrfs: add readmirror devid property
>> fs/btrfs/props.c   | 120 +++++++++++++++++++++++++++++++++++++++++++--
>> fs/btrfs/props.h   |   4 +-
>> fs/btrfs/volumes.c |  25 +++++++++-
>> fs/btrfs/volumes.h |   8 +++
>> fs/btrfs/xattr.c   |   2 +-
>> 5 files changed, 150 insertions(+), 9 deletions(-)
>> Anand Jain (2):
>>  btrfs-progs: add helper to create xattr name
>>  btrfs-progs: add readmirror policy
>> props.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 68 insertions(+), 7 deletions(-)

Re: [PATCH 0/3] readmirror feature

[Steven Davies]

Tested-by: Steven Davies <btrfs-list@steev.me.uk>

Series (inc. btrfs-progs changes) tested working as expected against 
current btrfs-devel and btrfs-progs/devel.

Am I correct in thinking that if more than one devid is assigned as a 
readmirror, the lowest available devid will be preferred?

Also I think it would be nice to be able to set this property at mkfs time.

I'd love to see this feature make it upstream.

-- 
Steven Davies

On 25/04/2019 12:59, Anand Jain wrote:
> These patches are tested to be working fine.
> 
> Function call chain  __btrfs_map_block()->find_live_mirror() uses
> thread pid to determine the %mirror_num when the mirror_num=0.
> 
> This patch introduces a framework so that we can add policies to determine
> the %mirror_num. And adds the devid as the readmirror policy.
> 
> The property is stored as an extented attributes of root inode
> (BTRFS_FS_TREE_OBJECTID).
> User provided devid list is validated against the fs_devices::dev_list.
> 
>   For example:
>     Usage:
>       btrfs property set <mnt> readmirror devid<n>[,<m>...]
>       btrfs property set <mnt> readmirror ""
> 
>     mkfs.btrfs -fq -draid1 -mraid1 /dev/sd[b-d] && mount /dev/sdb /btrfs
>     btrfs prop set /btrfs readmirror devid1,2
>     btrfs prop get /btrfs readmirror
>      readmirror=devid1,2
>     getfattr -n btrfs.readmirror --absolute-names /btrfs
>      btrfs.readmirror="devid1,2"
>     btrfs prop set /btrfs readmirror ""
>     getfattr -n btrfs.readmirror --absolute-names /btrfs
>      /btrfs: btrfs.readmirror: No such attribute
>     btrfs prop get /btrfs readmirror
> 
> RFC->v1:
>    Drops pid as one of the readmirror policy choices and as usual remains
>    as default. And when the devid is reset the readmirror policy falls back
>    to pid.
>    Drops the mount -o readmirror idea, it can be added at a later point of
>    time.
>    Property now accepts more than 1 devid as readmirror device. As shown
>    in the example above.
> 
> Anand Jain (3):
>    btrfs: add inode pointer to prop_handler::validate()
>    btrfs: add readmirror property framework
>    btrfs: add readmirror devid property
> 
>   fs/btrfs/props.c   | 120 +++++++++++++++++++++++++++++++++++++++++++--
>   fs/btrfs/props.h   |   4 +-
>   fs/btrfs/volumes.c |  25 +++++++++-
>   fs/btrfs/volumes.h |   8 +++
>   fs/btrfs/xattr.c   |   2 +-
>   5 files changed, 150 insertions(+), 9 deletions(-)
> 
> Anand Jain (2):
>    btrfs-progs: add helper to create xattr name
>    btrfs-progs: add readmirror policy
> 
>   props.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>   1 file changed, 68 insertions(+), 7 deletions(-)
> 

[PATCH 0/3] readmirror feature

[Anand Jain]

These patches are tested to be working fine.

Function call chain  __btrfs_map_block()->find_live_mirror() uses
thread pid to determine the %mirror_num when the mirror_num=0.

This patch introduces a framework so that we can add policies to determine
the %mirror_num. And adds the devid as the readmirror policy.

The property is stored as an extented attributes of root inode
(BTRFS_FS_TREE_OBJECTID).
User provided devid list is validated against the fs_devices::dev_list.

 For example:
   Usage:
     btrfs property set <mnt> readmirror devid<n>[,<m>...]
     btrfs property set <mnt> readmirror ""

   mkfs.btrfs -fq -draid1 -mraid1 /dev/sd[b-d] && mount /dev/sdb /btrfs
   btrfs prop set /btrfs readmirror devid1,2
   btrfs prop get /btrfs readmirror
    readmirror=devid1,2
   getfattr -n btrfs.readmirror --absolute-names /btrfs
    btrfs.readmirror="devid1,2"
   btrfs prop set /btrfs readmirror ""
   getfattr -n btrfs.readmirror --absolute-names /btrfs
    /btrfs: btrfs.readmirror: No such attribute
   btrfs prop get /btrfs readmirror

RFC->v1:
  Drops pid as one of the readmirror policy choices and as usual remains
  as default. And when the devid is reset the readmirror policy falls back
  to pid.
  Drops the mount -o readmirror idea, it can be added at a later point of
  time.
  Property now accepts more than 1 devid as readmirror device. As shown
  in the example above.

Anand Jain (3):
  btrfs: add inode pointer to prop_handler::validate()
  btrfs: add readmirror property framework
  btrfs: add readmirror devid property

 fs/btrfs/props.c   | 120 +++++++++++++++++++++++++++++++++++++++++++--
 fs/btrfs/props.h   |   4 +-
 fs/btrfs/volumes.c |  25 +++++++++-
 fs/btrfs/volumes.h |   8 +++
 fs/btrfs/xattr.c   |   2 +-
 5 files changed, 150 insertions(+), 9 deletions(-)

Anand Jain (2):
  btrfs-progs: add helper to create xattr name
  btrfs-progs: add readmirror policy

 props.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 68 insertions(+), 7 deletions(-)
-- 
2.20.1 (Apple Git-117)

[PATCH 0/3] readmirror feature

[Anand Jain]

These patches are tested to be working fine.

Function call chain  __btrfs_map_block()->find_live_mirror() uses
thread pid to determine the %mirror_num when the mirror_num=0.

This patch introduces a framework so that we can add policies to determine
the %mirror_num. And adds the devid as the readmirror policy.

The property is stored as an extented attributes of root inode
(BTRFS_FS_TREE_OBJECTID).
User provided devid list is validated against the fs_devices::dev_list.

 For example:
   Usage:
     btrfs property set <mnt> readmirror devid<n>[,<m>...]
     btrfs property set <mnt> readmirror ""

   mkfs.btrfs -fq -draid1 -mraid1 /dev/sd[b-d] && mount /dev/sdb /btrfs
   btrfs prop set /btrfs readmirror devid1,2
   btrfs prop get /btrfs readmirror
    readmirror=devid1,2
   getfattr -n btrfs.readmirror --absolute-names /btrfs
    btrfs.readmirror="devid1,2"
   btrfs prop set /btrfs readmirror ""
   getfattr -n btrfs.readmirror --absolute-names /btrfs
    /btrfs: btrfs.readmirror: No such attribute
   btrfs prop get /btrfs readmirror

RFC->v1:
  Drops pid as one of the readmirror policy choices and as usual remains
  as default. And when the devid is reset the readmirror policy falls back
  to pid.
  Drops the mount -o readmirror idea, it can be added at a later point of
  time.
  Property now accepts more than 1 devid as readmirror device. As shown
  in the example above.

Anand Jain (3):
  btrfs: add inode pointer to prop_handler::validate()
  btrfs: add readmirror property framework
  btrfs: add readmirror devid property

 fs/btrfs/props.c   | 120 +++++++++++++++++++++++++++++++++++++++++++--
 fs/btrfs/props.h   |   4 +-
 fs/btrfs/volumes.c |  25 +++++++++-
 fs/btrfs/volumes.h |   8 +++
 fs/btrfs/xattr.c   |   2 +-
 5 files changed, 150 insertions(+), 9 deletions(-)

Anand Jain (2):
  btrfs-progs: add helper to create xattr name
  btrfs-progs: add readmirror policy

 props.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 68 insertions(+), 7 deletions(-)
-- 
2.20.1 (Apple Git-117)