Re: [PATCH] btrfs: don't add both copies of DUP to reada extent tree

2012-03-15 Thread Andrea Gelmini
On Sat, Feb 25, 2012 at 09:09:47AM +0100, Arne Jansen wrote:
 Normally when there are 2 copies of a block, we add both to the
 reada extent tree and prefetch only the one that is easier to reach.
 This way we can better utilize multiple devices.
 In case of DUP this makes no sense as both copies reside on the
 same device.

I'm using this patch without problem since you published it (compressed
/home with hourly snapshot delete/creation).

Thanks a lot for your work,
Andrea
--
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


[PATCH] btrfs: don't add both copies of DUP to reada extent tree

2012-02-25 Thread Arne Jansen
Normally when there are 2 copies of a block, we add both to the
reada extent tree and prefetch only the one that is easier to reach.
This way we can better utilize multiple devices.
In case of DUP this makes no sense as both copies reside on the
same device.

Signed-off-by: Arne Jansen sensi...@gmx.net
---
 fs/btrfs/reada.c |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
index 85ce503..8127ae9 100644
--- a/fs/btrfs/reada.c
+++ b/fs/btrfs/reada.c
@@ -325,6 +325,7 @@ static struct reada_extent *reada_find_extent(struct 
btrfs_root *root,
struct btrfs_mapping_tree *map_tree = fs_info-mapping_tree;
struct btrfs_bio *bbio = NULL;
struct btrfs_device *dev;
+   struct btrfs_device *prev_dev;
u32 blocksize;
u64 length;
int nzones = 0;
@@ -404,8 +405,20 @@ static struct reada_extent *reada_find_extent(struct 
btrfs_root *root,
spin_unlock(fs_info-reada_lock);
goto error;
}
+   prev_dev = NULL;
for (i = 0; i  nzones; ++i) {
dev = bbio-stripes[i].dev;
+   if (dev == prev_dev) {
+   /*
+* in case of DUP, just add the first zone. As both
+* are on the same device, there's nothing to gain
+* from adding both.
+* Also, it wouldn't work, as the tree is per device
+* and adding would fail with EEXIST
+*/
+   continue;
+   }
+   prev_dev = dev;
ret = radix_tree_insert(dev-reada_extents, index, re);
if (ret) {
while (--i = 0) {
-- 
1.7.3.4

--
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


Re: [PATCH] btrfs: don't add both copies of DUP to reada extent tree

2012-02-25 Thread Duncan
Arne Jansen posted on Sat, 25 Feb 2012 09:09:47 +0100 as excerpted:

 Normally when there are 2 copies of a block, we add both to the reada
 extent tree and prefetch only the one that is easier to reach.
 This way we can better utilize multiple devices.
 In case of DUP this makes no sense as both copies reside on the same
 device.

I'm not a coder and thus can't really read the code to know, but wouldn't 
the same easier-to-reach logic apply there, only to seeking?  One of the 
DUPs should be easier to reach (less seeking) than the other.

-- 
Duncan - List replies preferred.   No HTML msgs.
Every nonfree program has a lord, a master --
and if you use the program, he is your master.  Richard Stallman

--
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


Re: [PATCH] btrfs: don't add both copies of DUP to reada extent tree

2012-02-25 Thread Arne Jansen

On 02/25/12 09:33, Duncan wrote:

Arne Jansen posted on Sat, 25 Feb 2012 09:09:47 +0100 as excerpted:


Normally when there are 2 copies of a block, we add both to the reada
extent tree and prefetch only the one that is easier to reach.
This way we can better utilize multiple devices.
In case of DUP this makes no sense as both copies reside on the same
device.


I'm not a coder and thus can't really read the code to know, but wouldn't
the same easier-to-reach logic apply there, only to seeking?  One of the
DUPs should be easier to reach (less seeking) than the other.



Well, the commit message is kind of sloppy. The reada code collects
as many blocks near to each other and reads them sequentially. From
time to time it moves on to the next filled zone. That's when a longer
seek happens. It doesn't try to keep these longer seeks as short as
possible, instead it picks a zone on the disk where it has many
blocks to read.
As both parts of a DUP are filled equally, it doesn't matter which one
it picks, so it is sufficient to keep track of only one half of the
mirror. Things change in a multi-disk setup, as the heads can move
independently.

(this explanation is kind of sloppy, too)

-Arne
--
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


Re: [PATCH] btrfs: don't add both copies of DUP to reada extent tree

2012-02-25 Thread Duncan
Arne Jansen posted on Sat, 25 Feb 2012 14:09:50 +0100 as excerpted:

 On 02/25/12 09:33, Duncan wrote:
 Arne Jansen posted on Sat, 25 Feb 2012 09:09:47 +0100 as excerpted:

 Normally when there are 2 copies of a block, we add both to the reada
 extent tree and prefetch only the one that is easier to reach.
 This way we can better utilize multiple devices.
 In case of DUP this makes no sense as both copies reside on the same
 device.

 I'm not a coder and thus can't really read the code to know, but
 wouldn't the same easier-to-reach logic apply there, only to seeking? 
 One of the DUPs should be easier to reach (less seeking) than the
 other.


 Well, the commit message is kind of sloppy. The reada code collects as
 many blocks near to each other and reads them sequentially. From time to
 time it moves on to the next filled zone. That's when a longer seek
 happens. It doesn't try to keep these longer seeks as short as possible,
 instead it picks a zone on the disk where it has many blocks to read.
 As both parts of a DUP are filled equally, it doesn't matter which one
 it picks, so it is sufficient to keep track of only one half of the
 mirror. Things change in a multi-disk setup, as the heads can move
 independently.
 
 (this explanation is kind of sloppy, too)

Thanks.  Makes sense, now.

-- 
Duncan - List replies preferred.   No HTML msgs.
Every nonfree program has a lord, a master --
and if you use the program, he is your master.  Richard Stallman

--
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