From: Anand Jain <anand.jain@oracle.com>
To: dsterba@suse.cz, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH RFC v2 0/2] readmirror feature
Date: Thu, 12 Sep 2019 15:48:07 +0800 [thread overview]
Message-ID: <5813d7aa-bfa0-698d-5c51-cf29e7c5c945@oracle.com> (raw)
In-Reply-To: <20190911163758.GG2850@twin.jikos.cz>
On 12/9/19 12:37 AM, David Sterba wrote:
> On Mon, Aug 26, 2019 at 05:04:36PM +0800, Anand Jain wrote:
>> 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 also adds the devid as the readmirror policy.
>>
>> The new property is stored as an item in the device tree as show below.
>> (BTRFS_READMIRROR_OBJECTID, BTRFS_PERSISTENT_ITEM_KEY, devid)
>>
>> To be able to set and get this new property also introduces new ioctls
>> BTRFS_IOC_GET_READMIRROR and BTRFS_IOC_SET_READMIRROR. The ioctl argument
>> is defined as
>> struct btrfs_ioctl_readmirror_args {
>> __u64 type; /* RW */
>> __u64 device_bitmap; /* RW */
>> }
>
> I don't remember if there was a suggestion to use ioctls for read
> mirror, but the property interface should be sufficient. Besides this
> ioctl interafce is quite an anti-pattern: narrow use, non-extensible
> structure, alternative and more convenient interface already available.
Extended attribute interface f(get/set)attr is inode bound, but
the readmirror property is filesystem bound. For the readmirror we
can still use the extended attribute, but it might be considered as
abuse which we haven't done so far, here below [1] is the list of
property with the interface it uses and where the property is saved.
[1]
property interface save-as
ro ioctl root-item
label ioctl super-block
compression xattr xattr
v1:readmirror xattr xattr
v2:readmirror ioctl dev-tree-item
You are asking for the combination of
property interface save-as
readmirror xattr dev-tree-item
I can give a try.
>> An usage example as follows:
>> btrfs property set /btrfs readmirror devid:1,3
>> btrfs property get /btrfs readmirror
>> readmirror devid:1 3
>> btrfs property set /btrfs readmirror ""
>> btrfs property get /btrfs readmirror
>> readmirror default
>>
>> This patchset has been tested completely, however marked as RFC for the
>> following reasons and comments on them (or any other) are appreciated as
>> usual.
>> . The new objectid is defined as
>> #define BTRFS_READMIRROR_OBJECTID -1ULL
>> Need consent we are fine to use this value, and with this value it
>> shall be placed just before the DEV_STATS_OBJECTID item which is more
>> frequently used only during the device errors.
>
> -1 can be considered a special value in other cases, not necessarily
> here but if the ordering of items is the only reason I'd say no. The
> keys/items will most likely land in the same node so there's no point
> forcing the order.
Agreed. Any suggestion on the value for the BTRFS_READMIRROR_OBJECTID.
>> . I am using a u64 bitmap to represent the devices id, so the max device
>> id that we could represent is 63, its a kind of limitation which should
>> be addressed before integration, I wonder if there is any suggestion?
>
> Uh 63, so now an implementation detail is posing a global limit? That
> sounds backwards.
Yes. And I was thinking of u64 array but that doesn't scale as well.
Anyways I have in the list to try using xattr interface which may
address this issue.
>> Kindly note that, multiple ioctls with each time representing a set of
>> device(s) is not a choice because we need to make sure the readmirror
>> changes happens in a commit transaction.
>
> I believe this can be guaranteed by the properties interface, ie. single
> value gets processed at once and with some locking around the state of
> devices can be updated atomically.
>
> The open question is still how to store the readmirror property
> per-device, it could be either an item or bit inside the device
> structure.
We discussed that before here.
https://lore.kernel.org/linux-btrfs/8d31c3a2-3fb0-63af-3173-948ed0e84de3@oracle.com/
> Besides the interface, I'm kind of missing the usecase description what
> is expected from the read mirror policies and how they could affect
> various scenarios. Maybe it was in some of the previous iterations, it's
> hard too track everything so this should be part of the cover letter (or
> at leat linked if it's too much text).
>
Yep. My mistake I missed it link it. Sorry about that.
Thanks, Anand
next prev parent reply other threads:[~2019-09-12 7:48 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-26 9:04 [PATCH RFC v2 0/2] readmirror feature Anand Jain
2019-08-26 9:04 ` [PATCH RFC v2] btrfs: add readmirror framework and policy devid Anand Jain
2019-08-26 9:04 ` [PATCH RFC v2] btrfs-progs: add readmirror property and ioctl to set get readmirror Anand Jain
2019-08-29 3:39 ` [PATCH RFC v2.1] btrfs-progs: add readmirror property and ioctl to set get Anand Jain
2019-09-11 16:37 ` [PATCH RFC v2 0/2] readmirror feature David Sterba
2019-09-12 7:48 ` Anand Jain [this message]
2019-09-11 18:42 ` Josef Bacik
2019-09-11 19:13 ` Eli V
2019-09-11 19:16 ` Josef Bacik
2019-09-12 7:41 ` Anand Jain
2019-09-12 9:50 ` Josef Bacik
2019-09-12 10:00 ` Anand Jain
2019-09-12 10:03 ` Josef Bacik
2019-09-12 10:10 ` Anand Jain
2019-09-12 10:13 ` Josef Bacik
2019-09-16 8:19 ` Anand Jain
2019-09-24 14:27 ` Anand Jain
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5813d7aa-bfa0-698d-5c51-cf29e7c5c945@oracle.com \
--to=anand.jain@oracle.com \
--cc=dsterba@suse.cz \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).