[PATCH] Btrfs: fix dentry->d_parent abuses

[PATCH] Btrfs: fix dentry->d_parent abuses

[Josef Bacik]

There are lots of places where we do dentry->d_parent->d_inode without holding
the dentry->d_lock.  This could cause problems with rename.  So instead use
dget_parent where we can, or in some cases we don't even need to use
dentry->d_parent->d_inode since we get the inode of the dir passed to us from
VFS.  I tested this with xfstests and my no space tests and everything turned
out fine.  Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 fs/btrfs/file.c        |    2 ++
 fs/btrfs/inode.c       |   48 ++++++++++++++++++++++++------------------------
 fs/btrfs/ioctl.c       |   11 +++++++++--
 fs/btrfs/transaction.c |    5 ++++-
 fs/btrfs/tree-log.c    |   22 ++++++++++++++++++----
 5 files changed, 57 insertions(+), 31 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index e354c33..6a4daa0 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1047,8 +1047,10 @@ out:
 
 		if ((file->f_flags & O_DSYNC) || IS_SYNC(inode)) {
 			trans = btrfs_start_transaction(root, 0);
+			mutex_lock(&inode->i_mutex);
 			ret = btrfs_log_dentry_safe(trans, root,
 						    file->f_dentry);
+			mutex_unlock(&inode->i_mutex);
 			if (ret == 0) {
 				ret = btrfs_sync_log(trans, root);
 				if (ret == 0)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 7146971..e77ee56 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4144,6 +4144,7 @@ static int btrfs_dentry_delete(struct dentry *dentry)
 		if (btrfs_root_refs(&root->root_item) == 0)
 			return 1;
 	}
+
 	return 0;
 }
 
@@ -4627,12 +4628,12 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
 }
 
 static int btrfs_add_nondir(struct btrfs_trans_handle *trans,
-			    struct dentry *dentry, struct inode *inode,
-			    int backref, u64 index)
+			    struct inode *dir, struct dentry *dentry,
+			    struct inode *inode, int backref, u64 index)
 {
-	int err = btrfs_add_link(trans, dentry->d_parent->d_inode,
-				 inode, dentry->d_name.name,
-				 dentry->d_name.len, backref, index);
+	int err = btrfs_add_link(trans, dir, inode,
+				 dentry->d_name.name, dentry->d_name.len,
+				 backref, index);
 	if (!err) {
 		d_instantiate(dentry, inode);
 		return 0;
@@ -4673,8 +4674,7 @@ static int btrfs_mknod(struct inode *dir, struct dentry *dentry,
 	btrfs_set_trans_block_group(trans, dir);
 
 	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
-				dentry->d_name.len,
-				dentry->d_parent->d_inode->i_ino, objectid,
+				dentry->d_name.len, dir->i_ino, objectid,
 				BTRFS_I(dir)->block_group, mode, &index);
 	err = PTR_ERR(inode);
 	if (IS_ERR(inode))
@@ -4687,7 +4687,7 @@ static int btrfs_mknod(struct inode *dir, struct dentry *dentry,
 	}
 
 	btrfs_set_trans_block_group(trans, inode);
-	err = btrfs_add_nondir(trans, dentry, inode, 0, index);
+	err = btrfs_add_nondir(trans, dir, dentry, inode, 0, index);
 	if (err)
 		drop_inode = 1;
 	else {
@@ -4735,10 +4735,8 @@ static int btrfs_create(struct inode *dir, struct dentry *dentry,
 	btrfs_set_trans_block_group(trans, dir);
 
 	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
-				dentry->d_name.len,
-				dentry->d_parent->d_inode->i_ino,
-				objectid, BTRFS_I(dir)->block_group, mode,
-				&index);
+				dentry->d_name.len, dir->i_ino, objectid,
+				BTRFS_I(dir)->block_group, mode, &index);
 	err = PTR_ERR(inode);
 	if (IS_ERR(inode))
 		goto out_unlock;
@@ -4750,7 +4748,7 @@ static int btrfs_create(struct inode *dir, struct dentry *dentry,
 	}
 
 	btrfs_set_trans_block_group(trans, inode);
-	err = btrfs_add_nondir(trans, dentry, inode, 0, index);
+	err = btrfs_add_nondir(trans, dir, dentry, inode, 0, index);
 	if (err)
 		drop_inode = 1;
 	else {
@@ -4810,15 +4808,17 @@ static int btrfs_link(struct dentry *old_dentry, struct inode *dir,
 	btrfs_set_trans_block_group(trans, dir);
 	atomic_inc(&inode->i_count);
 
-	err = btrfs_add_nondir(trans, dentry, inode, 1, index);
+	err = btrfs_add_nondir(trans, dir, dentry, inode, 1, index);
 
 	if (err) {
 		drop_inode = 1;
 	} else {
+		struct dentry *parent = dget_parent(dentry);
 		btrfs_update_inode_block_group(trans, dir);
 		err = btrfs_update_inode(trans, root, inode);
 		BUG_ON(err);
-		btrfs_log_new_name(trans, inode, NULL, dentry->d_parent);
+		btrfs_log_new_name(trans, inode, NULL, parent);
+		dput(parent);
 	}
 
 	nr = trans->blocks_used;
@@ -4858,8 +4858,7 @@ static int btrfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 	btrfs_set_trans_block_group(trans, dir);
 
 	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
-				dentry->d_name.len,
-				dentry->d_parent->d_inode->i_ino, objectid,
+				dentry->d_name.len, dir->i_ino, objectid,
 				BTRFS_I(dir)->block_group, S_IFDIR | mode,
 				&index);
 	if (IS_ERR(inode)) {
@@ -4882,9 +4881,8 @@ static int btrfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 	if (err)
 		goto out_fail;
 
-	err = btrfs_add_link(trans, dentry->d_parent->d_inode,
-				 inode, dentry->d_name.name,
-				 dentry->d_name.len, 0, index);
+	err = btrfs_add_link(trans, dir, inode, dentry->d_name.name,
+			     dentry->d_name.len, 0, index);
 	if (err)
 		goto out_fail;
 
@@ -6613,8 +6611,11 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 	BUG_ON(ret);
 
 	if (old_inode->i_ino != BTRFS_FIRST_FREE_OBJECTID) {
+		struct dentry *parent = dget_parent(new_dentry);
+
 		btrfs_log_new_name(trans, old_inode, old_dir,
-				   new_dentry->d_parent);
+				   parent);
+		dput(parent);
 		btrfs_end_log_trans(root);
 	}
 out_fail:
@@ -6764,8 +6765,7 @@ static int btrfs_symlink(struct inode *dir, struct dentry *dentry,
 	btrfs_set_trans_block_group(trans, dir);
 
 	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
-				dentry->d_name.len,
-				dentry->d_parent->d_inode->i_ino, objectid,
+				dentry->d_name.len, dir->i_ino, objectid,
 				BTRFS_I(dir)->block_group, S_IFLNK|S_IRWXUGO,
 				&index);
 	err = PTR_ERR(inode);
@@ -6779,7 +6779,7 @@ static int btrfs_symlink(struct inode *dir, struct dentry *dentry,
 	}
 
 	btrfs_set_trans_block_group(trans, inode);
-	err = btrfs_add_nondir(trans, dentry, inode, 0, index);
+	err = btrfs_add_nondir(trans, dir, dentry, inode, 0, index);
 	if (err)
 		drop_inode = 1;
 	else {
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e264072..396ccd1 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -232,13 +232,17 @@ static noinline int create_subvol(struct btrfs_root *root,
 	struct btrfs_inode_item *inode_item;
 	struct extent_buffer *leaf;
 	struct btrfs_root *new_root;
-	struct inode *dir = dentry->d_parent->d_inode;
+	struct dentry *parent = dget_parent(dentry);
+	struct inode *dir;
 	int ret;
 	int err;
 	u64 objectid;
 	u64 new_dirid = BTRFS_FIRST_FREE_OBJECTID;
 	u64 index = 0;
 
+	dir = parent->d_inode;
+	dput(parent);
+
 	ret = btrfs_find_free_objectid(NULL, root->fs_info->tree_root,
 				       0, &objectid);
 	if (ret)
@@ -347,6 +351,7 @@ fail:
 static int create_snapshot(struct btrfs_root *root, struct dentry *dentry)
 {
 	struct inode *inode;
+	struct dentry *parent;
 	struct btrfs_pending_snapshot *pending_snapshot;
 	struct btrfs_trans_handle *trans;
 	int ret;
@@ -382,7 +387,9 @@ static int create_snapshot(struct btrfs_root *root, struct dentry *dentry)
 
 	btrfs_orphan_cleanup(pending_snapshot->snap);
 
-	inode = btrfs_lookup_dentry(dentry->d_parent->d_inode, dentry);
+	parent = dget_parent(dentry);
+	inode = btrfs_lookup_dentry(parent->d_inode, dentry);
+	dput(parent);
 	if (IS_ERR(inode)) {
 		ret = PTR_ERR(inode);
 		goto fail;
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 0af647c..076729e 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -849,6 +849,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	struct btrfs_root *root = pending->root;
 	struct btrfs_root *parent_root;
 	struct inode *parent_inode;
+	struct dentry *parent_dentry;
 	struct dentry *dentry;
 	struct extent_buffer *tmp;
 	struct extent_buffer *old;
@@ -888,7 +889,9 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	trans->block_rsv = &pending->block_rsv;
 
 	dentry = pending->dentry;
-	parent_inode = dentry->d_parent->d_inode;
+	parent_dentry = dget_parent(dentry);
+	parent_inode = parent_dentry->d_inode;
+	dput(parent_dentry);
 	parent_root = BTRFS_I(parent_inode)->root;
 	record_root_in_trans(trans, parent_root);
 
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index fb102a9..bf01bdb 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2884,6 +2884,7 @@ static noinline int check_parent_dirs_for_sync(struct btrfs_trans_handle *trans,
 {
 	int ret = 0;
 	struct btrfs_root *root;
+	struct dentry *old_parent = NULL;
 
 	/*
 	 * for regular files, if its inode is already on disk, we don't
@@ -2925,10 +2926,13 @@ static noinline int check_parent_dirs_for_sync(struct btrfs_trans_handle *trans,
 		if (IS_ROOT(parent))
 			break;
 
-		parent = parent->d_parent;
+		parent = dget_parent(parent);
+		dput(old_parent);
+		old_parent = parent;
 		inode = parent->d_inode;
 
 	}
+	dput(old_parent);
 out:
 	return ret;
 }
@@ -2960,6 +2964,7 @@ int btrfs_log_inode_parent(struct btrfs_trans_handle *trans,
 {
 	int inode_only = exists_only ? LOG_INODE_EXISTS : LOG_INODE_ALL;
 	struct super_block *sb;
+	struct dentry *old_parent = NULL;
 	int ret = 0;
 	u64 last_committed = root->fs_info->last_trans_committed;
 
@@ -3031,10 +3036,13 @@ int btrfs_log_inode_parent(struct btrfs_trans_handle *trans,
 		if (IS_ROOT(parent))
 			break;
 
-		parent = parent->d_parent;
+		parent = dget_parent(parent);
+		dput(old_parent);
+		old_parent = parent;
 	}
 	ret = 0;
 end_trans:
+	dput(old_parent);
 	if (ret < 0) {
 		BUG_ON(ret != -ENOSPC);
 		root->fs_info->last_trans_log_full_commit = trans->transid;
@@ -3054,8 +3062,14 @@ end_no_trans:
 int btrfs_log_dentry_safe(struct btrfs_trans_handle *trans,
 			  struct btrfs_root *root, struct dentry *dentry)
 {
-	return btrfs_log_inode_parent(trans, root, dentry->d_inode,
-				      dentry->d_parent, 0);
+	struct dentry *parent = dget_parent(dentry);
+	int ret;
+
+	ret = btrfs_log_inode_parent(trans, root, dentry->d_inode, parent, 0);
+
+	dput(parent);
+
+	return ret;
 }
 
 /*
-- 
1.6.6.1

Re: system crash at mounting of btrfs

[Erik Hoppe]

I did another test with 2.6.36 same results (system instantly crash
after mount, even in read only mode) 

Userland View (mount filesystem image)
------------------------------ snip ------------------------------
vbox portable # uname -a
Linux vbox 2.6.36-gentoo-VBOX #1 SMP Tue Oct 26 12:56:53 CEST 2010
x86_64 Intel(R) Xeon(R) CPU W3520 @ 2.67GHz GenuineIntel GNU/Linux
vbox portable # modprobe loop   
vbox portable # losetup /dev/loop0 sd.img 
vbox portable # mount -t btrfs -o ro /dev/loop0 /mnt/sdcard/
------------------------------ snip ------------------------------

Kernel View
------------------------------ snip ------------------------------
Btrfs loaded
device label home devid 1 transid 122306 /dev/loop0
------------------------------ snip ------------------------------


btrfsck report (everything ok)
------------------------------ snip ------------------------------
vbox portable # btrfsck sd.img 
found 4694925312 bytes used err is 0
total csum bytes: 4518676
total tree bytes: 67801088
total fs tree bytes: 53567488
btree space waste bytes: 18724362
file data blocks allocated: 5819858944
 referenced 5092511744
Btrfs Btrfs v0.19
------------------------------ snip ------------------------------

any help appreciated


regards Erik

Re: [PATCH] Btrfs: fix dentry->d_parent abuses

[Ian Kent]

On Wed, 2010-10-27 at 10:39 -0400, Josef Bacik wrote:
> There are lots of places where we do dentry->d_parent->d_inode without holding
> the dentry->d_lock.  This could cause problems with rename.  So instead use
> dget_parent where we can, or in some cases we don't even need to use
> dentry->d_parent->d_inode since we get the inode of the dir passed to us from
> VFS.  I tested this with xfstests and my no space tests and everything turned
> out fine.  Thanks,
> 
> Signed-off-by: Josef Bacik <josef@redhat.com>
> ---
>  fs/btrfs/file.c        |    2 ++
>  fs/btrfs/inode.c       |   48 ++++++++++++++++++++++++------------------------
>  fs/btrfs/ioctl.c       |   11 +++++++++--
>  fs/btrfs/transaction.c |    5 ++++-
>  fs/btrfs/tree-log.c    |   22 ++++++++++++++++++----
>  5 files changed, 57 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index e354c33..6a4daa0 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1047,8 +1047,10 @@ out:
>  
>  		if ((file->f_flags & O_DSYNC) || IS_SYNC(inode)) {
>  			trans = btrfs_start_transaction(root, 0);

If btrfs_start_transaction() fails now the inode mutex will be held as
well. I guess the resulting oops makes this irrelevant, ;)

Looking through the dead end BUG_ON()s in the transaction code, and
callers, one thing I've found difficult is working out how to recover
from IS_ERR() returns from btrfs_start_transaction() in situations like
this.

Any chance of a patch to handle this to give me an example (well one
case anyway) of how to do it?

> +			mutex_lock(&inode->i_mutex);
>  			ret = btrfs_log_dentry_safe(trans, root,
>  						    file->f_dentry);
> +			mutex_unlock(&inode->i_mutex);
>  			if (ret == 0) {
>  				ret = btrfs_sync_log(trans, root);
>  				if (ret == 0)
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 7146971..e77ee56 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4144,6 +4144,7 @@ static int btrfs_dentry_delete(struct dentry *dentry)
>  		if (btrfs_root_refs(&root->root_item) == 0)
>  			return 1;
>  	}
> +
>  	return 0;
>  }
>  
> @@ -4627,12 +4628,12 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
>  }
>  
>  static int btrfs_add_nondir(struct btrfs_trans_handle *trans,
> -			    struct dentry *dentry, struct inode *inode,
> -			    int backref, u64 index)
> +			    struct inode *dir, struct dentry *dentry,
> +			    struct inode *inode, int backref, u64 index)
>  {
> -	int err = btrfs_add_link(trans, dentry->d_parent->d_inode,
> -				 inode, dentry->d_name.name,
> -				 dentry->d_name.len, backref, index);
> +	int err = btrfs_add_link(trans, dir, inode,
> +				 dentry->d_name.name, dentry->d_name.len,
> +				 backref, index);
>  	if (!err) {
>  		d_instantiate(dentry, inode);
>  		return 0;
> @@ -4673,8 +4674,7 @@ static int btrfs_mknod(struct inode *dir, struct dentry *dentry,
>  	btrfs_set_trans_block_group(trans, dir);
>  
>  	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
> -				dentry->d_name.len,
> -				dentry->d_parent->d_inode->i_ino, objectid,
> +				dentry->d_name.len, dir->i_ino, objectid,
>  				BTRFS_I(dir)->block_group, mode, &index);
>  	err = PTR_ERR(inode);
>  	if (IS_ERR(inode))
> @@ -4687,7 +4687,7 @@ static int btrfs_mknod(struct inode *dir, struct dentry *dentry,
>  	}
>  
>  	btrfs_set_trans_block_group(trans, inode);
> -	err = btrfs_add_nondir(trans, dentry, inode, 0, index);
> +	err = btrfs_add_nondir(trans, dir, dentry, inode, 0, index);
>  	if (err)
>  		drop_inode = 1;
>  	else {
> @@ -4735,10 +4735,8 @@ static int btrfs_create(struct inode *dir, struct dentry *dentry,
>  	btrfs_set_trans_block_group(trans, dir);
>  
>  	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
> -				dentry->d_name.len,
> -				dentry->d_parent->d_inode->i_ino,
> -				objectid, BTRFS_I(dir)->block_group, mode,
> -				&index);
> +				dentry->d_name.len, dir->i_ino, objectid,
> +				BTRFS_I(dir)->block_group, mode, &index);
>  	err = PTR_ERR(inode);
>  	if (IS_ERR(inode))
>  		goto out_unlock;
> @@ -4750,7 +4748,7 @@ static int btrfs_create(struct inode *dir, struct dentry *dentry,
>  	}
>  
>  	btrfs_set_trans_block_group(trans, inode);
> -	err = btrfs_add_nondir(trans, dentry, inode, 0, index);
> +	err = btrfs_add_nondir(trans, dir, dentry, inode, 0, index);
>  	if (err)
>  		drop_inode = 1;
>  	else {
> @@ -4810,15 +4808,17 @@ static int btrfs_link(struct dentry *old_dentry, struct inode *dir,
>  	btrfs_set_trans_block_group(trans, dir);
>  	atomic_inc(&inode->i_count);
>  
> -	err = btrfs_add_nondir(trans, dentry, inode, 1, index);
> +	err = btrfs_add_nondir(trans, dir, dentry, inode, 1, index);
>  
>  	if (err) {
>  		drop_inode = 1;
>  	} else {
> +		struct dentry *parent = dget_parent(dentry);
>  		btrfs_update_inode_block_group(trans, dir);
>  		err = btrfs_update_inode(trans, root, inode);
>  		BUG_ON(err);
> -		btrfs_log_new_name(trans, inode, NULL, dentry->d_parent);
> +		btrfs_log_new_name(trans, inode, NULL, parent);
> +		dput(parent);
>  	}
>  
>  	nr = trans->blocks_used;
> @@ -4858,8 +4858,7 @@ static int btrfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
>  	btrfs_set_trans_block_group(trans, dir);
>  
>  	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
> -				dentry->d_name.len,
> -				dentry->d_parent->d_inode->i_ino, objectid,
> +				dentry->d_name.len, dir->i_ino, objectid,
>  				BTRFS_I(dir)->block_group, S_IFDIR | mode,
>  				&index);
>  	if (IS_ERR(inode)) {
> @@ -4882,9 +4881,8 @@ static int btrfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
>  	if (err)
>  		goto out_fail;
>  
> -	err = btrfs_add_link(trans, dentry->d_parent->d_inode,
> -				 inode, dentry->d_name.name,
> -				 dentry->d_name.len, 0, index);
> +	err = btrfs_add_link(trans, dir, inode, dentry->d_name.name,
> +			     dentry->d_name.len, 0, index);
>  	if (err)
>  		goto out_fail;
>  
> @@ -6613,8 +6611,11 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  	BUG_ON(ret);
>  
>  	if (old_inode->i_ino != BTRFS_FIRST_FREE_OBJECTID) {
> +		struct dentry *parent = dget_parent(new_dentry);
> +
>  		btrfs_log_new_name(trans, old_inode, old_dir,
> -				   new_dentry->d_parent);
> +				   parent);
> +		dput(parent);
>  		btrfs_end_log_trans(root);
>  	}
>  out_fail:
> @@ -6764,8 +6765,7 @@ static int btrfs_symlink(struct inode *dir, struct dentry *dentry,
>  	btrfs_set_trans_block_group(trans, dir);
>  
>  	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
> -				dentry->d_name.len,
> -				dentry->d_parent->d_inode->i_ino, objectid,
> +				dentry->d_name.len, dir->i_ino, objectid,
>  				BTRFS_I(dir)->block_group, S_IFLNK|S_IRWXUGO,
>  				&index);
>  	err = PTR_ERR(inode);
> @@ -6779,7 +6779,7 @@ static int btrfs_symlink(struct inode *dir, struct dentry *dentry,
>  	}
>  
>  	btrfs_set_trans_block_group(trans, inode);
> -	err = btrfs_add_nondir(trans, dentry, inode, 0, index);
> +	err = btrfs_add_nondir(trans, dir, dentry, inode, 0, index);
>  	if (err)
>  		drop_inode = 1;
>  	else {
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index e264072..396ccd1 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -232,13 +232,17 @@ static noinline int create_subvol(struct btrfs_root *root,
>  	struct btrfs_inode_item *inode_item;
>  	struct extent_buffer *leaf;
>  	struct btrfs_root *new_root;
> -	struct inode *dir = dentry->d_parent->d_inode;
> +	struct dentry *parent = dget_parent(dentry);
> +	struct inode *dir;
>  	int ret;
>  	int err;
>  	u64 objectid;
>  	u64 new_dirid = BTRFS_FIRST_FREE_OBJECTID;
>  	u64 index = 0;
>  
> +	dir = parent->d_inode;
> +	dput(parent);

I get that parent is, well, the parent so the child were using will hold
a reference to it and so the dput() should be safe. But, since we have
the reference why not hold it while we use the inode to make certain the
inode cannot go away?

> +
>  	ret = btrfs_find_free_objectid(NULL, root->fs_info->tree_root,
>  				       0, &objectid);
>  	if (ret)
> @@ -347,6 +351,7 @@ fail:
>  static int create_snapshot(struct btrfs_root *root, struct dentry *dentry)
>  {
>  	struct inode *inode;
> +	struct dentry *parent;
>  	struct btrfs_pending_snapshot *pending_snapshot;
>  	struct btrfs_trans_handle *trans;
>  	int ret;
> @@ -382,7 +387,9 @@ static int create_snapshot(struct btrfs_root *root, struct dentry *dentry)
>  
>  	btrfs_orphan_cleanup(pending_snapshot->snap);
>  
> -	inode = btrfs_lookup_dentry(dentry->d_parent->d_inode, dentry);
> +	parent = dget_parent(dentry);
> +	inode = btrfs_lookup_dentry(parent->d_inode, dentry);
> +	dput(parent);
>  	if (IS_ERR(inode)) {
>  		ret = PTR_ERR(inode);
>  		goto fail;
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 0af647c..076729e 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -849,6 +849,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
>  	struct btrfs_root *root = pending->root;
>  	struct btrfs_root *parent_root;
>  	struct inode *parent_inode;
> +	struct dentry *parent_dentry;
>  	struct dentry *dentry;
>  	struct extent_buffer *tmp;
>  	struct extent_buffer *old;
> @@ -888,7 +889,9 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
>  	trans->block_rsv = &pending->block_rsv;
>  
>  	dentry = pending->dentry;
> -	parent_inode = dentry->d_parent->d_inode;
> +	parent_dentry = dget_parent(dentry);
> +	parent_inode = parent_dentry->d_inode;
> +	dput(parent_dentry);

And again.

>  	parent_root = BTRFS_I(parent_inode)->root;
>  	record_root_in_trans(trans, parent_root);
>  
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index fb102a9..bf01bdb 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -2884,6 +2884,7 @@ static noinline int check_parent_dirs_for_sync(struct btrfs_trans_handle *trans,
>  {
>  	int ret = 0;
>  	struct btrfs_root *root;
> +	struct dentry *old_parent = NULL;
>  
>  	/*
>  	 * for regular files, if its inode is already on disk, we don't
> @@ -2925,10 +2926,13 @@ static noinline int check_parent_dirs_for_sync(struct btrfs_trans_handle *trans,
>  		if (IS_ROOT(parent))
>  			break;
>  
> -		parent = parent->d_parent;
> +		parent = dget_parent(parent);
> +		dput(old_parent);
> +		old_parent = parent;
>  		inode = parent->d_inode;
>  
>  	}
> +	dput(old_parent);
>  out:
>  	return ret;
>  }
> @@ -2960,6 +2964,7 @@ int btrfs_log_inode_parent(struct btrfs_trans_handle *trans,
>  {
>  	int inode_only = exists_only ? LOG_INODE_EXISTS : LOG_INODE_ALL;
>  	struct super_block *sb;
> +	struct dentry *old_parent = NULL;
>  	int ret = 0;
>  	u64 last_committed = root->fs_info->last_trans_committed;
>  
> @@ -3031,10 +3036,13 @@ int btrfs_log_inode_parent(struct btrfs_trans_handle *trans,
>  		if (IS_ROOT(parent))
>  			break;
>  
> -		parent = parent->d_parent;
> +		parent = dget_parent(parent);
> +		dput(old_parent);
> +		old_parent = parent;
>  	}
>  	ret = 0;
>  end_trans:
> +	dput(old_parent);
>  	if (ret < 0) {
>  		BUG_ON(ret != -ENOSPC);
>  		root->fs_info->last_trans_log_full_commit = trans->transid;
> @@ -3054,8 +3062,14 @@ end_no_trans:
>  int btrfs_log_dentry_safe(struct btrfs_trans_handle *trans,
>  			  struct btrfs_root *root, struct dentry *dentry)
>  {
> -	return btrfs_log_inode_parent(trans, root, dentry->d_inode,
> -				      dentry->d_parent, 0);
> +	struct dentry *parent = dget_parent(dentry);
> +	int ret;
> +
> +	ret = btrfs_log_inode_parent(trans, root, dentry->d_inode, parent, 0);
> +
> +	dput(parent);
> +
> +	return ret;
>  }
>  
>  /*

Re: [PATCH] Btrfs: fix dentry->d_parent abuses

[Josef Bacik]

On Thu, Oct 28, 2010 at 11:42:04AM +0800, Ian Kent wrote:
> On Wed, 2010-10-27 at 10:39 -0400, Josef Bacik wrote:
> > There are lots of places where we do dentry->d_parent->d_inode without holding
> > the dentry->d_lock.  This could cause problems with rename.  So instead use
> > dget_parent where we can, or in some cases we don't even need to use
> > dentry->d_parent->d_inode since we get the inode of the dir passed to us from
> > VFS.  I tested this with xfstests and my no space tests and everything turned
> > out fine.  Thanks,
> > 
> > Signed-off-by: Josef Bacik <josef@redhat.com>
> > ---
> >  fs/btrfs/file.c        |    2 ++
> >  fs/btrfs/inode.c       |   48 ++++++++++++++++++++++++------------------------
> >  fs/btrfs/ioctl.c       |   11 +++++++++--
> >  fs/btrfs/transaction.c |    5 ++++-
> >  fs/btrfs/tree-log.c    |   22 ++++++++++++++++++----
> >  5 files changed, 57 insertions(+), 31 deletions(-)
> > 
> > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> > index e354c33..6a4daa0 100644
> > --- a/fs/btrfs/file.c
> > +++ b/fs/btrfs/file.c
> > @@ -1047,8 +1047,10 @@ out:
> >  
> >  		if ((file->f_flags & O_DSYNC) || IS_SYNC(inode)) {
> >  			trans = btrfs_start_transaction(root, 0);
> 
> If btrfs_start_transaction() fails now the inode mutex will be held as
> well. I guess the resulting oops makes this irrelevant, ;)
> 
> Looking through the dead end BUG_ON()s in the transaction code, and
> callers, one thing I've found difficult is working out how to recover
> from IS_ERR() returns from btrfs_start_transaction() in situations like
> this.
> 
> Any chance of a patch to handle this to give me an example (well one
> case anyway) of how to do it?
>

Yup I can do that.  Recovering from some of these errors is going to be tricky,
if you are going to work on that what I had in mind was doing alot of

static foo = 0;

foo++;

if (foo % 10000)
	return (error where we used to BUG_ON());

so to be sure that all of a sudden returning an error doesn't cause an panic
farther up the callchain.
 
> > +			mutex_lock(&inode->i_mutex);
> >  			ret = btrfs_log_dentry_safe(trans, root,
> >  						    file->f_dentry);
> > +			mutex_unlock(&inode->i_mutex);
> >  			if (ret == 0) {
> >  				ret = btrfs_sync_log(trans, root);
> >  				if (ret == 0)
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 7146971..e77ee56 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -4144,6 +4144,7 @@ static int btrfs_dentry_delete(struct dentry *dentry)
> >  		if (btrfs_root_refs(&root->root_item) == 0)
> >  			return 1;
> >  	}
> > +
> >  	return 0;
> >  }
> >  
> > @@ -4627,12 +4628,12 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
> >  }
> >  
> >  static int btrfs_add_nondir(struct btrfs_trans_handle *trans,
> > -			    struct dentry *dentry, struct inode *inode,
> > -			    int backref, u64 index)
> > +			    struct inode *dir, struct dentry *dentry,
> > +			    struct inode *inode, int backref, u64 index)
> >  {
> > -	int err = btrfs_add_link(trans, dentry->d_parent->d_inode,
> > -				 inode, dentry->d_name.name,
> > -				 dentry->d_name.len, backref, index);
> > +	int err = btrfs_add_link(trans, dir, inode,
> > +				 dentry->d_name.name, dentry->d_name.len,
> > +				 backref, index);
> >  	if (!err) {
> >  		d_instantiate(dentry, inode);
> >  		return 0;
> > @@ -4673,8 +4674,7 @@ static int btrfs_mknod(struct inode *dir, struct dentry *dentry,
> >  	btrfs_set_trans_block_group(trans, dir);
> >  
> >  	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
> > -				dentry->d_name.len,
> > -				dentry->d_parent->d_inode->i_ino, objectid,
> > +				dentry->d_name.len, dir->i_ino, objectid,
> >  				BTRFS_I(dir)->block_group, mode, &index);
> >  	err = PTR_ERR(inode);
> >  	if (IS_ERR(inode))
> > @@ -4687,7 +4687,7 @@ static int btrfs_mknod(struct inode *dir, struct dentry *dentry,
> >  	}
> >  
> >  	btrfs_set_trans_block_group(trans, inode);
> > -	err = btrfs_add_nondir(trans, dentry, inode, 0, index);
> > +	err = btrfs_add_nondir(trans, dir, dentry, inode, 0, index);
> >  	if (err)
> >  		drop_inode = 1;
> >  	else {
> > @@ -4735,10 +4735,8 @@ static int btrfs_create(struct inode *dir, struct dentry *dentry,
> >  	btrfs_set_trans_block_group(trans, dir);
> >  
> >  	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
> > -				dentry->d_name.len,
> > -				dentry->d_parent->d_inode->i_ino,
> > -				objectid, BTRFS_I(dir)->block_group, mode,
> > -				&index);
> > +				dentry->d_name.len, dir->i_ino, objectid,
> > +				BTRFS_I(dir)->block_group, mode, &index);
> >  	err = PTR_ERR(inode);
> >  	if (IS_ERR(inode))
> >  		goto out_unlock;
> > @@ -4750,7 +4748,7 @@ static int btrfs_create(struct inode *dir, struct dentry *dentry,
> >  	}
> >  
> >  	btrfs_set_trans_block_group(trans, inode);
> > -	err = btrfs_add_nondir(trans, dentry, inode, 0, index);
> > +	err = btrfs_add_nondir(trans, dir, dentry, inode, 0, index);
> >  	if (err)
> >  		drop_inode = 1;
> >  	else {
> > @@ -4810,15 +4808,17 @@ static int btrfs_link(struct dentry *old_dentry, struct inode *dir,
> >  	btrfs_set_trans_block_group(trans, dir);
> >  	atomic_inc(&inode->i_count);
> >  
> > -	err = btrfs_add_nondir(trans, dentry, inode, 1, index);
> > +	err = btrfs_add_nondir(trans, dir, dentry, inode, 1, index);
> >  
> >  	if (err) {
> >  		drop_inode = 1;
> >  	} else {
> > +		struct dentry *parent = dget_parent(dentry);
> >  		btrfs_update_inode_block_group(trans, dir);
> >  		err = btrfs_update_inode(trans, root, inode);
> >  		BUG_ON(err);
> > -		btrfs_log_new_name(trans, inode, NULL, dentry->d_parent);
> > +		btrfs_log_new_name(trans, inode, NULL, parent);
> > +		dput(parent);
> >  	}
> >  
> >  	nr = trans->blocks_used;
> > @@ -4858,8 +4858,7 @@ static int btrfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
> >  	btrfs_set_trans_block_group(trans, dir);
> >  
> >  	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
> > -				dentry->d_name.len,
> > -				dentry->d_parent->d_inode->i_ino, objectid,
> > +				dentry->d_name.len, dir->i_ino, objectid,
> >  				BTRFS_I(dir)->block_group, S_IFDIR | mode,
> >  				&index);
> >  	if (IS_ERR(inode)) {
> > @@ -4882,9 +4881,8 @@ static int btrfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
> >  	if (err)
> >  		goto out_fail;
> >  
> > -	err = btrfs_add_link(trans, dentry->d_parent->d_inode,
> > -				 inode, dentry->d_name.name,
> > -				 dentry->d_name.len, 0, index);
> > +	err = btrfs_add_link(trans, dir, inode, dentry->d_name.name,
> > +			     dentry->d_name.len, 0, index);
> >  	if (err)
> >  		goto out_fail;
> >  
> > @@ -6613,8 +6611,11 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry,
> >  	BUG_ON(ret);
> >  
> >  	if (old_inode->i_ino != BTRFS_FIRST_FREE_OBJECTID) {
> > +		struct dentry *parent = dget_parent(new_dentry);
> > +
> >  		btrfs_log_new_name(trans, old_inode, old_dir,
> > -				   new_dentry->d_parent);
> > +				   parent);
> > +		dput(parent);
> >  		btrfs_end_log_trans(root);
> >  	}
> >  out_fail:
> > @@ -6764,8 +6765,7 @@ static int btrfs_symlink(struct inode *dir, struct dentry *dentry,
> >  	btrfs_set_trans_block_group(trans, dir);
> >  
> >  	inode = btrfs_new_inode(trans, root, dir, dentry->d_name.name,
> > -				dentry->d_name.len,
> > -				dentry->d_parent->d_inode->i_ino, objectid,
> > +				dentry->d_name.len, dir->i_ino, objectid,
> >  				BTRFS_I(dir)->block_group, S_IFLNK|S_IRWXUGO,
> >  				&index);
> >  	err = PTR_ERR(inode);
> > @@ -6779,7 +6779,7 @@ static int btrfs_symlink(struct inode *dir, struct dentry *dentry,
> >  	}
> >  
> >  	btrfs_set_trans_block_group(trans, inode);
> > -	err = btrfs_add_nondir(trans, dentry, inode, 0, index);
> > +	err = btrfs_add_nondir(trans, dir, dentry, inode, 0, index);
> >  	if (err)
> >  		drop_inode = 1;
> >  	else {
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index e264072..396ccd1 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -232,13 +232,17 @@ static noinline int create_subvol(struct btrfs_root *root,
> >  	struct btrfs_inode_item *inode_item;
> >  	struct extent_buffer *leaf;
> >  	struct btrfs_root *new_root;
> > -	struct inode *dir = dentry->d_parent->d_inode;
> > +	struct dentry *parent = dget_parent(dentry);
> > +	struct inode *dir;
> >  	int ret;
> >  	int err;
> >  	u64 objectid;
> >  	u64 new_dirid = BTRFS_FIRST_FREE_OBJECTID;
> >  	u64 index = 0;
> >  
> > +	dir = parent->d_inode;
> > +	dput(parent);
> 
> I get that parent is, well, the parent so the child were using will hold
> a reference to it and so the dput() should be safe. But, since we have
> the reference why not hold it while we use the inode to make certain the
> inode cannot go away?
>

Yeah good point, I wasn't entirely sure how to deal with these cases so I just
made something up.  I'm happy to have a more solid alternative.  Thanks for the
review, I'll fix all this up at a more reasonable hour,

Josef 

Re: [PATCH] Btrfs: fix dentry->d_parent abuses

[Ian Kent]

On Thu, 2010-10-28 at 02:43 -0400, Josef Bacik wrote:
> On Thu, Oct 28, 2010 at 11:42:04AM +0800, Ian Kent wrote:
> > On Wed, 2010-10-27 at 10:39 -0400, Josef Bacik wrote:
> > > There are lots of places where we do dentry->d_parent->d_inode without holding
> > > the dentry->d_lock.  This could cause problems with rename.  So instead use
> > > dget_parent where we can, or in some cases we don't even need to use
> > > dentry->d_parent->d_inode since we get the inode of the dir passed to us from
> > > VFS.  I tested this with xfstests and my no space tests and everything turned
> > > out fine.  Thanks,
> > > 
> > > Signed-off-by: Josef Bacik <josef@redhat.com>
> > > ---
> > >  fs/btrfs/file.c        |    2 ++
> > >  fs/btrfs/inode.c       |   48 ++++++++++++++++++++++++------------------------
> > >  fs/btrfs/ioctl.c       |   11 +++++++++--
> > >  fs/btrfs/transaction.c |    5 ++++-
> > >  fs/btrfs/tree-log.c    |   22 ++++++++++++++++++----
> > >  5 files changed, 57 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> > > index e354c33..6a4daa0 100644
> > > --- a/fs/btrfs/file.c
> > > +++ b/fs/btrfs/file.c
> > > @@ -1047,8 +1047,10 @@ out:
> > >  
> > >  		if ((file->f_flags & O_DSYNC) || IS_SYNC(inode)) {
> > >  			trans = btrfs_start_transaction(root, 0);
> > 
> > If btrfs_start_transaction() fails now the inode mutex will be held as
> > well. I guess the resulting oops makes this irrelevant, ;)
> > 
> > Looking through the dead end BUG_ON()s in the transaction code, and
> > callers, one thing I've found difficult is working out how to recover
> > from IS_ERR() returns from btrfs_start_transaction() in situations like
> > this.
> > 
> > Any chance of a patch to handle this to give me an example (well one
> > case anyway) of how to do it?
> >
> 
> Yup I can do that.  Recovering from some of these errors is going to be tricky,
> if you are going to work on that what I had in mind was doing alot of

Yep, looks downright difficult to me, but it has to be done sometime.

I've looked at a few functions in relation to eliminating dead end
BUG_ON()s (loosely related to this), some at the bottom of the call tree
upward and others at the top of the call tree downward, and it doesn't
take much to realize why no-one has tackled it already.

The bigger issue is that the problem is self perpetuating in that, in
order to fix things, often one has to continue to not handle some of the
error cases because of possible side effects, ouch!

Ian

Re: system crash at mounting of btrfs

[Erik Hoppe]

Hi There,

I got the same problem (freezing kernel while mounting btfs disk). My block 
device is a 8gb sdcard, it should be physical intact cause 
i can dd the card into a file without problems.
Here is what i did so far:
mount mmcblk0 under 2.6.35 (freeze)
mount the diskdumped image as a loop device on another system using 2.6.35
and current git kernel (freeze)

so i'm pretty much lost a.t.m.

I'm willing to send the metadata image (instructions please) and Chris
could you please send me your wipe-log tool
so i can maybee restore my data files ;)
 

greetings

erik

Re: system crash at mounting of btrfs

[Gerhard Kulzer]

Chris Mason <chris.mason <at> oracle.com> writes:


> > Its a single device Intel X25-M G2 Postville 80GB
> 
> Ok, if you can confirm this is the very first oops you see:
> 
> kernel BUG at /build/buildd/linux-2.6.35/fs/btrfs/tree-log.c:813!
> 
> I've got a utility that will wipe the log and get you mountable again.
> The intel drive should definitely support barriers, are you doing
> anything with LVM or raid or dm-crypt on top of it?
> 
I think somebody replied in my place...

I don't use any lvm or raid or dm_crypt, just 1 straight partition formatted as
btrfs.

Gerhard

Re: system crash at mounting of btrfs

[Francis Galiegue]

On Wed, Oct 13, 2010 at 02:49, Chris Mason <chris.mason@oracle.com> wro=
te:
[...]
>
> I've got a utility that will wipe the log and get you mountable again=
=2E
> The intel drive should definitely support barriers, are you doing
> anything with LVM or raid or dm-crypt on top of it?
>

Not Intel related and I don't use dm-crypt, but I use btrfs on three
machines over RAID 1 (MD) + LVM, all with SATA disks (and I've learned
the lesson: write caching disabled on all drives). Are there ill
effects to such setups?


--=20
=46rancis Galiegue, fgaliegue@gmail.com
"It seems obvious [...] that at least some 'business intelligence'
tools invest so much intelligence on the business side that they have
nothing left for generating SQL queries" (St=C3=A9phane Faroult, in "Th=
e
Art of SQL", ISBN 0-596-00894-5)
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: system crash at mounting of btrfs

[Chris Mason]

On Tue, Oct 12, 2010 at 06:44:13AM +0000, Gerhard Kulzer wrote:
> Chris Mason <chris.mason <at> oracle.com> writes:
> 
> 
> > 
> > Ok, so tell me more about the SSD with the btrfs-of-death.  Is it a
> > multi-device SSD?
> > 
> > -chris
> > 
> Its a single device Intel X25-M G2 Postville 80GB

Ok, if you can confirm this is the very first oops you see:

[    7.923556] kernel BUG at /build/buildd/linux-2.6.35/fs/btrfs/tree-log.c:813!

I've got a utility that will wipe the log and get you mountable again.
The intel drive should definitely support barriers, are you doing
anything with LVM or raid or dm-crypt on top of it?

-chris

Re: system crash at mounting of btrfs

[Gerhard Kulzer]

Chris Mason <chris.mason <at> oracle.com> writes:


> 
> Ok, so tell me more about the SSD with the btrfs-of-death.  Is it a
> multi-device SSD?
> 
> -chris
> 
Its a single device Intel X25-M G2 Postville 80GB

-Gerhard

Re: system crash at mounting of btrfs

[Chris Mason]

On Sat, Oct 09, 2010 at 01:08:40PM +0000, Gerhard Kulzer wrote:
> Gerhard Kulzer <gerhard <at> kulzer.net> writes:
> 
> > 
> > Chris Mason <chris.mason <at> oracle.com> writes:
> > 
> > > > [    7.881078] Btrfs detected SSD devices, enabling SSD mode
> > > > [    7.923553] ------------[ cut here ]------------
> > > > [    7.923556] kernel BUG at /build/buildd/linux-2.6.35/fs/btrfs
> > /tree-log.c:813!
> > > > [    7.923558] invalid opcode: 0000 [#1] SMP 
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > 
> > > Once you see kernel BUG() that's the crash ;)
> > > 
> > > This isn't from the btrfs scan, this is from mounting the FS.  Could you
> > > please mount the filesystems one at a time and see if they all fail or
> > > of it is just this one.
> > > 
> > > -chris
> > > --
> > Ahh big surprise, I can mount my other btrfs partitions (4 on 2 HDD) all of a
> > sudden. 
> > I wonder why, because I tried before and I got a crash with all of them. 
> > The only difference now is that I took the SSD with the 5th partition out.
> > I have a boot partition as ext4 on that same SSD drive which I can mount
> > w/o problems. So I think it's not the SSD itself that is buggy.
> > 
> 
> Ok, I understand a bit more now.
> a) when I boot my system from CD and the faulty SSD btrfs is physically present,
> then there's a little hick-up during boot resulting in my first trace and the
> system crashes if I subsequently mount ANY other btrfs partition.
> b) If I boot w/o the SSD, then there is no hick-up and I can mount all remaining
> btrfs partitions w/o crash and the data is there.

Ok, so tell me more about the SSD with the btrfs-of-death.  Is it a
multi-device SSD?

-chris

Re: system crash at mounting of btrfs

[Gerhard Kulzer]

Gerhard Kulzer <gerhard <at> kulzer.net> writes:

> 
> Chris Mason <chris.mason <at> oracle.com> writes:
> 
> > > [    7.881078] Btrfs detected SSD devices, enabling SSD mode
> > > [    7.923553] ------------[ cut here ]------------
> > > [    7.923556] kernel BUG at /build/buildd/linux-2.6.35/fs/btrfs
> /tree-log.c:813!
> > > [    7.923558] invalid opcode: 0000 [#1] SMP 
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > 
> > Once you see kernel BUG() that's the crash ;)
> > 
> > This isn't from the btrfs scan, this is from mounting the FS.  Could you
> > please mount the filesystems one at a time and see if they all fail or
> > of it is just this one.
> > 
> > -chris
> > --
> Ahh big surprise, I can mount my other btrfs partitions (4 on 2 HDD) all of a
> sudden. 
> I wonder why, because I tried before and I got a crash with all of them. 
> The only difference now is that I took the SSD with the 5th partition out.
> I have a boot partition as ext4 on that same SSD drive which I can mount
> w/o problems. So I think it's not the SSD itself that is buggy.
> 

Ok, I understand a bit more now.
a) when I boot my system from CD and the faulty SSD btrfs is physically present,
then there's a little hick-up during boot resulting in my first trace and the
system crashes if I subsequently mount ANY other btrfs partition.
b) If I boot w/o the SSD, then there is no hick-up and I can mount all remaining
btrfs partitions w/o crash and the data is there.

Gerhard

Re: system crash at mounting of btrfs

[Gerhard Kulzer]

Chris Mason <chris.mason <at> oracle.com> writes:

> > [    7.881078] Btrfs detected SSD devices, enabling SSD mode
> > [    7.923553] ------------[ cut here ]------------
> > [    7.923556] kernel BUG at /build/buildd/linux-2.6.35/fs/btrfs
/tree-log.c:813!
> > [    7.923558] invalid opcode: 0000 [#1] SMP 
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Once you see kernel BUG() that's the crash ;)
> 
> This isn't from the btrfs scan, this is from mounting the FS.  Could you
> please mount the filesystems one at a time and see if they all fail or
> of it is just this one.
> 
> -chris
> --
Ahh big surprise, I can mount my other btrfs partitions (4 on 2 HDD) all of a
sudden. 
I wonder why, because I tried before and I got a crash with all of them. 
The only difference now is that I took the SSD with the 5th partition out.
I have a boot partition as ext4 on that same SSD drive which I can mount
w/o problems. So I think it's not the SSD itself that is buggy.

Re: system crash at mounting of btrfs

[Chris Mason]

On Thu, Oct 07, 2010 at 02:12:33PM +0000, Gerhard Kulzer wrote:
> I have a weird problem concerning 5 btrfs partitions on 3 different disks:
> My system became slow and started to hang and go, so I shut it down and it 
> hang totally whilst shutting down. With the sysrq shortcut I could kill it 
> and restart.
> 
> Booting didn't work any more throwing me into a shell at initramfs.

Ok, I'd like to go through each of these btrfs partitions one at a time.

> Here is a dmesg trace during booting from the CD, it not the crash yet.
> 
> [    7.782823] Btrfs loaded
> [    7.786081] xor: automatically using best checksumming function: pIII_sse
> [    7.804000]    pIII_sse  : 11626.000 MB/sec
> [    7.804001] xor: using function: pIII_sse (11626.000 MB/sec)
> [    7.805746] device-mapper: dm-raid45: initialized v0.2594b
> [    7.851186] device fsid 3a4b4bc6c07de70b-2b32b3df70d2459f devid 1 transid
> 407064 /dev/sda2
> [    7.881078] Btrfs detected SSD devices, enabling SSD mode
> [    7.923553] ------------[ cut here ]------------
> [    7.923556] kernel BUG at /build/buildd/linux-2.6.35/fs/btrfs/tree-log.c:813!
> [    7.923558] invalid opcode: 0000 [#1] SMP 
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Once you see kernel BUG() that's the crash ;)

This isn't from the btrfs scan, this is from mounting the FS.  Could you
please mount the filesystems one at a time and see if they all fail or
of it is just this one.

-chris

Re: system crash at mounting of btrfs

[Gerhard Kulzer]

Chris Mason <chris.mason <at> oracle.com> writes:

> 
> So this is a completely different race from the first one.  Do you get
> the same crashes every time you try to boot or are they different every
> time?
> 
> We can definitely help get the data off.
> 
> -chris
> 
Thanks Chris,
the first trace was from dmesg during boot where it actually did not mount the
btrfs partition but just "saw" it during boot. The system continued running.

The last trace is the one that occurs when I try to mount the system, and it
dies with it. 

Re: system crash at mounting of btrfs

[Chris Mason]

So this is a completely different race from the first one.  Do you get
the same crashes every time you try to boot or are they different every
time?

We can definitely help get the data off.

-chris

On Fri, Oct 08, 2010 at 04:49:11PM +0000, Gerhard Kulzer wrote:
> Tomasz Chmielewski <mangoo <at> wpkg.org> writes:
> 
> > 
> > You could try:
> > 
> > http://kernel.ubuntu.com/~kernel-ppa/mainline/v2.6.36-rc7-maverick/
> > 
> 
> Alright I tried 2.6.36-rc7 to no avail, same crashing behaviour.
> 
> This is a kernel trace (from Intel-Atom netbook => 32bit) at which 
> the machine freezes after trying to mount the btrfs SSD drive:
> 
> ----trace---
> Pid: 1882, comm: mount Tainted: G D 2.6.36-020636rc7-generic
> EIP: 0060:[>f81039ef>] EFLAGS: 00010283 CPU:0
> EIP is at btrfs_test_super+0xf/0x30 [btrfs]
> EAX: 000000000 EBX: e2e6ca00 ECX: c0221090 EDX: f1689b80
> ESI: f81039e0 EDI: f816d318 EBP: d3001e78 ESP: d300115c
>  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 068
> Process mount (pid: 1882, ti=d3000000 task=ef54cc20 task.ti=d300000000)
> Stack:
>  c02221af c0221090 f816d300 00000000 fffffff3 00000000 f6eb7a00 d3001ee8
> <0> f8103e28 f1689b80 d3001ea8 d3001eb4 e62670c0 00000000 eee22b00 00000000
> <0> f816d300 00000000 000000d0 00000000 00000000 e0a18e00 f1689b80 00000004
> Call Trace:
>  [<c02221af>] ? sget+0x3f/0x1f0
>  [<c0221090>] ? set_anon_super+0x0/0xe0
>  [<f8103e28>] ? btrfs_get_sb+0x108/0x2c0 [btrfs]
>  [<c021671f>] ? __alloc_percpu+0xf/0x20
>  [<c0221f4e>] ? vfs_kern_mount+0x6e/0x1b0
>  [<c0235bb5>] ? get_fs_type+0x35/0xa0
>  [<c02220ee>] ? do_kern_mount+0x3e/0x80
>  [<c0238869>] ? do_new_mount+0x69/0xa0
>  [<c0238b48>] ? do_mount+0x188/0x1a0
>  [<c0238bcd>] ? sys_mount+0x6d/0xa0
>  [<c0102cdf>] ? sysenter_do_call+0x12/0x28
> Code: eb dc ba c0 02 00 00 b8 34 6d 16 f8 e8 1b ab 04 c8 eb d6 89 f6 
> 8d bc 27 00 00 00 00 55 89 e5 0f 1f 44 00 00 8b 80 7c 01 00 00 5d 
> <8b> 80 14 01 00 00 39 90 ec 1d 00 00 0f 94 c0 0f b6 c0 c3 8d b4
> EIP: [<f71039ef>] btrfs_test_super+0xf/0x30 [btrfs] SS:ESP 0068:d3001e5c
> CR2: 0000000000000114
> ---[ end trace ]---
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: system crash at mounting of btrfs

[Gerhard Kulzer]

Tomasz Chmielewski <mangoo <at> wpkg.org> writes:

> 
> You could try:
> 
> http://kernel.ubuntu.com/~kernel-ppa/mainline/v2.6.36-rc7-maverick/
> 

Alright I tried 2.6.36-rc7 to no avail, same crashing behaviour.

This is a kernel trace (from Intel-Atom netbook => 32bit) at which 
the machine freezes after trying to mount the btrfs SSD drive:

----trace---
Pid: 1882, comm: mount Tainted: G D 2.6.36-020636rc7-generic
EIP: 0060:[>f81039ef>] EFLAGS: 00010283 CPU:0
EIP is at btrfs_test_super+0xf/0x30 [btrfs]
EAX: 000000000 EBX: e2e6ca00 ECX: c0221090 EDX: f1689b80
ESI: f81039e0 EDI: f816d318 EBP: d3001e78 ESP: d300115c
 DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 068
Process mount (pid: 1882, ti=d3000000 task=ef54cc20 task.ti=d300000000)
Stack:
 c02221af c0221090 f816d300 00000000 fffffff3 00000000 f6eb7a00 d3001ee8
<0> f8103e28 f1689b80 d3001ea8 d3001eb4 e62670c0 00000000 eee22b00 00000000
<0> f816d300 00000000 000000d0 00000000 00000000 e0a18e00 f1689b80 00000004
Call Trace:
 [<c02221af>] ? sget+0x3f/0x1f0
 [<c0221090>] ? set_anon_super+0x0/0xe0
 [<f8103e28>] ? btrfs_get_sb+0x108/0x2c0 [btrfs]
 [<c021671f>] ? __alloc_percpu+0xf/0x20
 [<c0221f4e>] ? vfs_kern_mount+0x6e/0x1b0
 [<c0235bb5>] ? get_fs_type+0x35/0xa0
 [<c02220ee>] ? do_kern_mount+0x3e/0x80
 [<c0238869>] ? do_new_mount+0x69/0xa0
 [<c0238b48>] ? do_mount+0x188/0x1a0
 [<c0238bcd>] ? sys_mount+0x6d/0xa0
 [<c0102cdf>] ? sysenter_do_call+0x12/0x28
Code: eb dc ba c0 02 00 00 b8 34 6d 16 f8 e8 1b ab 04 c8 eb d6 89 f6 
8d bc 27 00 00 00 00 55 89 e5 0f 1f 44 00 00 8b 80 7c 01 00 00 5d 
<8b> 80 14 01 00 00 39 90 ec 1d 00 00 0f 94 c0 0f b6 c0 c3 8d b4
EIP: [<f71039ef>] btrfs_test_super+0xf/0x30 [btrfs] SS:ESP 0068:d3001e5c
CR2: 0000000000000114
---[ end trace ]---

Re: system crash at mounting of btrfs

[Tomasz Chmielewski]

> I tried it with 3 kernels from the 2.5.35 series.
> My system is a AMD64, so I put one SSD disk as an external drive (USB) to a
> laptop running a 32bit system, it crashed the same at my wanting to mount the
> partition.
>
> This is my config:
> $uname -a
> Linux ubuntu 2.6.35-22-generic #33-Ubuntu SMP Sun Sep 19 20:34:50 UTC 2010 i686
> GNU/Linux

You could try:

http://kernel.ubuntu.com/~kernel-ppa/mainline/v2.6.36-rc7-maverick/


-- 
Tomasz Chmielewski
http://wpkg.org

Re: system crash at mounting of btrfs

[Gerhard Kulzer]

I did an btrfs-image -c 9 dump on one partition, but it generated 142MB of data.
How can I send this?

system crash at mounting of btrfs

[Gerhard Kulzer]

I have a weird problem concerning 5 btrfs partitions on 3 different disks:
My system became slow and started to hang and go, so I shut it down and it 
hang totally whilst shutting down. With the sysrq shortcut I could kill it 
and restart.

Booting didn't work any more throwing me into a shell at initramfs.

I booted from a CD and tried to mount the first btrfs partition, the system hung
up after about 1 sec, just throwing "killed" to the shell.
I tried (after reboot) to mount the 4 other partitions which are on different
disks: the failure is always the same. I also tried mount options as "degraded",
"read-only" w/o success. But btrfsck works flawless, also btrfsctl -A, no
crashes. Any other partition on the same disk like ext4 mount perfectly.
I tried it with 3 kernels from the 2.5.35 series.
My system is a AMD64, so I put one SSD disk as an external drive (USB) to a
laptop running a 32bit system, it crashed the same at my wanting to mount the
partition.

This is my config:
$uname -a
Linux ubuntu 2.6.35-22-generic #33-Ubuntu SMP Sun Sep 19 20:34:50 UTC 2010 i686
GNU/Linux

Here is a dmesg trace during booting from the CD, it not the crash yet.

[    7.782823] Btrfs loaded
[    7.786081] xor: automatically using best checksumming function: pIII_sse
[    7.804000]    pIII_sse  : 11626.000 MB/sec
[    7.804001] xor: using function: pIII_sse (11626.000 MB/sec)
[    7.805746] device-mapper: dm-raid45: initialized v0.2594b
[    7.851186] device fsid 3a4b4bc6c07de70b-2b32b3df70d2459f devid 1 transid
407064 /dev/sda2
[    7.881078] Btrfs detected SSD devices, enabling SSD mode
[    7.923553] ------------[ cut here ]------------
[    7.923556] kernel BUG at /build/buildd/linux-2.6.35/fs/btrfs/tree-log.c:813!
[    7.923558] invalid opcode: 0000 [#1] SMP 
[    7.923561] last sysfs file: /sys/devices/virtual/bdi/btrfs-1/uevent
[    7.923562] Modules linked in: dm_raid45 xor btrfs zlib_deflate crc32c
libcrc32c nouveau ttm drm_kms_helper usbhid hid usb_storage drm sky2
firewire_ohci firewire_core intel_agp crc_itu_t ahci pata_jmicron libahci
agpgart i2c_algo_bit
[    7.923577] 
[    7.923579] Pid: 453, comm: exe Not tainted 2.6.35-22-generic #33-Ubuntu
P5K-E/P5K-E
[    7.923582] EIP: 0060:[<f9c44024>] EFLAGS: 00010246 CPU: 0
[    7.923590] EIP is at add_inode_ref+0x3f4/0x410 [btrfs]
[    7.923592] EAX: 00000000 EBX: 00000097 ECX: 00000000 EDX: 00000274
[    7.923594] ESI: 00000002 EDI: f6fe8af0 EBP: c1257c30 ESP: c1257bd4
[    7.923596]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[    7.923598] Process exe (pid: 453, ti=c1256000 task=f6a30000 task.ti=c1256000)
[    7.923599] Stack:
[    7.923600]  c1257be8 c1257be8 c0132ba6 f6fe8af0 00000011 c1257bf0 f9c2d2c1
c1257c30
[    7.923605] <0> f9c21d37 c1257c20 00000000 f6f9b000 f643887c 00000004
00000000 f6fe8af0
[    7.923610] <0> f68e6000 f7156000 fffb6000 fffb6000 00000097 00000002
f6fe8af0 c1257c94
[    7.923615] Call Trace:
[    7.923620]  [<c0132ba6>] ? kunmap_atomic+0x66/0x80
[    7.923628]  [<f9c2d2c1>] ? unmap_extent_buffer+0x11/0x20 [btrfs]
[    7.923637]  [<f9c21d37>] ? btrfs_item_size+0xc7/0xd0 [btrfs]
[    7.923644]  [<f9c45a46>] ? replay_one_buffer+0x246/0x320 [btrfs]
[    7.923651]  [<f9c423a9>] ? walk_down_log_tree+0x219/0x3b0 [btrfs]
[    7.923658]  [<f9c425e9>] ? walk_log_tree+0xa9/0x1d0 [btrfs]
[    7.923665]  [<f9c44d94>] ? btrfs_recover_log_trees+0x1d4/0x2b0 [btrfs]
[    7.923672]  [<f9c45800>] ? replay_one_buffer+0x0/0x320 [btrfs]
[    7.923680]  [<f9c0d07c>] ? open_ctree+0x101c/0x14c0 [btrfs]
[    7.923686]  [<f9bee672>] ? btrfs_fill_super+0x52/0x110 [btrfs]
[    7.923690]  [<c0356069>] ? strlcpy+0x39/0x50
[    7.923695]  [<f9beebbd>] ? btrfs_get_sb+0x24d/0x2d0 [btrfs]
[    7.923699]  [<c020f66f>] ? __alloc_percpu+0xf/0x20
[    7.923701]  [<c0231109>] ? alloc_vfsmnt+0xf9/0x130
[    7.923704]  [<c021aee4>] ? vfs_kern_mount+0x74/0x1c0
[    7.923707]  [<c022f493>] ? get_fs_type+0x33/0xb0
[    7.923709]  [<c021b08e>] ? do_kern_mount+0x3e/0xe0
[    7.923711]  [<c023260c>] ? do_mount+0x1dc/0x220
[    7.923714]  [<c02326bb>] ? sys_mount+0x6b/0xa0
[    7.923717]  [<c05c90a4>] ? syscall_call+0x7/0xb
[    7.923718] Code: e8 42 b3 fa ff 8b 45 d4 e8 5a 87 5e c6 8b 45 cc e8 52 87 5e
c6 31 c0 83 c4 50 5b 5e 5f 5d c3 0f 0b eb fe 0f 0b eb fe 0f 0b eb fe <0f> 0b eb
fe 0f 0b eb fe 0f 0b eb fe 0f 0b eb fe 8d b6 00 00 00 
[    7.923746] EIP: [<f9c44024>] add_inode_ref+0x3f4/0x410 [btrfs] SS:ESP
0068:c1257bd4
[    7.923755] ---[ end trace 2b634c981d89a441 ]---

Any help how to get my data off those disk is seriously appreciated.