Re: [PATCH 1/2] Btrfs: don't make a file partly checksummed through file clone

2011-09-15 Thread David Sterba 
On Wed, Sep 14, 2011 at 01:25:21PM +0800, Li Zefan wrote:
 It's because part of the file is checksummed and the other part is not,
 and then btrfs will complain checksum is not found when we read the file.
 
 Disallow file clone if src and dst file have different checksum flag,
 so we ensure a file is completely checksummed or unchecksummed.

Your fix prevents the bug, but I don't think it's good to let file clone
fail without any other message. ret is set to -EINVAL at the time of
'goto out_fput', which is fine, but the user has no clue what happened
or how to fix it.

The nodatasum status is recorded in inode flags and remains like that
regardless of the 'mount -o nodatasum', persistent and de facto
unchangable (unless the file is created again with the opposite nodatasum
mount). Even more, the user has no way to find out nodatasum flag of
any inode/file (the corresponding FS_NODATASUM_FL is not there).

My suggestion how to fix this:
1. add FS_NODATASUM_FL file flag and code to set/get via setflags ioctl
2. [this patch to skip cloning in case of nodatasum flag mismatch]
3. ... add a printk why it failed

The user then has at least option to drop/add the nodatasum flag for one of
the. Unfortunatelly this makes file cloning less straightforward.


david


 
 Signed-off-by: Li Zefan l...@cn.fujitsu.com
 ---
  fs/btrfs/ioctl.c |5 +
  1 files changed, 5 insertions(+), 0 deletions(-)
 
 diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
 index 970977a..dc82bbb 100644
 --- a/fs/btrfs/ioctl.c
 +++ b/fs/btrfs/ioctl.c
 @@ -2177,6 +2177,11 @@ static noinline long btrfs_ioctl_clone(struct file 
 *file, unsigned long srcfd,
   if (!(src_file-f_mode  FMODE_READ))
   goto out_fput;
  
 + /* don't make the dst file partly checksummed */
 + if ((BTRFS_I(src)-flags  BTRFS_INODE_NODATASUM) !=
 + (BTRFS_I(inode)-flags  BTRFS_INODE_NODATASUM))
 + goto out_fput;
 +
   ret = -EISDIR;
   if (S_ISDIR(src-i_mode) || S_ISDIR(inode-i_mode))
   goto out_fput;
 -- 1.7.3.1 
 --
 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
--
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 2/2] Btrfs: don't change inode flag of the dest clone file

2011-09-15 Thread David Sterba 
On Wed, Sep 14, 2011 at 01:25:36PM +0800, Li Zefan wrote:
 The dst file will have the same inode flags with dst file after
 file clone, and I think it's unexpected.
 
 For example, the dst file will suddenly become immutable after
 getting some share of data with src file, if the src is immutable.

Good catch! (I did not find any further direct assignment of two inode
flags.)

 
 Signed-off-by: Li Zefan l...@cn.fujitsu.com
Reviewed-by: David Sterba dste...@suse.cz

IMNSHO should go to stable.


david

 ---
  fs/btrfs/ioctl.c |1 -
  1 files changed, 0 insertions(+), 1 deletions(-)
 
 diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
 index dc82bbb..a401514 100644
 --- a/fs/btrfs/ioctl.c
 +++ b/fs/btrfs/ioctl.c
 @@ -2434,7 +2434,6 @@ static noinline long btrfs_ioctl_clone(struct file 
 *file, unsigned long srcfd,
   if (endoff  inode-i_size)
   btrfs_i_size_write(inode, endoff);
  
 - BTRFS_I(inode)-flags = BTRFS_I(src)-flags;
   ret = btrfs_update_inode(trans, root, inode);
   BUG_ON(ret);
   btrfs_end_transaction(trans, root);
 -- 1.7.3.1 
 --
 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
--
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: trivial fix, a potential memory leak in btrfs_parse_early_options()

2011-09-15 Thread David Sterba 
On Wed, Sep 14, 2011 at 02:11:21PM +0800, Jeff Liu wrote:
 On 09/14/2011 01:40 PM, Li Zefan wrote:
 
  14:06, Jeff Liu wrote:
  Signed-off-by: Jie Liu jeff@oracle.com
 
  ---
   fs/btrfs/super.c |   10 --
   1 files changed, 8 insertions(+), 2 deletions(-)
 
  diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
  index 15634d4..16f31e1 100644
  --- a/fs/btrfs/super.c
  +++ b/fs/btrfs/super.c
  @@ -406,7 +406,7 @@ static int btrfs_parse_early_options(const char 
  *options, fmode_t flags,
   u64 *subvol_rootid, struct btrfs_fs_devices **fs_devices)
   {
   substring_t args[MAX_OPT_ARGS];
  -char *opts, *orig, *p;
  +char *device_name, *opts, *orig, *p;
  
  Seems your email client replaced tabs with spaces.
 
 Fixed, thank you.
 
 Signed-off-by: Jie Liu jeff@oracle.com
 
 ---
  fs/btrfs/super.c |   10 --
  1 files changed, 8 insertions(+), 2 deletions(-)
 
 diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
 index 15634d4..16f31e1 100644
 --- a/fs/btrfs/super.c
 +++ b/fs/btrfs/super.c
 @@ -406,7 +406,7 @@ static int btrfs_parse_early_options(const char
 *options, fmode_t flags,
  

long lines are still getting wrapped.

   u64 *subvol_rootid, struct btrfs_fs_devices **fs_devices)
  {
   substring_t args[MAX_OPT_ARGS];
 - char *opts, *orig, *p;
 + char *device_name, *opts, *orig, *p;
   int error = 0;
   int intarg;
 
 @@ -457,8 +457,14 @@ static int btrfs_parse_early_options(const char
 *options, fmode_t flags,
  

   }
   break;
   case Opt_device:
 - error = btrfs_scan_one_device(match_strdup(args[0]),
 + device_name = match_strdup(args[0]);
 + if (!device_name) {
 + error = -ENOMEM;
 + goto out_free_opts;
 + }
 + error = btrfs_scan_one_device(device_name,
   flags, holder, fs_devices);
 + kfree(device_name);
   if (error)
   goto out_free_opts;
   break;
 -- 
 1.7.4.1

and you do not need to keep unrelated replies and signatures (like the
following quoted text). Just send the patch again with proper changelog
and add a version tag eg

[PATCH v2] btrfs: ...

It's really annoying to hand fix corrupted patches from mailinglist, 

 
 
  
  Please read Documentation/email-clients.txt
  
   int error = 0;
   int intarg;
 
  @@ -457,8 +457,14 @@ static int btrfs_parse_early_options(const char 
  *options, fmode_t flags,
   }
   break;
   case Opt_device:
  -error = btrfs_scan_one_device(match_strdup(args[0]),
  +device_name = match_strdup(args[0]);
  +if (!device_name) {
  +error = -ENOMEM;
  +goto out_free_opts;
  +}
  +error = btrfs_scan_one_device(device_name,
   flags, holder, fs_devices);
  +kfree(device_name);
   if (error)
   goto out_free_opts;
   break;
  --
  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
 
 
 --
 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
--
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] btrfs: trivial fix, a potential memory leak in btrfs_parse_early_options()

2011-09-15 Thread Jeff Liu 
Signed-off-by: Jie Liu jeff@oracle.com

---
 fs/btrfs/super.c |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 15634d4..16f31e1 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -406,7 +406,7 @@ static int btrfs_parse_early_options(const char *options, 
fmode_t flags,
u64 *subvol_rootid, struct btrfs_fs_devices **fs_devices)
 {
substring_t args[MAX_OPT_ARGS];
-   char *opts, *orig, *p;
+   char *device_name, *opts, *orig, *p;
int error = 0;
int intarg;
 
@@ -457,8 +457,14 @@ static int btrfs_parse_early_options(const char *options, 
fmode_t flags,
}
break;
case Opt_device:
-   error = btrfs_scan_one_device(match_strdup(args[0]),
+   device_name = match_strdup(args[0]);
+   if (!device_name) {
+   error = -ENOMEM;
+   goto out_free_opts;
+   }
+   error = btrfs_scan_one_device(device_name,
flags, holder, fs_devices);
+   kfree(device_name);
if (error)
goto out_free_opts;
break;
-- 
1.7.4.1
--
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: Rename BTRfs to MuchSlowerFS ?

2011-09-15 Thread Swâmi Petaramesh 
Le Mercredi 7 Septembre 2011 00:11:25 vous avez écrit :
 Reading your post, at this point I'd actually recommend you stick with
 ext4.

I actually shifted back from BTRFS to ext4 and fell like having offered myself 
a brand new computer,  about 20 times faster, me happy ;-)

 Both btrfs and zfs are great, but IMHO btrfs is not ready for
 daily use by ordinary user yet, while zfs is a memory hog
 (especially for laptops, which is part of the reason why I'm using
 btrfs instead of zfs on this one).

True, ZFS is excellent but a memory hog (and strongly advises using a 64-bit 
OS) but I was surprised to discover that BTRFS was such a memory eater itself, 
with kernel 3.0. My system was swapping like mad !

I use (kernel) ZFS on my 64-bit main machine and I'm plain happy with it, and 
tried ZFS on my 32-bit laptop in the hope to get more performance for less 
memory ; alas I just got a memory-hungry system running damn slow... For the 
time being I will stick to ZFS for 64-bit machines with = 4GB RAM, and to 
ext4 for 32-bit systems with less RAM...

I don't feel that BTRFS gives any advantage in its current state of 
development. Alas.

-- 
Swâmi Petaramesh sw...@petaramesh.org http://petaramesh.org PGP 9076E32E

--
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 0/20] btrfs: More error handling fixes

2011-09-15 Thread Mark Fasheh 
Hi,

The following are assorted fixes to error handling from all parts of the
Btrfs code.  I included in this series an uncommited patch from Tsutomu Itoh
which was a better version of a patch I had written.  He should be cc'd on
that mail.

Some of the patches (the first 8) were previously sent to this list but got
no comments so I'm resending them with this set. Changes from last time are
that I also started setting the file system readonly on errors which look
like possible corruption.

For the most part, I'm still concentrating on eliminating sites where we
BUG_ON(ret) instead of bubbling errors up the stack. The patches were
tested using some simple file system commands and a background kernel build.

Please review, all constructive feedback is appreciated :)

Thanks,
--Mark

--
Mark Fasheh
--
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 01/20] btrfs: Don't BUG_ON errors from btrfs_create_subvol_root()

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

This is called from only one place - create_subvol() which passes errors
safely back out to it's caller, btrfs_mksubvol where they are handled.

Additionally, btrfs_create_subvol_root() itself bug's needlessly from error
return of btrfs_update_inode(). Since create_subvol() was fixed to catch
errors we can bubble this one up too.

Signed-off-by: Mark Fasheh mfas...@suse.com
---
 fs/btrfs/inode.c |3 +--
 fs/btrfs/ioctl.c |2 ++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 15fceef..7028c0c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6722,10 +6722,9 @@ int btrfs_create_subvol_root(struct btrfs_trans_handle 
*trans,
btrfs_i_size_write(inode, 0);
 
err = btrfs_update_inode(trans, new_root, inode);
-   BUG_ON(err);
 
iput(inode);
-   return 0;
+   return err;
 }
 
 struct inode *btrfs_alloc_inode(struct super_block *sb)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 7cf0133..b3440f5 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -411,6 +411,8 @@ static noinline int create_subvol(struct btrfs_root *root,
btrfs_record_root_in_trans(trans, new_root);
 
ret = btrfs_create_subvol_root(trans, new_root, new_dirid);
+   if (ret)
+   goto fail;
/*
 * insert the directory item
 */
-- 
1.7.6

--
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 04/20] btrfs: make insert_ptr() void

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

insert_ptr() always returns zero, so all the exta error handling can go
away.  This makes it trivial to also make copy_for_split() a void function
as it's only return was from insert_ptr(). Finally, this all makes the
BUG_ON(ret) in split_leaf() meaningless so I removed that.

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/ctree.c |   59 -
 1 files changed, 18 insertions(+), 41 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 011cab3..41605ac 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2123,12 +2123,10 @@ static noinline int insert_new_root(struct 
btrfs_trans_handle *trans,
  *
  * slot and level indicate where you want the key to go, and
  * blocknr is the block the key points to.
- *
- * returns zero on success and  0 on any error
  */
-static int insert_ptr(struct btrfs_trans_handle *trans, struct btrfs_root
- *root, struct btrfs_path *path, struct btrfs_disk_key
- *key, u64 bytenr, int slot, int level)
+static void insert_ptr(struct btrfs_trans_handle *trans, struct btrfs_root
+  *root, struct btrfs_path *path, struct btrfs_disk_key
+  *key, u64 bytenr, int slot, int level)
 {
struct extent_buffer *lower;
int nritems;
@@ -2152,7 +2150,6 @@ static int insert_ptr(struct btrfs_trans_handle *trans, 
struct btrfs_root
btrfs_set_node_ptr_generation(lower, slot, trans-transid);
btrfs_set_header_nritems(lower, nritems + 1);
btrfs_mark_buffer_dirty(lower);
-   return 0;
 }
 
 /*
@@ -2173,7 +2170,6 @@ static noinline int split_node(struct btrfs_trans_handle 
*trans,
struct btrfs_disk_key disk_key;
int mid;
int ret;
-   int wret;
u32 c_nritems;
 
c = path-nodes[level];
@@ -2230,11 +2226,8 @@ static noinline int split_node(struct btrfs_trans_handle 
*trans,
btrfs_mark_buffer_dirty(c);
btrfs_mark_buffer_dirty(split);
 
-   wret = insert_ptr(trans, root, path, disk_key, split-start,
- path-slots[level + 1] + 1,
- level + 1);
-   if (wret)
-   ret = wret;
+   insert_ptr(trans, root, path, disk_key, split-start,
+  path-slots[level + 1] + 1, level + 1);
 
if (path-slots[level] = mid) {
path-slots[level] -= mid;
@@ -2724,18 +2717,16 @@ out:
  *
  * returns 0 if all went well and  0 on failure.
  */
-static noinline int copy_for_split(struct btrfs_trans_handle *trans,
-  struct btrfs_root *root,
-  struct btrfs_path *path,
-  struct extent_buffer *l,
-  struct extent_buffer *right,
-  int slot, int mid, int nritems)
+static noinline void copy_for_split(struct btrfs_trans_handle *trans,
+   struct btrfs_root *root,
+   struct btrfs_path *path,
+   struct extent_buffer *l,
+   struct extent_buffer *right,
+   int slot, int mid, int nritems)
 {
int data_copy_size;
int rt_data_off;
int i;
-   int ret = 0;
-   int wret;
struct btrfs_disk_key disk_key;
 
nritems = nritems - mid;
@@ -2763,12 +2754,9 @@ static noinline int copy_for_split(struct 
btrfs_trans_handle *trans,
}
 
btrfs_set_header_nritems(l, mid);
-   ret = 0;
btrfs_item_key(right, disk_key, 0);
-   wret = insert_ptr(trans, root, path, disk_key, right-start,
- path-slots[1] + 1, 1);
-   if (wret)
-   ret = wret;
+   insert_ptr(trans, root, path, disk_key, right-start,
+  path-slots[1] + 1, 1);
 
btrfs_mark_buffer_dirty(right);
btrfs_mark_buffer_dirty(l);
@@ -2786,8 +2774,6 @@ static noinline int copy_for_split(struct 
btrfs_trans_handle *trans,
}
 
BUG_ON(path-slots[0]  0);
-
-   return ret;
 }
 
 /*
@@ -2976,12 +2962,8 @@ again:
if (split == 0) {
if (mid = slot) {
btrfs_set_header_nritems(right, 0);
-   wret = insert_ptr(trans, root, path,
- disk_key, right-start,
- path-slots[1] + 1, 1);
-   if (wret)
-   ret = wret;
-
+   insert_ptr(trans, root, path, disk_key, right-start,
+  path-slots[1] + 1, 1);
btrfs_tree_unlock(path-nodes[0]);
free_extent_buffer(path-nodes[0]);
path-nodes[0] = right;
@@ -2989,12 +2971,8 @@ again:
path-slots[1] += 1;

[PATCH 02/20] btrfs: Don't BUG_ON() errors in update_ref_for_cow()

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

The only caller of update_ref_for_cow() is __btrfs_cow_block() which was
originally ignoring any return values. update_ref_for_cow() however doesn't
look like a candidate to become a void function - there are a few places
where errors can occur.

So instead I changed update_ref_for_cow() to bubble all errors up (instead
of BUG_ON). __btrfs_cow_block() was then updated to catch and BUG_ON() any
errors from update_ref_for_cow(). The end effect is that we have no change
in behavior, but about 8 different places where a BUG_ON(ret) was removed.

Obviously a future patch will have to address the BUG_ON() in
__btrfs_cow_block().

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/ctree.c |   31 +--
 1 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 011cab3..5064930 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -331,7 +331,8 @@ static noinline int update_ref_for_cow(struct 
btrfs_trans_handle *trans,
if (btrfs_block_can_be_shared(root, buf)) {
ret = btrfs_lookup_extent_info(trans, root, buf-start,
   buf-len, refs, flags);
-   BUG_ON(ret);
+   if (ret)
+   return ret;
BUG_ON(refs == 0);
} else {
refs = 1;
@@ -351,14 +352,18 @@ static noinline int update_ref_for_cow(struct 
btrfs_trans_handle *trans,
 root-root_key.objectid == BTRFS_TREE_RELOC_OBJECTID) 
!(flags  BTRFS_BLOCK_FLAG_FULL_BACKREF)) {
ret = btrfs_inc_ref(trans, root, buf, 1);
-   BUG_ON(ret);
+   if (ret)
+   return ret;
 
if (root-root_key.objectid ==
BTRFS_TREE_RELOC_OBJECTID) {
ret = btrfs_dec_ref(trans, root, buf, 0);
-   BUG_ON(ret);
+   if (ret)
+   return ret;
+
ret = btrfs_inc_ref(trans, root, cow, 1);
-   BUG_ON(ret);
+   if (ret)
+   return ret;
}
new_flags |= BTRFS_BLOCK_FLAG_FULL_BACKREF;
} else {
@@ -368,14 +373,16 @@ static noinline int update_ref_for_cow(struct 
btrfs_trans_handle *trans,
ret = btrfs_inc_ref(trans, root, cow, 1);
else
ret = btrfs_inc_ref(trans, root, cow, 0);
-   BUG_ON(ret);
+   if (ret)
+   return ret;
}
if (new_flags != 0) {
ret = btrfs_set_disk_extent_flags(trans, root,
  buf-start,
  buf-len,
  new_flags, 0);
-   BUG_ON(ret);
+   if (ret)
+   return ret;
}
} else {
if (flags  BTRFS_BLOCK_FLAG_FULL_BACKREF) {
@@ -384,9 +391,12 @@ static noinline int update_ref_for_cow(struct 
btrfs_trans_handle *trans,
ret = btrfs_inc_ref(trans, root, cow, 1);
else
ret = btrfs_inc_ref(trans, root, cow, 0);
-   BUG_ON(ret);
+   if (ret)
+   return ret;
+
ret = btrfs_dec_ref(trans, root, buf, 1);
-   BUG_ON(ret);
+   if (ret)
+   return ret;
}
clean_tree_block(trans, root, buf);
*last_ref = 1;
@@ -415,7 +425,7 @@ static noinline int __btrfs_cow_block(struct 
btrfs_trans_handle *trans,
 {
struct btrfs_disk_key disk_key;
struct extent_buffer *cow;
-   int level;
+   int level, ret;
int last_ref = 0;
int unlock_orig = 0;
u64 parent_start;
@@ -467,7 +477,8 @@ static noinline int __btrfs_cow_block(struct 
btrfs_trans_handle *trans,
(unsigned long)btrfs_header_fsid(cow),
BTRFS_FSID_SIZE);
 
-   update_ref_for_cow(trans, root, buf, cow, last_ref);
+   ret = update_ref_for_cow(trans, root, buf, cow, last_ref);
+   BUG_ON(ret);
 
if (root-ref_cows)
btrfs_reloc_cow_block(trans, root, buf, cow);
-- 
1.7.6

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

[PATCH 06/20] btrfs: fix error check of btrfs_lookup_dentry()

2011-09-15 Thread Mark Fasheh 
From: Tsutomu Itoh t-i...@jp.fujitsu.com

Clean up btrfs_lookup_dentry() to never return NULL, but PTR_ERR(-ENOENT)
instead. This keeps the return value convention consistent.

Callers who pass to d_instatiate() require a trivial update.

create_snapshot() in particular looks like it can also lose a BUG_ON(!inode)
which is not really needed - there seems less harm in returning ENOENT to
userspace at that point in the stack than there is to crash the machine.

Mark: Fixed conflicts against latest tree, gave the patch a more thorough
description.

Signed-off-by: Tsutomu Itoh t-i...@jp.fujitsu.com
Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/inode.c |   12 ++--
 fs/btrfs/ioctl.c |   11 +--
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 15fceef..5fdb700 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4027,7 +4027,7 @@ struct inode *btrfs_lookup_dentry(struct inode *dir, 
struct dentry *dentry)
return ERR_PTR(ret);
 
if (location.objectid == 0)
-   return NULL;
+   return ERR_PTR(-ENOENT);
 
if (location.type == BTRFS_INODE_ITEM_KEY) {
inode = btrfs_iget(dir-i_sb, location, root, NULL);
@@ -4085,7 +4085,15 @@ static void btrfs_dentry_release(struct dentry *dentry)
 static struct dentry *btrfs_lookup(struct inode *dir, struct dentry *dentry,
   struct nameidata *nd)
 {
-   return d_splice_alias(btrfs_lookup_dentry(dir, dentry), dentry);
+   struct inode *inode = btrfs_lookup_dentry(dir, dentry);
+   if (IS_ERR(inode)) {
+   if (PTR_ERR(inode) == -ENOENT)
+   inode = NULL;
+   else
+   return ERR_CAST(inode);
+   }
+
+   return d_splice_alias(inode, dentry);
 }
 
 unsigned char btrfs_filetype_table[] = {
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 7cf0133..9245d24 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -325,6 +325,7 @@ static noinline int create_subvol(struct btrfs_root *root,
struct btrfs_root *new_root;
struct dentry *parent = dentry-d_parent;
struct inode *dir;
+   struct inode *inode;
int ret;
int err;
u64 objectid;
@@ -433,7 +434,13 @@ static noinline int create_subvol(struct btrfs_root *root,
 
BUG_ON(ret);
 
-   d_instantiate(dentry, btrfs_lookup_dentry(dir, dentry));
+   inode = btrfs_lookup_dentry(dir, dentry);
+   if (IS_ERR(inode)) {
+   ret = PTR_ERR(inode);
+   goto fail;
+   }
+
+   d_instantiate(dentry, inode);
 fail:
if (async_transid) {
*async_transid = trans-transid;
@@ -503,7 +510,7 @@ static int create_snapshot(struct btrfs_root *root, struct 
dentry *dentry,
ret = PTR_ERR(inode);
goto fail;
}
-   BUG_ON(!inode);
+
d_instantiate(dentry, inode);
ret = 0;
 fail:
-- 
1.7.6

--
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 08/20] btrfs: make del_ptr() and btrfs_del_leaf() void

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

Since fixup_low_keys() has been made void, del_ptr() always returns zero. We
can then make it void as well. This allows us in turn to make
btrfs_del_leaf() void as the only return value it was previously catching
was from del_ptr(). This winds up removing a couple of un-needed BUG_ON(ret)
lines.

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/ctree.c |   40 +---
 1 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index b4a0801..2b2f8fd 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -36,8 +36,8 @@ static int balance_node_right(struct btrfs_trans_handle 
*trans,
  struct btrfs_root *root,
  struct extent_buffer *dst_buf,
  struct extent_buffer *src_buf);
-static int del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root,
-  struct btrfs_path *path, int level, int slot);
+static void del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root,
+   struct btrfs_path *path, int level, int slot);
 
 struct btrfs_path *btrfs_alloc_path(void)
 {
@@ -994,10 +994,7 @@ static noinline int balance_level(struct 
btrfs_trans_handle *trans,
if (btrfs_header_nritems(right) == 0) {
clean_tree_block(trans, root, right);
btrfs_tree_unlock(right);
-   wret = del_ptr(trans, root, path, level + 1, pslot +
-  1);
-   if (wret)
-   ret = wret;
+   del_ptr(trans, root, path, level + 1, pslot + 1);
root_sub_used(root, right-len);
btrfs_free_tree_block(trans, root, right, 0, 1);
free_extent_buffer(right);
@@ -1035,9 +1032,7 @@ static noinline int balance_level(struct 
btrfs_trans_handle *trans,
if (btrfs_header_nritems(mid) == 0) {
clean_tree_block(trans, root, mid);
btrfs_tree_unlock(mid);
-   wret = del_ptr(trans, root, path, level + 1, pslot);
-   if (wret)
-   ret = wret;
+   del_ptr(trans, root, path, level + 1, pslot);
root_sub_used(root, mid-len);
btrfs_free_tree_block(trans, root, mid, 0, 1);
free_extent_buffer(mid);
@@ -3685,12 +3680,11 @@ int btrfs_insert_item(struct btrfs_trans_handle *trans, 
struct btrfs_root
  * the tree should have been previously balanced so the deletion does not
  * empty a node.
  */
-static int del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root,
-  struct btrfs_path *path, int level, int slot)
+static void del_ptr(struct btrfs_trans_handle *trans, struct btrfs_root *root,
+   struct btrfs_path *path, int level, int slot)
 {
struct extent_buffer *parent = path-nodes[level];
u32 nritems;
-   int ret = 0;
 
nritems = btrfs_header_nritems(parent);
if (slot != nritems - 1) {
@@ -3713,7 +3707,6 @@ static int del_ptr(struct btrfs_trans_handle *trans, 
struct btrfs_root *root,
fixup_low_keys(trans, root, path, disk_key, level + 1);
}
btrfs_mark_buffer_dirty(parent);
-   return ret;
 }
 
 /*
@@ -3726,17 +3719,13 @@ static int del_ptr(struct btrfs_trans_handle *trans, 
struct btrfs_root *root,
  * The path must have already been setup for deleting the leaf, including
  * all the proper balancing.  path-nodes[1] must be locked.
  */
-static noinline int btrfs_del_leaf(struct btrfs_trans_handle *trans,
-  struct btrfs_root *root,
-  struct btrfs_path *path,
-  struct extent_buffer *leaf)
+static noinline void btrfs_del_leaf(struct btrfs_trans_handle *trans,
+   struct btrfs_root *root,
+   struct btrfs_path *path,
+   struct extent_buffer *leaf)
 {
-   int ret;
-
WARN_ON(btrfs_header_generation(leaf) != trans-transid);
-   ret = del_ptr(trans, root, path, 1, path-slots[1]);
-   if (ret)
-   return ret;
+   del_ptr(trans, root, path, 1, path-slots[1]);
 
/*
 * btrfs_free_extent is expensive, we want to make sure we
@@ -3747,7 +3736,6 @@ static noinline int btrfs_del_leaf(struct 
btrfs_trans_handle *trans,
root_sub_used(root, leaf-len);
 
btrfs_free_tree_block(trans, root, leaf, 0, 1);
-   return 0;
 }
 /*
  * delete the item at the leaf level in path.  If that empties
@@ -3804,8 +3792,7 @@ int btrfs_del_items(struct btrfs_trans_handle *trans, 
struct btrfs_root *root,
} else {
btrfs_set_path_blocking(path);

[PATCH 10/20] btrfs: go readonly on insert error in btrfs_add_root_ref()

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

In btrfs_add_root_ref() we BUG if an error is encountered during
REF/BACKREF insertion. This does not look like a logic error, thus the BUG
is not called for. However, I don't think there's a simple way to recover
from such an error at that point, so we mark the fs readonly instead.

At the same time, we can update the following caller of
btrfs_add_root_ref() which BUG_ON from an error:

- create_subvol: now passes the return code back to userspace

- create_pending_snapshot: goes readonly on any error from
  btrfs_add_root_ref().

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/ioctl.c   |4 ++--
 fs/btrfs/root-tree.c   |8 ++--
 fs/btrfs/transaction.c |2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 7cf0133..8adb220 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -430,8 +430,8 @@ static noinline int create_subvol(struct btrfs_root *root,
ret = btrfs_add_root_ref(trans, root-fs_info-tree_root,
 objectid, root-root_key.objectid,
 btrfs_ino(dir), index, name, namelen);
-
-   BUG_ON(ret);
+   if (ret)
+   goto fail;
 
d_instantiate(dentry, btrfs_lookup_dentry(dir, dentry));
 fail:
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index f409990..02f2bf3 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -407,7 +407,10 @@ int btrfs_add_root_ref(struct btrfs_trans_handle *trans,
 again:
ret = btrfs_insert_empty_item(trans, tree_root, path, key,
  sizeof(*ref) + name_len);
-   BUG_ON(ret);
+   if (ret) {
+   btrfs_std_error(tree_root-fs_info, ret);
+   goto out_free;
+   }
 
leaf = path-nodes[0];
ref = btrfs_item_ptr(leaf, path-slots[0], struct btrfs_root_ref);
@@ -426,8 +429,9 @@ again:
goto again;
}
 
+out_free:
btrfs_free_path(path);
-   return 0;
+   return ret;
 }
 
 /*
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 7dc36fa..7c46ece 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -991,7 +991,7 @@ static noinline int create_pending_snapshot(struct 
btrfs_trans_handle *trans,
 parent_root-root_key.objectid,
 btrfs_ino(parent_inode), index,
 dentry-d_name.name, dentry-d_name.len);
-   BUG_ON(ret);
+   btrfs_std_error(fs_info, ret);
dput(parent);
 
key.offset = (u64)-1;
-- 
1.7.6

--
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 14/20] btrfs: Document BUG() in find_lock_delalloc_range()

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

We BUG_ON a nonzero, non -EAGAIN ret from lock_delalloc_range(). As it turns
out there is no other possible return value that makes sense anyway.  The
bare BUG_ON(ret) was a bit confusing and looked like something that needed
fixing. This patch documents the BUG_ON() so we know why it's there.

Signed-off-by: Mark Fasheh mfas...@suse.com
---
 fs/btrfs/extent_io.c |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index d418164..2eb366d 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1303,7 +1303,15 @@ again:
goto out_failed;
}
}
-   BUG_ON(ret);
+   if (ret) {
+   /*
+* This should never happen - lock_delalloc_pages only returns
+* 0 or -EAGAIN which are handled above.
+*/
+   printk(KERN_ERR btrfs: unexpected return %d from 
+  lock_delalloc_pages\n, ret);
+   BUG();
+   }
 
/* step three, lock the state bits for the whole range */
lock_extent_bits(tree, delalloc_start, delalloc_end,
-- 
1.7.6

--
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 15/20] btrfs: Go readonly on missing ref in btrfs_get_parent()

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

btrfs_get_parent() searches the btree for a ref to the current object. From
there it can compute the parent objectid from which it can return a dentry.
if the reference is not found in the tree however, we BUG(). I believe a
more appropriate response would be to go read-only as this seems like a
corruption.

Signed-off-by: Mark Fasheh mfas...@suse.com
---
 fs/btrfs/export.c |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c
index 1b8dc33..a335169 100644
--- a/fs/btrfs/export.c
+++ b/fs/btrfs/export.c
@@ -193,7 +193,13 @@ static struct dentry *btrfs_get_parent(struct dentry 
*child)
if (ret  0)
goto fail;
 
-   BUG_ON(ret == 0);
+   if (ret == 0) {
+   /* Missing ref, should go readonly. */
+   ret = -ENOENT;
+   btrfs_std_error(root-fs_info, ret);
+   goto fail;
+   }
+
if (path-slots[0] == 0) {
ret = -ENOENT;
goto fail;
-- 
1.7.6

--
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 18/20] btrfs: Don't BUG_ON insert errors in btrfs_alloc_dev_extent()

2011-09-15 Thread Mark Fasheh 
The only caller of btrfs_alloc_dev_extent() is __btrfs_alloc_chunk() which
already bugs on any error returned. We can remove the BUG_ON's in
btrfs_alloc_dev_extent() then since __btrfs_alloc_chunk() will catch them
anyway.

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/volumes.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 53875ae..1f5c3b1 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1008,7 +1008,8 @@ int btrfs_alloc_dev_extent(struct btrfs_trans_handle 
*trans,
key.type = BTRFS_DEV_EXTENT_KEY;
ret = btrfs_insert_empty_item(trans, root, path, key,
  sizeof(*extent));
-   BUG_ON(ret);
+   if (ret)
+   goto out;
 
leaf = path-nodes[0];
extent = btrfs_item_ptr(leaf, path-slots[0],
@@ -1023,6 +1024,7 @@ int btrfs_alloc_dev_extent(struct btrfs_trans_handle 
*trans,
 
btrfs_set_dev_extent_length(leaf, extent, num_bytes);
btrfs_mark_buffer_dirty(leaf);
+out:
btrfs_free_path(path);
return ret;
 }
-- 
1.7.6

--
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 20/20] btrfs: Remove BUG_ON from __finish_chunk_alloc()

2011-09-15 Thread Mark Fasheh 
btrfs_alloc_chunk() unconditionally BUGs on any error returned from
__finish_chunk_alloc() so there's no need for two BUG_ON lines. Remove the
one from __finish_chunk_alloc().

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/volumes.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 50547f3..804328c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2571,7 +2571,8 @@ static int __finish_chunk_alloc(struct btrfs_trans_handle 
*trans,
device = map-stripes[index].dev;
device-bytes_used += stripe_size;
ret = btrfs_update_device(trans, device);
-   BUG_ON(ret);
+   if (ret)
+   goto out_free;
index++;
}
 
@@ -2611,6 +2612,7 @@ static int __finish_chunk_alloc(struct btrfs_trans_handle 
*trans,
BUG_ON(ret);
}
 
+out_free:
kfree(chunk);
return 0;
 }
-- 
1.7.6

--
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 16/20] btrfs: make add_delayed_ref_head() void

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

This is trivial as the function always returns success. We can remove 3
BUG_ON(ret) lines as a result.

Signed-off-by: Mark Fasheh mfas...@suse.com
---
 fs/btrfs/delayed-ref.c |   26 ++
 1 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index 125cf76..b5c3d7c 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -390,10 +390,10 @@ update_existing_head_ref(struct btrfs_delayed_ref_node 
*existing,
  * this does all the dirty work in terms of maintaining the correct
  * overall modification count.
  */
-static noinline int add_delayed_ref_head(struct btrfs_trans_handle *trans,
-   struct btrfs_delayed_ref_node *ref,
-   u64 bytenr, u64 num_bytes,
-   int action, int is_data)
+static noinline void add_delayed_ref_head(struct btrfs_trans_handle *trans,
+ struct btrfs_delayed_ref_node *ref,
+ u64 bytenr, u64 num_bytes,
+ int action, int is_data)
 {
struct btrfs_delayed_ref_node *existing;
struct btrfs_delayed_ref_head *head_ref = NULL;
@@ -462,7 +462,6 @@ static noinline int add_delayed_ref_head(struct 
btrfs_trans_handle *trans,
delayed_refs-num_entries++;
trans-delayed_ref_updates++;
}
-   return 0;
 }
 
 /*
@@ -610,9 +609,8 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle 
*trans,
 * insert both the head node and the new ref without dropping
 * the spin lock
 */
-   ret = add_delayed_ref_head(trans, head_ref-node, bytenr, num_bytes,
-  action, 0);
-   BUG_ON(ret);
+   add_delayed_ref_head(trans, head_ref-node, bytenr, num_bytes, action,
+0);
 
ret = add_delayed_tree_ref(trans, ref-node, bytenr, num_bytes,
   parent, ref_root, level, action);
@@ -655,9 +653,8 @@ int btrfs_add_delayed_data_ref(struct btrfs_trans_handle 
*trans,
 * insert both the head node and the new ref without dropping
 * the spin lock
 */
-   ret = add_delayed_ref_head(trans, head_ref-node, bytenr, num_bytes,
-  action, 1);
-   BUG_ON(ret);
+   add_delayed_ref_head(trans, head_ref-node, bytenr, num_bytes,
+action, 1);
 
ret = add_delayed_data_ref(trans, ref-node, bytenr, num_bytes,
   parent, ref_root, owner, offset, action);
@@ -672,7 +669,6 @@ int btrfs_add_delayed_extent_op(struct btrfs_trans_handle 
*trans,
 {
struct btrfs_delayed_ref_head *head_ref;
struct btrfs_delayed_ref_root *delayed_refs;
-   int ret;
 
head_ref = kmalloc(sizeof(*head_ref), GFP_NOFS);
if (!head_ref)
@@ -683,10 +679,8 @@ int btrfs_add_delayed_extent_op(struct btrfs_trans_handle 
*trans,
delayed_refs = trans-transaction-delayed_refs;
spin_lock(delayed_refs-lock);
 
-   ret = add_delayed_ref_head(trans, head_ref-node, bytenr,
-  num_bytes, BTRFS_UPDATE_DELAYED_HEAD,
-  extent_op-is_data);
-   BUG_ON(ret);
+   add_delayed_ref_head(trans, head_ref-node, bytenr, num_bytes,
+BTRFS_UPDATE_DELAYED_HEAD, extent_op-is_data);
 
spin_unlock(delayed_refs-lock);
return 0;
-- 
1.7.6

--
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 19/20] btrfs: Remove BUG_ON from __btrfs_alloc_chunk()

2011-09-15 Thread Mark Fasheh 
We BUG_ON() error from add_extent_mapping(), but that error looks pretty
easy to bubble back up - as far as I can tell there have not been any
permanent modifications to fs state at that point.

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/volumes.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 1f5c3b1..50547f3 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2515,8 +2515,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle 
*trans,
write_lock(em_tree-lock);
ret = add_extent_mapping(em_tree, em);
write_unlock(em_tree-lock);
-   BUG_ON(ret);
free_extent_map(em);
+   if (ret)
+   goto error;
 
ret = btrfs_make_block_group(trans, extent_root, 0, type,
 BTRFS_FIRST_CHUNK_TREE_OBJECTID,
-- 
1.7.6

--
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 17/20] btrfs: make add_delayed_tree_ref() void

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

add_delayed_tree_ref() unconditionally returns 0. This makes it trivial to
turn into a void function. This kills another BUG_ON().

Signed-off-by: Mark Fasheh mfas...@suse.com
---
 fs/btrfs/delayed-ref.c |   16 +++-
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index b5c3d7c..afe1b1e 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -467,10 +467,10 @@ static noinline void add_delayed_ref_head(struct 
btrfs_trans_handle *trans,
 /*
  * helper to insert a delayed tree ref into the rbtree.
  */
-static noinline int add_delayed_tree_ref(struct btrfs_trans_handle *trans,
-struct btrfs_delayed_ref_node *ref,
-u64 bytenr, u64 num_bytes, u64 parent,
-u64 ref_root, int level, int action)
+static void add_delayed_tree_ref(struct btrfs_trans_handle *trans,
+struct btrfs_delayed_ref_node *ref,
+u64 bytenr, u64 num_bytes, u64 parent,
+u64 ref_root, int level, int action)
 {
struct btrfs_delayed_ref_node *existing;
struct btrfs_delayed_tree_ref *full_ref;
@@ -515,7 +515,6 @@ static noinline int add_delayed_tree_ref(struct 
btrfs_trans_handle *trans,
delayed_refs-num_entries++;
trans-delayed_ref_updates++;
}
-   return 0;
 }
 
 /*
@@ -587,7 +586,6 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle 
*trans,
struct btrfs_delayed_tree_ref *ref;
struct btrfs_delayed_ref_head *head_ref;
struct btrfs_delayed_ref_root *delayed_refs;
-   int ret;
 
BUG_ON(extent_op  extent_op-is_data);
ref = kmalloc(sizeof(*ref), GFP_NOFS);
@@ -612,9 +610,9 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle 
*trans,
add_delayed_ref_head(trans, head_ref-node, bytenr, num_bytes, action,
 0);
 
-   ret = add_delayed_tree_ref(trans, ref-node, bytenr, num_bytes,
-  parent, ref_root, level, action);
-   BUG_ON(ret);
+   add_delayed_tree_ref(trans, ref-node, bytenr, num_bytes, parent,
+ref_root, level, action);
+
spin_unlock(delayed_refs-lock);
return 0;
 }
-- 
1.7.6

--
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 11/20] btrfs: Go readonly on bad extent refs in update_ref_for_cow()

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

update_ref_for_cow() will BUG_ON() after it's call to
btrfs_lookup_extent_info() if no existing references are found.  Since refs
are computed directly from disk, this should be treated as a corruption
instead of a logic error.

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/ctree.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 5064930..8d8182f 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -333,7 +333,11 @@ static noinline int update_ref_for_cow(struct 
btrfs_trans_handle *trans,
   buf-len, refs, flags);
if (ret)
return ret;
-   BUG_ON(refs == 0);
+   if (refs == 0) {
+   ret = -EROFS;
+   btrfs_std_error(root-fs_info, ret);
+   return ret;
+   }
} else {
refs = 1;
if (root-root_key.objectid == BTRFS_TREE_RELOC_OBJECTID ||
-- 
1.7.6

--
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 13/20] btrfs: Go readonly on tree errors in balance_level

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

balace_level() seems to deal with missing tree nodes by BUG_ON(). Instead,
we can easily just set the file system readonly and bubble -EROFS back up
the stack.

Signed-off-by: Mark Fasheh mfas...@suse.com
---
 fs/btrfs/ctree.c |   13 +++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 011cab3..dfb061b 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -918,7 +918,12 @@ static noinline int balance_level(struct 
btrfs_trans_handle *trans,
 
/* promote the child to a root */
child = read_node_slot(root, mid, 0);
-   BUG_ON(!child);
+   if (!child) {
+   ret = -EROFS;
+   btrfs_std_error(root-fs_info, ret);
+   goto enospc;
+   }
+
btrfs_tree_lock(child);
btrfs_set_lock_blocking(child);
ret = btrfs_cow_block(trans, root, child, mid, 0, child);
@@ -1019,7 +1024,11 @@ static noinline int balance_level(struct 
btrfs_trans_handle *trans,
 * otherwise we would have pulled some pointers from the
 * right
 */
-   BUG_ON(!left);
+   if (!left) {
+   ret = -EROFS;
+   btrfs_std_error(root-fs_info, ret);
+   goto enospc;
+   }
wret = balance_node_right(trans, root, mid, left);
if (wret  0) {
ret = wret;
-- 
1.7.6

--
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 12/20] btrfs: Don't BUG_ON errors from update_ref_for_cow()

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

__btrfs_cow_block(), the only caller of update_ref_for_cow() will BUG_ON()
any error return.  Instead, we can go read-only fs as update_ref_for_cow()
manipulates disk data in a way which doesn't look like it's easily rolled
back.

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/ctree.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 8d8182f..31fdadf 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -482,7 +482,10 @@ static noinline int __btrfs_cow_block(struct 
btrfs_trans_handle *trans,
BTRFS_FSID_SIZE);
 
ret = update_ref_for_cow(trans, root, buf, cow, last_ref);
-   BUG_ON(ret);
+   if (ret) {
+   btrfs_std_error(root-fs_info, ret);
+   return ret;
+   }
 
if (root-ref_cows)
btrfs_reloc_cow_block(trans, root, buf, cow);
-- 
1.7.6

--
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 07/20] btrfs: make fixup_low_keys() void

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

This is trivial - fixup_low_keys always returns zero so we can make it void.
As a result, we can then make setup_items_for_insert() void too which lets
us cut out a couple of BUG_ON(ret) lines.

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/ctree.c |   59 ++---
 fs/btrfs/ctree.h |8 +++---
 fs/btrfs/delayed-inode.c |6 +---
 3 files changed, 25 insertions(+), 48 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 011cab3..b4a0801 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1863,16 +1863,12 @@ done:
  * This is used after shifting pointers to the left, so it stops
  * fixing up pointers when a given leaf/node is not in slot 0 of the
  * higher levels
- *
- * If this fails to write a tree block, it returns -1, but continues
- * fixing up the blocks in ram so the tree is consistent.
  */
-static int fixup_low_keys(struct btrfs_trans_handle *trans,
- struct btrfs_root *root, struct btrfs_path *path,
- struct btrfs_disk_key *key, int level)
+static void fixup_low_keys(struct btrfs_trans_handle *trans,
+  struct btrfs_root *root, struct btrfs_path *path,
+  struct btrfs_disk_key *key, int level)
 {
int i;
-   int ret = 0;
struct extent_buffer *t;
 
for (i = level; i  BTRFS_MAX_LEVEL; i++) {
@@ -1885,7 +1881,6 @@ static int fixup_low_keys(struct btrfs_trans_handle 
*trans,
if (tslot != 0)
break;
}
-   return ret;
 }
 
 /*
@@ -2520,7 +2515,6 @@ static noinline int __push_leaf_left(struct 
btrfs_trans_handle *trans,
u32 old_left_nritems;
u32 nr;
int ret = 0;
-   int wret;
u32 this_item_size;
u32 old_left_item_size;
 
@@ -2626,9 +2620,7 @@ static noinline int __push_leaf_left(struct 
btrfs_trans_handle *trans,
clean_tree_block(trans, root, right);
 
btrfs_item_key(right, disk_key, 0);
-   wret = fixup_low_keys(trans, root, path, disk_key, 1);
-   if (wret)
-   ret = wret;
+   fixup_low_keys(trans, root, path, disk_key, 1);
 
/* then fixup the leaf pointer in the path */
if (path-slots[0]  push_items) {
@@ -2999,12 +2991,8 @@ again:
free_extent_buffer(path-nodes[0]);
path-nodes[0] = right;
path-slots[0] = 0;
-   if (path-slots[1] == 0) {
-   wret = fixup_low_keys(trans, root,
-   path, disk_key, 1);
-   if (wret)
-   ret = wret;
-   }
+   if (path-slots[1] == 0)
+   fixup_low_keys(trans, root, path, disk_key, 1);
}
btrfs_mark_buffer_dirty(right);
return ret;
@@ -3221,10 +3209,9 @@ int btrfs_duplicate_item(struct btrfs_trans_handle 
*trans,
return ret;
 
path-slots[0]++;
-   ret = setup_items_for_insert(trans, root, path, new_key, item_size,
-item_size, item_size +
-sizeof(struct btrfs_item), 1);
-   BUG_ON(ret);
+   setup_items_for_insert(trans, root, path, new_key, item_size,
+  item_size, item_size +
+  sizeof(struct btrfs_item), 1);
 
leaf = path-nodes[0];
memcpy_extent_buffer(leaf,
@@ -3527,7 +3514,7 @@ int btrfs_insert_some_items(struct btrfs_trans_handle 
*trans,
ret = 0;
if (slot == 0) {
btrfs_cpu_key_to_disk(disk_key, cpu_key);
-   ret = fixup_low_keys(trans, root, path, disk_key, 1);
+   fixup_low_keys(trans, root, path, disk_key, 1);
}
 
if (btrfs_leaf_free_space(root, leaf)  0) {
@@ -3545,17 +3532,16 @@ out:
  * to save stack depth by doing the bulk of the work in a function
  * that doesn't call btrfs_search_slot
  */
-int setup_items_for_insert(struct btrfs_trans_handle *trans,
-  struct btrfs_root *root, struct btrfs_path *path,
-  struct btrfs_key *cpu_key, u32 *data_size,
-  u32 total_data, u32 total_size, int nr)
+void setup_items_for_insert(struct btrfs_trans_handle *trans,
+   struct btrfs_root *root, struct btrfs_path *path,
+   struct btrfs_key *cpu_key, u32 *data_size,
+   u32 total_data, u32 total_size, int nr)
 {
struct btrfs_item *item;
int i;
u32 nritems;
unsigned int data_end;
struct btrfs_disk_key disk_key;
-   int ret;
struct extent_buffer *leaf;
int slot;
 
@@ -3616,10 +3602,9 @@ int

[PATCH 09/20] btrfs: Don't BUG_ON failures in find_and_setup_root()

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

This is very easy - we can just pass an error from btrfs_find_last_root()
back out to the callers - all of them have proper error handling.

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/disk-io.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 07b3ac6..e9c7afb 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1131,7 +1131,8 @@ static int find_and_setup_root(struct btrfs_root 
*tree_root,
   root-root_item, root-root_key);
if (ret  0)
return -ENOENT;
-   BUG_ON(ret);
+   if (ret  0)
+   return ret;
 
generation = btrfs_root_generation(root-root_item);
blocksize = btrfs_level_size(root, btrfs_root_level(root-root_item));
-- 
1.7.6

--
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 05/20] btrfs: Don't BUG_ON errors in __finish_chunk_alloc()

2011-09-15 Thread Mark Fasheh 
From: Mark Fasheh mfas...@suse.com

All callers of __finish_chunk_alloc() BUG_ON() return value, so it's trivial
for us to always bubble up any errors caught in __finish_chunk_alloc() to be
caught there.

Signed-off-by: Mark Fasheh mfas...@suse.de
---
 fs/btrfs/volumes.c |7 ++-
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 53875ae..5d166c2 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2600,16 +2600,13 @@ static int __finish_chunk_alloc(struct 
btrfs_trans_handle *trans,
key.offset = chunk_offset;
 
ret = btrfs_insert_item(trans, chunk_root, key, chunk, item_size);
-   BUG_ON(ret);
-
-   if (map-type  BTRFS_BLOCK_GROUP_SYSTEM) {
+   if (ret == 0  map-type  BTRFS_BLOCK_GROUP_SYSTEM) {
ret = btrfs_add_system_chunk(trans, chunk_root, key, chunk,
 item_size);
-   BUG_ON(ret);
}
 
kfree(chunk);
-   return 0;
+   return ret;
 }
 
 /*
-- 
1.7.6

--
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 0/20] btrfs: More error handling fixes

2011-09-15 Thread David Sterba 
Hi,

On Thu, Sep 15, 2011 at 10:34:39AM -0700, Mark Fasheh wrote:
 Some of the patches (the first 8) were previously sent to this list but got
 no comments so I'm resending them with this set. Changes from last time are
 that I also started setting the file system readonly on errors which look
 like possible corruption.

I've been testing your branch 'all_fixes' from your git.k.org repo for
some time, among other patchsets and fixes in my development repository
at http://repo.or.cz/w/linux-2.6/btrfs-unstable.git #integration/btrfs-next
and I have not hit any single problem which would point back to your patches.
I'm testing with xfstests and various hand-made stress tests for specific
areas (usually from reports on irc).


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

Re: [PATCH V2] btrfs: trivial fix, a potential memory leak in btrfs_parse_early_options()

2011-09-15 Thread David Sterba 
On Thu, Sep 15, 2011 at 11:01:28PM +0800, Jeff Liu wrote:
 Signed-off-by: Jie Liu jeff@oracle.com
Reviewed-by: David Sterba dste...@suse.cz

 
 ---
  fs/btrfs/super.c |   10 --
  1 files changed, 8 insertions(+), 2 deletions(-)
 
 diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
 index 15634d4..16f31e1 100644
 --- a/fs/btrfs/super.c
 +++ b/fs/btrfs/super.c
 @@ -406,7 +406,7 @@ static int btrfs_parse_early_options(const char *options, 
 fmode_t flags,
   u64 *subvol_rootid, struct btrfs_fs_devices **fs_devices)
  {
   substring_t args[MAX_OPT_ARGS];
 - char *opts, *orig, *p;
 + char *device_name, *opts, *orig, *p;
   int error = 0;
   int intarg;
  
 @@ -457,8 +457,14 @@ static int btrfs_parse_early_options(const char 
 *options, fmode_t flags,
   }
   break;
   case Opt_device:
 - error = btrfs_scan_one_device(match_strdup(args[0]),
 + device_name = match_strdup(args[0]);
 + if (!device_name) {
 + error = -ENOMEM;
 + goto out_free_opts;
 + }
 + error = btrfs_scan_one_device(device_name,
   flags, holder, fs_devices);
 + kfree(device_name);
   if (error)
   goto out_free_opts;
   break;
 -- 
 1.7.4.1
 --
 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
--
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: WARNING: at fs/btrfs/inode.c:2193 btrfs_orphan_commit_root+0xb0/0xc0 [btrfs]()

2011-09-15 Thread Sage Weil 
On Tue, 13 Sep 2011, Liu Bo wrote:
 On 09/11/2011 05:47 AM, Martin Mailand wrote:
  Hi
  I am hitting this Warning reproducible, the workload is a ceph osd,
  kernel ist 3.1.0-rc5.
  
 
 Have posted a patch for this:
 
 http://marc.info/?l=linux-btrfsm=131547325515336w=2

We're still seeing this with -rc6, which includes 98c9942 and 65450aa.

I haven't looked at the reservation code in much detail.  Is there 
anything I can do to help track this down?

Thanks-
sage


 
 thanks,
 liubo
 
  Best Regards,
   martin
  
  [ 5472.099766] [ cut here ]
  [ 5472.099833] WARNING: at fs/btrfs/inode.c:2193
  btrfs_orphan_commit_root+0xb0/0xc0 [btrfs]()
  [ 5472.099838] Hardware name: MS-96B3
  [ 5472.099842] Modules linked in: radeon ttm drm_kms_helper drm
  i2c_algo_bit psmouse sp5100_tco edac_core lp shpchp serio_raw k8temp
  i2c_piix4 edac_mce_amd parport ahci pata_atiixp e1000e libahci btrfs
  zlib_deflate libcrc32c
  [ 5472.099878] Pid: 2066, comm: kworker/1:1 Not tainted 3.1.0-rc5-custom #1
  [ 5472.099882] Call Trace:
  [ 5472.099898]  [81063c1f] warn_slowpath_common+0x7f/0xc0
  [ 5472.099907]  [81063c7a] warn_slowpath_null+0x1a/0x20
  [ 5472.099935]  [a003f420] btrfs_orphan_commit_root+0xb0/0xc0
  [btrfs]
  [ 5472.099961]  [a00380aa] commit_fs_roots.clone.21+0xba/0x1a0
  [btrfs]
  [ 5472.099971]  [815db96e] ? _raw_spin_lock+0xe/0x20
  [ 5472.07]  [a003966f]
  btrfs_commit_transaction+0x3ef/0x870 [btrfs]
  [ 5472.100065]  [81012871] ? __switch_to+0x261/0x2f0
  [ 5472.100084]  [81086bf0] ? wake_up_bit+0x40/0x40
  [ 5472.100120]  [a0039af0] ?
  btrfs_commit_transaction+0x870/0x870 [btrfs]
  [ 5472.100155]  [a0039b0f] do_async_commit+0x1f/0x30 [btrfs]
  [ 5472.100171]  [8108110d] process_one_work+0x11d/0x430
  [ 5472.100187]  [81081c69] worker_thread+0x169/0x360
  [ 5472.100203]  [81081b00] ? manage_workers.clone.21+0x240/0x240
  [ 5472.100220]  [81086496] kthread+0x96/0xa0
  [ 5472.100236]  [815e5bb4] kernel_thread_helper+0x4/0x10
  [ 5472.100253]  [81086400] ? flush_kthread_worker+0xb0/0xb0
  [ 5472.100269]  [815e5bb0] ? gs_change+0x13/0x13
  [ 5472.100279] ---[ end trace a8bae5767c2c3e55 ]---
  -- 
  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
  
 
 --
 To unsubscribe from this list: send the line unsubscribe ceph-devel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 
 
--
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: Rename BTRfs to MuchSlowerFS ?

2011-09-15 Thread Felix Blanke 
I'm using btrfs since one year now and it's quite fast. I don't feel any 
differences to other filesystems. Never tried a benchmark but for my 
daily work it's nice. I also never had any issues with the memory. Imho 
nowadays memory isn't a problem at all in desktop computers. I bought 
8gb of memory 2 years before because it was so damn cheap. Never used 
that mutch, but it was almost for free :)


The advantage to ext4 for me is the build in raid1 and the snapshots. 
I'm using the snapshot feature for my local backups. I like it because 
it's really easy and uses very few storage. A simple Snapshot - Rsync 
to a different disk - Snapshot script is the perfect local backup method.


I really appreciate the work of the developers! Btrfs is great and I'm 
110% sure it will become better and better over the next month.


Best regards,
Felix

On 9/7/11 4:15 PM, Swâmi Petaramesh wrote:

Le Mercredi 7 Septembre 2011 00:11:25 vous avez écrit :

Reading your post, at this point I'd actually recommend you stick with
ext4.


I actually shifted back from BTRFS to ext4 and fell like having offered myself
a brand new computer,  about 20 times faster, me happy ;-)


Both btrfs and zfs are great, but IMHO btrfs is not ready for
daily use by ordinary user yet, while zfs is a memory hog
(especially for laptops, which is part of the reason why I'm using
btrfs instead of zfs on this one).


True, ZFS is excellent but a memory hog (and strongly advises using a 64-bit
OS) but I was surprised to discover that BTRFS was such a memory eater itself,
with kernel 3.0. My system was swapping like mad !

I use (kernel) ZFS on my 64-bit main machine and I'm plain happy with it, and
tried ZFS on my 32-bit laptop in the hope to get more performance for less
memory ; alas I just got a memory-hungry system running damn slow... For the
time being I will stick to ZFS for 64-bit machines with= 4GB RAM, and to
ext4 for 32-bit systems with less RAM...

I don't feel that BTRFS gives any advantage in its current state of
development. Alas.


--
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: WARNING: at fs/btrfs/inode.c:2193 btrfs_orphan_commit_root+0xb0/0xc0 [btrfs]()

2011-09-15 Thread Josef Bacik 
On Thu, Sep 15, 2011 at 11:44:09AM -0700, Sage Weil wrote:
 On Tue, 13 Sep 2011, Liu Bo wrote:
  On 09/11/2011 05:47 AM, Martin Mailand wrote:
   Hi
   I am hitting this Warning reproducible, the workload is a ceph osd,
   kernel ist 3.1.0-rc5.
   
  
  Have posted a patch for this:
  
  http://marc.info/?l=linux-btrfsm=131547325515336w=2
 
 We're still seeing this with -rc6, which includes 98c9942 and 65450aa.
 
 I haven't looked at the reservation code in much detail.  Is there 
 anything I can do to help track this down?
 

This should be taken care of with all my enospc changes.  You can pull them down
from my btrfs-work tree as soon as kernel.org comes back from the dead :).
Thanks,

Josef
--
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: WARNING: at fs/btrfs/inode.c:2193 btrfs_orphan_commit_root+0xb0/0xc0 [btrfs]()

2011-09-15 Thread David Sterba 
On Thu, Sep 15, 2011 at 11:44:09AM -0700, Sage Weil wrote:
 On Tue, 13 Sep 2011, Liu Bo wrote:
  On 09/11/2011 05:47 AM, Martin Mailand wrote:
   Hi
   I am hitting this Warning reproducible, the workload is a ceph osd,
   kernel ist 3.1.0-rc5.
   
  
  Have posted a patch for this:
  
  http://marc.info/?l=linux-btrfsm=131547325515336w=2
 
 We're still seeing this with -rc6, which includes 98c9942 and 65450aa.

Me too, for the 

WARNING: at fs/btrfs/extent-tree.c:5711 btrfs_alloc_free_block+0x180/0x350 
[btrfs]()

case mentioned in the changelog. I optimistically dropped the ratelimit
patch for that WARN_ON, but had to add it quickly back. Unfortunatelly I
do not have a reliable reproducer. It justs starts sometime during
xfstests, maybe after a few rounds.


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

Re: WARNING: at fs/btrfs/inode.c:2193 btrfs_orphan_commit_root+0xb0/0xc0 [btrfs]()

2011-09-15 Thread David Sterba 
On Thu, Sep 15, 2011 at 03:50:29PM -0400, Josef Bacik wrote:
  We're still seeing this with -rc6, which includes 98c9942 and 65450aa.
  
  I haven't looked at the reservation code in much detail.  Is there 
  anything I can do to help track this down?
  
 
 This should be taken care of with all my enospc changes.  You can pull them 
 down
 from my btrfs-work tree as soon as kernel.org comes back from the dead :).

should you need it earlier, here's a copy:

git://repo.or.cz/linux-2.6/btrfs-unstable.git #git.kernel.org/josef/master


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

Re: WARNING: at fs/btrfs/inode.c:2193 btrfs_orphan_commit_root+0xb0/0xc0 [btrfs]()

2011-09-15 Thread Sage Weil 
On Thu, 15 Sep 2011, David Sterba wrote:
 On Thu, Sep 15, 2011 at 03:50:29PM -0400, Josef Bacik wrote:
   We're still seeing this with -rc6, which includes 98c9942 and 65450aa.
   
   I haven't looked at the reservation code in much detail.  Is there 
   anything I can do to help track this down?
   
  
  This should be taken care of with all my enospc changes.  You can pull them 
  down
  from my btrfs-work tree as soon as kernel.org comes back from the dead :).
 
 should you need it earlier, here's a copy:
 
 git://repo.or.cz/linux-2.6/btrfs-unstable.git #git.kernel.org/josef/master

Thanks!  We'll do some testing today and see if it behaves better.  :)

sage
--
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 1/2] Btrfs: don't make a file partly checksummed through file clone

2011-09-15 Thread Li Zefan 
David Sterba wrote:
 On Wed, Sep 14, 2011 at 01:25:21PM +0800, Li Zefan wrote:
 It's because part of the file is checksummed and the other part is not,
 and then btrfs will complain checksum is not found when we read the file.

 Disallow file clone if src and dst file have different checksum flag,
 so we ensure a file is completely checksummed or unchecksummed.
 
 Your fix prevents the bug, but I don't think it's good to let file clone
 fail without any other message. ret is set to -EINVAL at the time of
 'goto out_fput', which is fine, but the user has no clue what happened
 or how to fix it.

While I agree with you on this comment..

 
 The nodatasum status is recorded in inode flags and remains like that
 regardless of the 'mount -o nodatasum', persistent and de facto
 unchangable (unless the file is created again with the opposite nodatasum
 mount). Even more, the user has no way to find out nodatasum flag of
 any inode/file (the corresponding FS_NODATASUM_FL is not there).
 
 My suggestion how to fix this:
 1. add FS_NODATASUM_FL file flag and code to set/get via setflags ioctl

This means we can have a file partly checksummed, which is what we want
to avoid in this patch.

 2. [this patch to skip cloning in case of nodatasum flag mismatch]
 3. ... add a printk why it failed

I don't think this is a good idea.

 
 The user then has at least option to drop/add the nodatasum flag for one of
 the. Unfortunatelly this makes file cloning less straightforward.
 

I don't know if Chris has plan on finer-grained checksum (not per file
but per extent), if yes, we can eliminate this constraint in file cloning
in the future.
--
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 2/2] Btrfs: don't change inode flag of the dest clone file

2011-09-15 Thread Jeff Liu 
Add CC to Coreutils, cp --reflink performs btrfs clone operation.

Thanks,
-Jeff
On 09/15/2011 07:43 PM, David Sterba wrote:

 On Wed, Sep 14, 2011 at 01:25:36PM +0800, Li Zefan wrote:
 The dst file will have the same inode flags with dst file after
 file clone, and I think it's unexpected.

 For example, the dst file will suddenly become immutable after
 getting some share of data with src file, if the src is immutable.
 
 Good catch! (I did not find any further direct assignment of two inode
 flags.)
 

 Signed-off-by: Li Zefan l...@cn.fujitsu.com
 Reviewed-by: David Sterba dste...@suse.cz
 
 IMNSHO should go to stable.
 
 
 david
 
 ---
  fs/btrfs/ioctl.c |1 -
  1 files changed, 0 insertions(+), 1 deletions(-)

 diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
 index dc82bbb..a401514 100644
 --- a/fs/btrfs/ioctl.c
 +++ b/fs/btrfs/ioctl.c
 @@ -2434,7 +2434,6 @@ static noinline long btrfs_ioctl_clone(struct file 
 *file, unsigned long srcfd,
  if (endoff  inode-i_size)
  btrfs_i_size_write(inode, endoff);
  
 -BTRFS_I(inode)-flags = BTRFS_I(src)-flags;
  ret = btrfs_update_inode(trans, root, inode);
  BUG_ON(ret);
  btrfs_end_transaction(trans, root);
 -- 1.7.3.1 
 --
 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
 --
 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


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