On 01/31/2018 05:54 PM, Nikolay Borisov wrote:


On 31.01.2018 11:28, Anand Jain wrote:


On 01/31/2018 04:38 PM, Nikolay Borisov wrote:


On 30.01.2018 08:30, Anand Jain wrote:
Adds the mount option:
    mount -o read_mirror_policy=<devid>

To set the devid of the device which should be used for read. That
means all the normal reads will go to that particular device only.

This also helps testing and gives a better control for the test
scripts including mount context reads.

Some code comments below. OTOH, does such policy really make sense, what
happens if the selected device fails, will the other mirror be retried?

  Everything as usual, read_mirror_policy=devid just lets the user to
  specify his read optimized disk, so that we don't depend on the pid
  to pick a stripe mirrored disk, and instead we would pick as suggested
  by the user, and if that disk fails then we go back to the other mirror
  which may not be the read optimized disk as we have no other choice.

If the answer to the previous question is positive then why do we really
care which device is going to be tried first?

  It matters.
    - If you are reading from both disks alternatively, then it
      duplicates the LUN cache on the storage.
    - Some disks are read-optimized and using that for reading and going
      back to the other disk only when this disk fails provides a better
      overall read performance.

So usually this should be functionality handled by the raid/san
controller I guess, > but given that btrfs is playing the role of a
controller here at what point are we drawing the line of not
implementing block-level functionality into the filesystem ?

 Don't worry this is not invading into the block layer. How
 can you even build this functionality in the block layer ?
 Block layer even won't know that disks are mirrored. RAID
 does or BTRFS in our case.

::
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 39ba59832f38..478623e6e074 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5270,6 +5270,16 @@ static int find_live_mirror(struct
btrfs_fs_info *fs_info,
           num = map->num_stripes;
         switch(fs_info->read_mirror_policy) {
+    case BTRFS_READ_MIRROR_BY_DEV:
+        optimal = first;
+        if (test_bit(BTRFS_DEV_STATE_READ_MIRROR,
+                 &map->stripes[optimal].dev->dev_state))
+            break;
+        if (test_bit(BTRFS_DEV_STATE_READ_MIRROR,
+                 &map->stripes[++optimal].dev->dev_state))
+            break;
+        optimal = first;

you set optimal 2 times, the second one seems redundant.

  No actually. When both the disks containing the stripe does not
  have the BTRFS_DEV_STATE_READ_MIRROR, then I would just want to
  use first found stripe.

Yes, and the fact that you've already set optimal = first right after
BTRFS_READ_MIRROR_BY_DEV ensures that, no ? Why do you need to again set
optimal right before the final break? What am I missing here?

  Ah. I think you are missing ++optimal in the 2nd if.

Thanks, Anand
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to