Ocfs2 devel - Dec 2009 - [PATCH 0/2] 2 bug fixes for reflink in ocfs2.

Hi Joel,
	These are 2 bug fixes for reflink.
	Although these 2 are both one line fix, they are important.
[PATCH 1/2] resolves a bug tristan found serveral months ago. I tracked 
it down recently. It is really race.
[PATCH 2/2] is found when I tested a corruption in reflink. It resolved 
a bug that the file can't be replayed successfully in orphan dir.

I have run tristan's test cases against these 2 fixes the whole week. 
They should be stable enough to be integrated.

Regards,
Tao

We used to add reflinked file's inode to inode hash when
we add it to the dest dir. But actually there is a race.
Consider the following sequence.
1. reflink happens and create the inode in orphan dir.
2. reflink thread is scheduled out because of some io.
3. recovery begins to work and calls ocfs2_recover_orphans.
   It calls ocfs2_iget and get a new inode and i_count = 1.
   It calls iput then and delete inode. the buffer's
   uptodate state is cleared.

This patch move insert_inode_hash to the create function so
that it can be found by step 3 and prevented from deleting
because i_count > 1.

This resolves the bug
http://oss.oracle.com/bugzilla/show_bug.cgi?id=1183.

Signed-off-by: Tao Ma <tao.ma at oracle.com>
---
 fs/ocfs2/namei.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index f010b22..5ac4d52 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -2136,6 +2136,7 @@ int ocfs2_create_inode_in_orphan(struct inode *dir,
 	if (status < 0)
 		mlog_errno(status);
 
+	insert_inode_hash(inode);
 leave:
 	if (status < 0 && did_quota_inode)
 		vfs_dq_free_inode(inode);
@@ -2284,7 +2285,6 @@ int ocfs2_mv_orphaned_inode_to_new(struct inode *dir,
 		goto out_commit;
 	}
 
-	insert_inode_hash(inode);
 	dentry->d_op = &ocfs2_dentry_ops;
 	d_instantiate(dentry, inode);
 	status = 0;
-- 
1.5.4.4

We create a file in orphan dir for reflink so that if there
is any error, we don't create any wrong dentry in the dir.
But actually the file in orphan dir should be i_nlink = 0
so that it can be replayed and freed successfully.

This patch first set i_nlink to 0 when creating the file in
orphan dir and then set it to 1(reflink now only works for
regular file) when we move it to the dest dir.

Signed-off-by: Tao Ma <tao.ma at oracle.com>
---
 fs/ocfs2/namei.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 5ac4d52..3e9b460 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -2108,6 +2108,7 @@ int ocfs2_create_inode_in_orphan(struct inode *dir,
 	}
 	did_quota_inode = 1;
 
+	inode->i_nlink = 0;
 	/* do the real work now. */
 	status = ocfs2_mknod_locked(osb, dir, inode,
 				    0, &new_di_bh, parent_di_bh, handle,
@@ -2268,6 +2269,8 @@ int ocfs2_mv_orphaned_inode_to_new(struct inode *dir,
 	di = (struct ocfs2_dinode *)di_bh->b_data;
 	le32_add_cpu(&di->i_flags, -OCFS2_ORPHANED_FL);
 	di->i_orphaned_slot = 0;
+	inode->i_nlink = 1;
+	ocfs2_set_links_count(di, inode->i_nlink);
 	ocfs2_journal_dirty(handle, di_bh);
 
 	status = ocfs2_add_entry(handle, dentry, inode,
-- 
1.5.4.4

On Fri, Dec 18, 2009 at 10:10:16AM +0800, Tao Ma wrote:
> 	These are 2 bug fixes for reflink.
> 	Although these 2 are both one line fix, they are important.
> [PATCH 1/2] resolves a bug tristan found serveral months ago. I
> tracked it down recently. It is really race.
> [PATCH 2/2] is found when I tested a corruption in reflink. It
> resolved a bug that the file can't be replayed successfully in
> orphan dir.
> 
> I have run tristan's test cases against these 2 fixes the whole
> week. They should be stable enough to be integrated.
	They are both in the 'fixes' branch of ocfs2.git.

Joel

-- 

"Depend on the rabbit's foot if you will, but remember, it didn't
 help the rabbit."
	- R. E. Shay

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127