Btrfs devel - Sep 2011 - Error on creating snapshots (btrfs: could not do orphan cleanup -116)

This happens on a freshly created btrfs filesystem in a raid10 (4x1TB)
configuration with three
subvolumes and 1.5 TB data.

When I try to snapshot one of the subvolumes (with 100 GB of data), it
says that the snapshot
creation failed and I get the following error message:

btrfs: could not do orphan cleanup -116

After the failure:
- The snapshot exists in `btrfs subvolume list''
- The snapshot CAN''T be accessed from the mounted subvolume where I
created the
snapshot (but the directory entry exists and displays non-sense when
you try to do `ls -la'')
- The snapshot CAN be accessed from any other mount of the same subvolume
- The snapshot CAN be accessed after unmount/remount

This was also coupled with a warning in dmesg that was fixed with
slyich''s patch from
`[PATCH] btrfs: fix warning in iput for bad-inode.''

I did a little bit of research on the problem, and while I''m too
unfamiliar with the code of btrfs to diagnose it,
I''d like to share some of the observations as they appear to be of
importance:

The failure happens when btrfs_orphan_cleanup calls btrfs_iget to get
the inode and it returns
ERR_PTR(-ESTALE). This happens when is_bad_inode(inode) is true.
However, after the call
to btrfs_iget there is an explicit code path for the case when
is_bad_inode(inode) is true (line 2393).
This code path would *never ever* get executed unless something
happening in another thread can
affect the return value between the two calls. I believe that lines
2394-2402 would never be executed.

What''s more I discovered the following patch:

[PATCH 5/6] btrfs: Add per subvolume cached inode tree #2
http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg02435.html

The patch changes the behaviour of btrfs_orphan_cleanup -- without it
there is no code that would lead
to ERR_PTR(-ESTALE) and the bad inode code path will run. With it,
suddenly this code path isn''t called
and instead an error is produced.

I think I should try and reverse this specific change in the following
days and see if it fixes the problem, but
I''m not confident enough to blindly change the code without asking for
comments first, so any comment is
appreciated.

Thank you.
--
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

When btrfs_orphan_cleanup encounters an inode marked as bad, instead of going
down the code path that handles such inodes, it fails with the following error
message:

     btrfs: could not do orphan cleanup -116

This happens as the called btrfs_iget returns an error when encountering such
inode.

This patch attempts to correct this issue by calling btrfs_iget_locked
instead, and handling inodes in I_NEW state manually, while leaving bad inodes
to the handling that''s already present in the function.

Signed-off-by: Milko Krachounov <milko@3mhz.net>
---
diff -pru linux-2.6-3.1.0~rc4.original/fs/btrfs/inode.c
linux-2.6-3.1.0~rc4.fixed/fs/btrfs/inode.c
--- linux-2.6-3.1.0~rc4.original/fs/btrfs/inode.c	2011-08-29 07:16:01.000000000
+0300
+++ linux-2.6-3.1.0~rc4.fixed/fs/btrfs/inode.c	2011-09-06 19:26:58.490416217
+0300
@@ -94,6 +94,12 @@ static noinline int cow_file_range(struc
 				   u64 start, u64 end, int *page_started,
 				   unsigned long *nr_written, int unlock);
 
+static struct inode *btrfs_iget_locked(struct super_block *s,
+                                       u64 objectid, 
+                                       struct btrfs_root *root);
+static void btrfs_read_locked_inode(struct inode *inode);
+static void inode_tree_add(struct inode *inode);
+
 static int btrfs_init_inode_security(struct btrfs_trans_handle *trans,
 				     struct inode *inode,  struct inode *dir,
 				     const struct qstr *qstr)
@@ -2367,15 +2373,27 @@ int btrfs_orphan_cleanup(struct btrfs_ro
 		 * crossing root thing.  we store the inode number in the
 		 * offset of the orphan item.
 		 */
-		found_key.objectid = found_key.offset;
-		found_key.type = BTRFS_INODE_ITEM_KEY;
-		found_key.offset = 0;
-		inode = btrfs_iget(root->fs_info->sb, &found_key, root, NULL);
-		if (IS_ERR(inode)) {
-			ret = PTR_ERR(inode);
+		inode = btrfs_iget_locked(root->fs_info->sb,
+		                          found_key.offset, root);
+		if (!inode) {
+			ret = -ENOMEM;
 			goto out;
 		}
 
+		if (inode->i_state & I_NEW) {
+			BTRFS_I(inode)->root = root;
+			BTRFS_I(inode)->location.objectid = found_key.offset;
+			BTRFS_I(inode)->location.type = BTRFS_INODE_ITEM_KEY;
+			BTRFS_I(inode)->location.offset = 0;
+
+			btrfs_read_locked_inode(inode);
+
+			if (!is_bad_inode(inode))
+				inode_tree_add(inode);
+
+			unlock_new_inode(inode);
+		}
+
 		/*
 		 * add this inode to the orphan list so btrfs_orphan_del does
 		 * the proper thing when we hit it
--
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