date:20140206

Re: [PATCH 1/2] btrfs: add small program for clone testing

2014-02-06 Thread David Disseldorp

Hi Dave,

On Thu, 6 Feb 2014 10:09:36 +1100, Dave Chinner wrote:

 On Wed, Feb 05, 2014 at 12:16:48PM +0100, David Disseldorp wrote:
  The cloner program is capable of cloning files using the BTRFS_IOC_CLONE
  and BTRFS_IOC_CLONE_RANGE ioctls.
  
  Signed-off-by: David Disseldorp dd...@suse.de
 
 Hi Dave - long time since I've seen your head pop up around here ;)

Indeed, it's been a while. Thanks for the review :)

 
 A few comments below.
 
  +struct btrfs_ioctl_clone_range_args {
  +   int64_t src_fd;
  +   uint64_t src_offset;
  +   uint64_t src_length;
  +   uint64_t dest_offset;
  +};
  +
  +#define BTRFS_IOCTL_MAGIC 0x94
  +#define BTRFS_IOC_CLONE   _IOW(BTRFS_IOCTL_MAGIC, 9, int)
  +#define BTRFS_IOC_CLONE_RANGE _IOW(BTRFS_IOCTL_MAGIC, 13, \
  +  struct btrfs_ioctl_clone_range_args)
 
 Is there some published header file that these belong to? i.e.
 somewhere in the include/linux/uapi/ kernel directory? Normally the
 way to handle this sort of thing is by autoconf - if the header file
 exists, then we include it, otherwise we use the manual definitions.
 This just means that if the public api ever changes, we'll pick it
 up automatically in future...

I'd wanted to avoid the addition of another xfsqa prereq, but I guess
it'll work with the fall-back.
I'll add the autoconf logic to the next round, along with changes
addressing your other remarks.

Cheers, David
--
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 v2 1/2 ] Btrfs: switch to btrfs_previous_extent_item()

2014-02-06 Thread Wang Shilong

From: Wang Shilong wangsl.f...@cn.fujitsu.com

Since we have introduced btrfs_previous_extent_item() to search previous
extent item, just switch into it.

Signed-off-by: Wang Shilong wangsl.f...@cn.fujitsu.com
---
Changelog:
v1-v2:
set @found_key properly and fix return value(thanks to Filipe)
---
 fs/btrfs/backref.c | 37 ++---
 1 file changed, 6 insertions(+), 31 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index c39891c..4dedb89 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -1325,38 +1325,13 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, 
u64 logical,
if (ret  0)
return ret;
 
-   while (1) {
-   u32 nritems;
-   if (path-slots[0] == 0) {
-   btrfs_set_path_blocking(path);
-   ret = btrfs_prev_leaf(fs_info-extent_root, path);
-   if (ret != 0) {
-   if (ret  0) {
-   pr_debug(logical %llu is not within 
-any extent\n, logical);
-   ret = -ENOENT;
-   }
-   return ret;
-   }
-   } else {
-   path-slots[0]--;
-   }
-   nritems = btrfs_header_nritems(path-nodes[0]);
-   if (nritems == 0) {
-   pr_debug(logical %llu is not within any extent\n,
-logical);
-   return -ENOENT;
-   }
-   if (path-slots[0] == nritems)
-   path-slots[0]--;
-
-   btrfs_item_key_to_cpu(path-nodes[0], found_key,
- path-slots[0]);
-   if (found_key-type == BTRFS_EXTENT_ITEM_KEY ||
-   found_key-type == BTRFS_METADATA_ITEM_KEY)
-   break;
+   ret = btrfs_previous_extent_item(fs_info-extent_root, path, 0);
+   if (ret) {
+   if (ret  0)
+   ret = -ENOENT;
+   return ret;
}
-
+   btrfs_item_key_to_cpu(path-nodes[0], found_key, path-slots[0]);
if (found_key-type == BTRFS_METADATA_ITEM_KEY)
size = fs_info-extent_root-leafsize;
else if (found_key-type == BTRFS_EXTENT_ITEM_KEY)
-- 
1.8.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 v2 1/2 ] Btrfs: switch to btrfs_previous_extent_item()

2014-02-06 Thread Filipe David Manana

On Thu, Feb 6, 2014 at 12:02 PM, Wang Shilong wangshilong1...@gmail.com wrote:
 From: Wang Shilong wangsl.f...@cn.fujitsu.com

 Since we have introduced btrfs_previous_extent_item() to search previous
 extent item, just switch into it.

 Signed-off-by: Wang Shilong wangsl.f...@cn.fujitsu.com

Reviewed-by: Filipe Manana fdman...@gmail.com

Thanks Shilong.

 ---
 Changelog:
 v1-v2:
 set @found_key properly and fix return value(thanks to Filipe)
 ---
  fs/btrfs/backref.c | 37 ++---
  1 file changed, 6 insertions(+), 31 deletions(-)

 diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
 index c39891c..4dedb89 100644
 --- a/fs/btrfs/backref.c
 +++ b/fs/btrfs/backref.c
 @@ -1325,38 +1325,13 @@ int extent_from_logical(struct btrfs_fs_info 
 *fs_info, u64 logical,
 if (ret  0)
 return ret;

 -   while (1) {
 -   u32 nritems;
 -   if (path-slots[0] == 0) {
 -   btrfs_set_path_blocking(path);
 -   ret = btrfs_prev_leaf(fs_info-extent_root, path);
 -   if (ret != 0) {
 -   if (ret  0) {
 -   pr_debug(logical %llu is not within 
 -any extent\n, logical);
 -   ret = -ENOENT;
 -   }
 -   return ret;
 -   }
 -   } else {
 -   path-slots[0]--;
 -   }
 -   nritems = btrfs_header_nritems(path-nodes[0]);
 -   if (nritems == 0) {
 -   pr_debug(logical %llu is not within any extent\n,
 -logical);
 -   return -ENOENT;
 -   }
 -   if (path-slots[0] == nritems)
 -   path-slots[0]--;
 -
 -   btrfs_item_key_to_cpu(path-nodes[0], found_key,
 - path-slots[0]);
 -   if (found_key-type == BTRFS_EXTENT_ITEM_KEY ||
 -   found_key-type == BTRFS_METADATA_ITEM_KEY)
 -   break;
 +   ret = btrfs_previous_extent_item(fs_info-extent_root, path, 0);
 +   if (ret) {
 +   if (ret  0)
 +   ret = -ENOENT;
 +   return ret;
 }
 -
 +   btrfs_item_key_to_cpu(path-nodes[0], found_key, path-slots[0]);
 if (found_key-type == BTRFS_METADATA_ITEM_KEY)
 size = fs_info-extent_root-leafsize;
 else if (found_key-type == BTRFS_EXTENT_ITEM_KEY)
 --
 1.8.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



-- 
Filipe David Manana,

Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men.
--
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: Provide a better free space estimate on RAID1

2014-02-06 Thread Roman Mamedov

On Thu, 06 Feb 2014 09:38:15 +0200
Brendan Hide bren...@swiftspirit.co.za wrote:

This is a known issue:
https://btrfs.wiki.kernel.org/index.php/FAQ#Why_does_df_show_incorrect_free_space_for_my_RAID_volume.3F
Btrfs is still considered experimental

It's long overdue to start tackling these snags and 'stop hiding behind the
experimental flag' [1], which is also no longer present as of 3.13.

[1] http://www.spinics.net/lists/linux-btrfs/msg30396.html

this is just one of those caveats we've learned to adjust to.

Sure, but it's hard to argue this particular behavior is clearly broken from
the user perspective, even if it's broken by design, and there are a number
of very smart and future-proof reasons to keep it broken today.

Personally I tired of trying to keep in mind which partitions are btrfs raid1,
and mentally divide the displayed free space by two for those. That's what
computers are for, to keep track of such things.

The change could work well for now and I'm sure it has been considered.
I guess the biggest end-user issue is that you can, at a whim, change
the model for new blocks - raid0/5/6,single etc and your value from 5
minutes ago is far out from your new value without having written
anything or taken up any space. Not a show-stopper problem, really.

Changing the allocation profile for new blocks is a serious change you don't do
accidentally, it's about the same importance level as e.g. resizing the
filesystem. And no one is really surprised when the 'df' result changes after
an FS resize.

The biggest dev issue is that future features will break this behaviour,

That could be years away.

such as the per-subvolume RAID profiles you mentioned. It is difficult
to motivate including code (for which there's a known workaround) where
we know it will be obsoleted.

There's not a lot of code to include (as my 3-line patch demonstrates), it
could just as easily be removed when it's obsolete. But I did not have any
high hopes of defeating the broken by design philosophy, that's why I didn't
submit it as a real patch for inclusion but rather just as a helpful hint for
people to add to their own kernels if they want this change to happen.

48 matches

Mail list logo