All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Davies <btrfs-list@steev.me.uk>
To: Anand Jain <anand.jain@oracle.com>, David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2 1/3] btrfs: add readmirror type framework
Date: Fri, 3 Jan 2020 14:51:07 +0000	[thread overview]
Message-ID: <d9e23ee8-a1bc-b79f-60c4-9fa19d9e5592@steev.me.uk> (raw)
In-Reply-To: <e652f476-eef8-62d1-4a3d-b01dbce2677a@oracle.com>

On 03/01/2020 10:28, Anand Jain wrote:
> On 3/1/20 3:32 AM, Steven Davies wrote:
>> On 02/01/2020 10:12, Anand Jain wrote:

>>> So this patch introduces a framework where we could add more readmirror
>>> policies, such as routing the IO based on device's wait-queue or manual
>>> when we have a read-preferred device or a policy based on the target
>>> storage caching.
>>
>> I think the idea is good but that it would be cleaner if the tunable 
>> was named read_policy rather than readmirror as it's more obvious that 
>> it contains a policy tunable.
> 
>   Um. 'read_policy' sounds good, but I hope it is clear enough to
>   indicate that we are talking about read for only mirrored-chunks.
>   Will rename to read_policy.

I think it would be obvious that the policy will only apply to mirrored 
chunks; after all, you can only read a chunk from a device that contains it.

>> Do you envisage allowing more than one policy to be active for a 
>> filesystem? If not, what about using the same structure as the CPU 
>> frequency and block IO schedulers with the format
>>
>> #cat /sys/block/sda/queue/scheduler
>> noop [deadline] cfq
>>
>> Such that btrfs would (eventually) have something like
>>
>> #cat /sys/fs/btrfs/UUID/read_policy
>> by_pid [user_defined_device] by_shortest_queue
>>
> 
>   And in case of user_defined_device, the device for the read shall be
>   specified in
> 
>    cat /sys/fs/btrfs/UUID/devinfo/<devid>/read_preferred
> 
>    0 = unset, 1 = set.
> 
>    (devinfo patches are in the ML [1] open for comment)
>    [1]
>    [PATCH v3 4/4] btrfs: sysfs, add devid/dev_state kobject and attribute

I remember seeing that patch and I think the approach is logical.

Steve

  reply	other threads:[~2020-01-03 14:51 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-02 10:12 [PATCH v2 0/3] readmirror feature (sysfs and in-memory only approach) Anand Jain
2020-01-02 10:12 ` [PATCH v2 1/3] btrfs: add readmirror type framework Anand Jain
2020-01-02 16:24   ` Josef Bacik
2020-01-03  9:57     ` Anand Jain
2020-01-02 19:32   ` Steven Davies
2020-01-03 10:28     ` Anand Jain
2020-01-03 14:51       ` Steven Davies [this message]
2020-01-03 10:31   ` [PATCH v3 " Anand Jain
2020-01-02 10:12 ` [PATCH v2 2/3] btrfs: sysfs, add readmirror kobject Anand Jain
2020-01-02 10:12 ` [PATCH v2 3/3] btrfs: sysfs, create by_pid readmirror attribute 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=d9e23ee8-a1bc-b79f-60c4-9fa19d9e5592@steev.me.uk \
    --to=btrfs-list@steev.me.uk \
    --cc=anand.jain@oracle.com \
    --cc=dsterba@suse.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.