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

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

[Tao Ma]

Hi Joel,
	These 2 patches are the right ones I want to send you. The detail can 
be found in the commit log.

Regards,
Tao

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

[Tao Ma]

As its name ocfs2_complete_reflink indicates, it should be called
after all the work for reflink is done, so move it after
ocfs2_reflink_xattrs.

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

[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/refcounttree.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)

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 2/2] ocfs2: duplicate inline data properly during reflink.v2

[Tristan]

Tao,

I applied this patch series to allow reflink on inlined files on linus's 
MM branch.

Now it's able to do reflinks on inlined file with patches, but the bad 
thing is, a write operation on original file or reflink target always 
failed at following error after reflinks have been done:

write error 34: "Numerical result out of range"

Error msg from dmesg:

(8531,1):ocfs2_get_clusters:618 ERROR: status = -34
(8531,1):ocfs2_check_range_for_refcount:1725 ERROR: status = -34
(8531,1):ocfs2_prepare_inode_for_write:1838 ERROR: status = -34
(8531,1):ocfs2_file_aio_write:1946 ERROR: status = -34

For more detailed info, please refer to bug:
http://oss.oracle.com/bugzilla/show_bug.cgi?id=1186

Thanks,
Tristan.


Tao Ma wrote:
> 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/refcounttree.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 55 insertions(+), 0 deletions(-)
>
> 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) {
>   

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

[Joel Becker]

On Mon, Oct 12, 2009 at 04:01:02PM +0800, Tao Ma wrote:
> Hi Joel,
> 	These 2 patches are the right ones I want to send you. The detail
> can be found in the commit log.

	These patches look good, but I'm going to wait until you have a
new set that fixes the bug Tristan found.

Joel

-- 

"Can any of you seriously say the Bill of Rights could get through
 Congress today?  It wouldn't even get out of committee."
	- F. Lee Bailey

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