[Ocfs2-devel] [PATCH 0/2] ocfs2: Some patches for reflink. v3

[Ocfs2-devel] [PATCH 0/2] ocfs2: Some patches for reflink. v3

[Tao Ma]

Hi Joel,
	This patch set integrate the fix for the bug tristan found.

Regards,
Tao

[Ocfs2-devel] [PATCH 1/2] ocfs2: Move ocfs2_complete_reflink to the right place.

[Tao Ma]

As its name ocfs2_complete_reflink indicates, it should
be called after all the work for reflink is done, so
it really should be called after we reflink xattr
successfully.

Signed-off-by: Tao Ma <tao.ma@oracle.com>
---
 fs/ocfs2/refcounttree.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
index 60287fc..9d439b2 100644
--- a/fs/ocfs2/refcounttree.c
+++ b/fs/ocfs2/refcounttree.c
@@ -4013,10 +4013,6 @@ static int ocfs2_create_reflink_node(struct inode *s_inode,
 		goto out_unlock_refcount;
 	}
 
-	ret = ocfs2_complete_reflink(s_inode, s_bh, t_inode, t_bh, preserve);
-	if (ret)
-		mlog_errno(ret);
-
 out_unlock_refcount:
 	ocfs2_unlock_refcount_tree(osb, ref_tree, 1);
 	brelse(ref_root_bh);
@@ -4068,9 +4064,17 @@ static int __ocfs2_reflink(struct dentry *old_dentry,
 		ret = ocfs2_reflink_xattrs(inode, old_bh,
 					   new_inode, new_bh,
 					   preserve);
-		if (ret)
+		if (ret) {
 			mlog_errno(ret);
+			goto inode_unlock;
+		}
 	}
+
+	ret = ocfs2_complete_reflink(inode, old_bh,
+				     new_inode, new_bh, preserve);
+	if (ret)
+		mlog_errno(ret);
+
 inode_unlock:
 	ocfs2_inode_unlock(new_inode, 1);
 	brelse(new_bh);
-- 
1.6.2.rc2.16.gf474c

[Ocfs2-devel] [PATCH 2/2] ocfs2: duplicate inline data properly during reflink.

[Tao Ma]

The old reflink fails to handle inline-data and can cause kernel
oops. So this patch fix it. It will try to copy the whole inline
content to the new inode and set the corresponding feature flag.

Signed-off-by: Tao Ma <tao.ma@oracle.com>
---
 fs/ocfs2/file.c         |    3 +-
 fs/ocfs2/refcounttree.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 1 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 89fc8ee..de059f4 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1712,7 +1712,8 @@ int ocfs2_check_range_for_refcount(struct inode *inode, loff_t pos,
 	struct super_block *sb = inode->i_sb;
 
 	if (!ocfs2_refcount_tree(OCFS2_SB(inode->i_sb)) ||
-	    !(OCFS2_I(inode)->ip_dyn_features & OCFS2_HAS_REFCOUNT_FL))
+	    !(OCFS2_I(inode)->ip_dyn_features & OCFS2_HAS_REFCOUNT_FL) ||
+	    OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL)
 		return 0;
 
 	cpos = pos >> OCFS2_SB(sb)->s_clustersize_bits;
diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
index 9d439b2..3a0df7a 100644
--- a/fs/ocfs2/refcounttree.c
+++ b/fs/ocfs2/refcounttree.c
@@ -3743,6 +3743,9 @@ static int ocfs2_attach_refcount_tree(struct inode *inode,
 		goto out;
 	}
 
+	if (oi->ip_dyn_features & OCFS2_INLINE_DATA_FL)
+		goto attach_xattr;
+
 	ocfs2_init_dinode_extent_tree(&di_et, INODE_CACHE(inode), di_bh);
 
 	size = i_size_read(inode);
@@ -3769,6 +3772,7 @@ static int ocfs2_attach_refcount_tree(struct inode *inode,
 		cpos += num_clusters;
 	}
 
+attach_xattr:
 	if (oi->ip_dyn_features & OCFS2_HAS_XATTR_FL) {
 		ret = ocfs2_xattr_attach_refcount_tree(inode, di_bh,
 						       &ref_tree->rf_ci,
@@ -3858,6 +3862,49 @@ out:
 	return ret;
 }
 
+static int ocfs2_duplicate_inline_data(struct inode *s_inode,
+				       struct buffer_head *s_bh,
+				       struct inode *t_inode,
+				       struct buffer_head *t_bh)
+{
+	int ret;
+	handle_t *handle;
+	struct ocfs2_super *osb = OCFS2_SB(s_inode->i_sb);
+	struct ocfs2_dinode *s_di = (struct ocfs2_dinode *)s_bh->b_data;
+	struct ocfs2_dinode *t_di = (struct ocfs2_dinode *)t_bh->b_data;
+
+	BUG_ON(!(OCFS2_I(s_inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL));
+
+	handle = ocfs2_start_trans(osb, OCFS2_INODE_UPDATE_CREDITS);
+	if (IS_ERR(handle)) {
+		ret = PTR_ERR(handle);
+		mlog_errno(ret);
+		goto out;
+	}
+
+	ret = ocfs2_journal_access_di(handle, INODE_CACHE(t_inode), t_bh,
+				      OCFS2_JOURNAL_ACCESS_WRITE);
+	if (ret) {
+		mlog_errno(ret);
+		goto out_commit;
+	}
+
+	t_di->id2.i_data.id_count = s_di->id2.i_data.id_count;
+	memcpy(t_di->id2.i_data.id_data, s_di->id2.i_data.id_data,
+	       le16_to_cpu(s_di->id2.i_data.id_count));
+	spin_lock(&OCFS2_I(t_inode)->ip_lock);
+	OCFS2_I(t_inode)->ip_dyn_features |= OCFS2_INLINE_DATA_FL;
+	t_di->i_dyn_features = cpu_to_le16(OCFS2_I(t_inode)->ip_dyn_features);
+	spin_unlock(&OCFS2_I(t_inode)->ip_lock);
+
+	ocfs2_journal_dirty(handle, t_bh);
+
+out_commit:
+	ocfs2_commit_trans(osb, handle);
+out:
+	return ret;
+}
+
 static int ocfs2_duplicate_extent_list(struct inode *s_inode,
 				struct inode *t_inode,
 				struct buffer_head *t_bh,
@@ -3997,6 +4044,14 @@ static int ocfs2_create_reflink_node(struct inode *s_inode,
 		goto out;
 	}
 
+	if (OCFS2_I(s_inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) {
+		ret = ocfs2_duplicate_inline_data(s_inode, s_bh,
+						  t_inode, t_bh);
+		if (ret)
+			mlog_errno(ret);
+		goto out;
+	}
+
 	ret = ocfs2_lock_refcount_tree(osb, le64_to_cpu(di->i_refcount_loc),
 				       1, &ref_tree, &ref_root_bh);
 	if (ret) {
-- 
1.6.2.rc2.16.gf474c

[Ocfs2-devel] [PATCH 0/2] ocfs2: Some patches for reflink. v3

[Joel Becker]

On Thu, Oct 15, 2009 at 11:06:53AM +0800, Tao Ma wrote:
> 	This patch set integrate the fix for the bug tristan found.

	These both look good.  You know, if an inode is inline and has
no xattrs, ie:

(oi->i_dyn_flags & (OCFS2_HAS_XATTR_FL | OCFS2_HAS_INLINE_DATA_FL)) ==
  OCFS2_HAS_INLINE_DATA_FL

you can just skip the refcount tree entirely.  Copy the inode.  Reinit
the security if !preserve.  Call it a day.
	But this is a later optimization.

Joel

-- 

Life's Little Instruction Book #450

	"Don't be afraid to say, 'I need help.'"

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

[Ocfs2-devel] [PATCH 0/2] ocfs2: Some patches for reflink. v3

[TaoMa]

Joel Becker wrote:
> On Thu, Oct 15, 2009 at 11:06:53AM +0800, Tao Ma wrote:
>   
>> 	This patch set integrate the fix for the bug tristan found.
>>     
>
> 	These both look good.  You know, if an inode is inline and has
> no xattrs, ie:
>
> (oi->i_dyn_flags & (OCFS2_HAS_XATTR_FL | OCFS2_HAS_INLINE_DATA_FL)) ==
>   OCFS2_HAS_INLINE_DATA_FL
>
> you can just skip the refcount tree entirely.  Copy the inode.  Reinit
> the security if !preserve.  Call it a day.
>   
yeah, that is also one of the reason I moved complete_reflink out of 
create_reflink_node and insert it into __ocfs2_reflink(patch 1). So if 
we detect the situation you mentioned above, we will just copy the inode 
and then go directly to ocfs2_complete_reflink, both create_reflink_node 
and ocfs2_reflink_xattrs will be skipped. That would be quite easy.
> 	But this is a later optimization.
>   
agree.

Regards,
Tao

[Ocfs2-devel] [PATCH 0/2] ocfs2: Some patches for reflink. v3

[tristan.ye]

Tao Ma Wrote:
> Hi Joel,
> 	This patch set integrate the fix for the bug tristan found.
>   

Tested-by Tristan.

bug 1185 got fixed with this patch series.

> Regards,
> Tao
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
>   

[Ocfs2-devel] [PATCH 0/2] ocfs2: Some patches for reflink. v3

[Joel Becker]

On Fri, Oct 16, 2009 at 10:22:10AM +0800, tristan.ye wrote:
> Tao Ma Wrote:
> >Hi Joel,
> >	This patch set integrate the fix for the bug tristan found.
> 
> Tested-by Tristan.

Tristan,
	Can you put full Tested-by/Reviewed-by/etc lines in future
emails?  That way I can cut and paste them as I take patches for the
tree.
	For example, I'm putting these patches in ocfs2-fixes with the
line:

  Tested-by: Tristan Ye <tristan.ye@oracle.com>

Thanks!
Joel

-- 

"The one important thing i have learned over the years is the
 difference between taking one's work seriously and taking one's self
 seriously.  The first is imperative and the second is disastrous."
	-Margot Fonteyn

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

[Ocfs2-devel] [PATCH 0/2] ocfs2: Some patches for reflink. v3

[Joel Becker]

On Thu, Oct 15, 2009 at 11:06:53AM +0800, Tao Ma wrote:
> Hi Joel,
> 	This patch set integrate the fix for the bug tristan found.

	These patches are now part of the fixes branch of ocfs2.git.

Joel

-- 

None of our men are "experts."  We have most unfortunately found
it necessary to get rid of a man as soon as he thinks himself an
expert -- because no one ever considers himself expert if he really
knows his job.  A man who knows a job sees so much more to be done
than he has done, that he is always pressing forward and never
gives up an instant of thought to how good and how efficient he is.
Thinking always ahead, thinking always of trying to do more, brings
a state of mind in which nothing is impossible. The moment one gets
into the "expert" state of mind a great number of things become
impossible.
	- From Henry Ford Sr., "My Life and Work"

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