[Ocfs2-devel] [PATCH 1/1] Ocfs2: Re-access the journal after ocfs2_insert_extent() in dxdir codes.

[Ocfs2-devel] [PATCH 1/1] Ocfs2: Re-access the journal after ocfs2_insert_extent() in dxdir codes.

[Tristan Ye]

In ocfs2_dx_dir_rebalance(), we need to rejournal_acess the blocks after
calling ocfs2_insert_extent() since growing an extent tree may trigger
ocfs2_extend_trans(), which makes previous journal_access meaningless.

Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
---
 fs/ocfs2/dir.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index f04ebcf..c49f6de 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -3931,6 +3931,15 @@ static int ocfs2_dx_dir_rebalance(struct ocfs2_super *osb, struct inode *dir,
 		goto out_commit;
 	}
 
+	cpos = split_hash;
+	ret = ocfs2_dx_dir_new_cluster(dir, &et, cpos, handle,
+				       data_ac, meta_ac, new_dx_leaves,
+				       num_dx_leaves);
+	if (ret) {
+		mlog_errno(ret);
+		goto out_commit;
+	}
+
 	for (i = 0; i < num_dx_leaves; i++) {
 		ret = ocfs2_journal_access_dl(handle, INODE_CACHE(dir),
 					      orig_dx_leaves[i],
@@ -3939,15 +3948,14 @@ static int ocfs2_dx_dir_rebalance(struct ocfs2_super *osb, struct inode *dir,
 			mlog_errno(ret);
 			goto out_commit;
 		}
-	}
 
-	cpos = split_hash;
-	ret = ocfs2_dx_dir_new_cluster(dir, &et, cpos, handle,
-				       data_ac, meta_ac, new_dx_leaves,
-				       num_dx_leaves);
-	if (ret) {
-		mlog_errno(ret);
-		goto out_commit;
+		ret = ocfs2_journal_access_dl(handle, INODE_CACHE(dir),
+					      new_dx_leaves[i],
+					      OCFS2_JOURNAL_ACCESS_WRITE);
+		if (ret) {
+			mlog_errno(ret);
+			goto out_commit;
+		}
 	}
 
 	ocfs2_dx_dir_transfer_leaf(dir, split_hash, handle, tmp_dx_leaf,
-- 
1.5.5

[Ocfs2-devel] [PATCH 1/1] Ocfs2: Re-access the journal after ocfs2_insert_extent() in dxdir codes.

[Mark Fasheh]

On Wed, Sep 08, 2010 at 05:12:38PM +0800, Tristan Ye wrote:
> In ocfs2_dx_dir_rebalance(), we need to rejournal_acess the blocks after
> calling ocfs2_insert_extent() since growing an extent tree may trigger
> ocfs2_extend_trans(), which makes previous journal_access meaningless.
> 
> Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
> ---
>  fs/ocfs2/dir.c |   24 ++++++++++++++++--------
>  1 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index f04ebcf..c49f6de 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -3931,6 +3931,15 @@ static int ocfs2_dx_dir_rebalance(struct ocfs2_super *osb, struct inode *dir,
>  		goto out_commit;
>  	}
>  
> +	cpos = split_hash;
> +	ret = ocfs2_dx_dir_new_cluster(dir, &et, cpos, handle,
> +				       data_ac, meta_ac, new_dx_leaves,
> +				       num_dx_leaves);
> +	if (ret) {
> +		mlog_errno(ret);
> +		goto out_commit;
> +	}
> +
>  	for (i = 0; i < num_dx_leaves; i++) {
>  		ret = ocfs2_journal_access_dl(handle, INODE_CACHE(dir),
>  					      orig_dx_leaves[i],

Ok, I see what you're going for here. Don't we still need to add a
journal_access_dl call against new_dx_leaves[i] in the for loop though? They
will be dirtied again in ocfs2_dx_dir_transfer_leaf.
	--Mark

--
Mark Fasheh

[Ocfs2-devel] [PATCH 1/1] Ocfs2: Re-access the journal after ocfs2_insert_extent() in dxdir codes.

[tristan]

Mark Fasheh wrote:
> On Wed, Sep 08, 2010 at 05:12:38PM +0800, Tristan Ye wrote:
>> In ocfs2_dx_dir_rebalance(), we need to rejournal_acess the blocks after
>> calling ocfs2_insert_extent() since growing an extent tree may trigger
>> ocfs2_extend_trans(), which makes previous journal_access meaningless.
>>
>> Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
>> ---
>>  fs/ocfs2/dir.c |   24 ++++++++++++++++--------
>>  1 files changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
>> index f04ebcf..c49f6de 100644
>> --- a/fs/ocfs2/dir.c
>> +++ b/fs/ocfs2/dir.c
>> @@ -3931,6 +3931,15 @@ static int ocfs2_dx_dir_rebalance(struct ocfs2_super *osb, struct inode *dir,
>>  		goto out_commit;
>>  	}
>>  
>> +	cpos = split_hash;
>> +	ret = ocfs2_dx_dir_new_cluster(dir, &et, cpos, handle,
>> +				       data_ac, meta_ac, new_dx_leaves,
>> +				       num_dx_leaves);
>> +	if (ret) {
>> +		mlog_errno(ret);
>> +		goto out_commit;
>> +	}
>> +
>>  	for (i = 0; i < num_dx_leaves; i++) {
>>  		ret = ocfs2_journal_access_dl(handle, INODE_CACHE(dir),
>>  					      orig_dx_leaves[i],
>
> Ok, I see what you're going for here. Don't we still need to add a
> journal_access_dl call against new_dx_leaves[i] in the for loop though? They
> will be dirtied again in ocfs2_dx_dir_transfer_leaf.

Didn't I?

This patch already had the codes calling the journal_access_dl against 
new_dx_leaves.

Look at the second portion of this patch, that is it.



> 	--Mark
>
> --
> Mark Fasheh

[Ocfs2-devel] [PATCH 1/1] Ocfs2: Re-access the journal after ocfs2_insert_extent() in dxdir codes.

[Mark Fasheh]

On Thu, Sep 09, 2010 at 09:35:20AM +0800, tristan wrote:
> Mark Fasheh wrote:
> > On Wed, Sep 08, 2010 at 05:12:38PM +0800, Tristan Ye wrote:
> >> In ocfs2_dx_dir_rebalance(), we need to rejournal_acess the blocks after
> >> calling ocfs2_insert_extent() since growing an extent tree may trigger
> >> ocfs2_extend_trans(), which makes previous journal_access meaningless.
> >>
> >> Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
> >> ---
> >>  fs/ocfs2/dir.c |   24 ++++++++++++++++--------
> >>  1 files changed, 16 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> >> index f04ebcf..c49f6de 100644
> >> --- a/fs/ocfs2/dir.c
> >> +++ b/fs/ocfs2/dir.c
> >> @@ -3931,6 +3931,15 @@ static int ocfs2_dx_dir_rebalance(struct ocfs2_super *osb, struct inode *dir,
> >>  		goto out_commit;
> >>  	}
> >>  
> >> +	cpos = split_hash;
> >> +	ret = ocfs2_dx_dir_new_cluster(dir, &et, cpos, handle,
> >> +				       data_ac, meta_ac, new_dx_leaves,
> >> +				       num_dx_leaves);
> >> +	if (ret) {
> >> +		mlog_errno(ret);
> >> +		goto out_commit;
> >> +	}
> >> +
> >>  	for (i = 0; i < num_dx_leaves; i++) {
> >>  		ret = ocfs2_journal_access_dl(handle, INODE_CACHE(dir),
> >>  					      orig_dx_leaves[i],
> >
> > Ok, I see what you're going for here. Don't we still need to add a
> > journal_access_dl call against new_dx_leaves[i] in the for loop though? They
> > will be dirtied again in ocfs2_dx_dir_transfer_leaf.
> 
> Didn't I?
> 
> This patch already had the codes calling the journal_access_dl against 
> new_dx_leaves.
> 
> Look at the second portion of this patch, that is it.

Ahh yes, you are correct. Sorry about that, not sure how I missed it.
Anyway, you can add my ack:

Acked-by: Mark Fasheh <mfasheh@suse.com>

The patch looks good to me.
	--Mark 


--
Mark Fasheh

[Ocfs2-devel] [PATCH 1/1] Ocfs2: Re-access the journal after ocfs2_insert_extent() in dxdir codes.

[Joel Becker]

On Wed, Sep 08, 2010 at 05:12:38PM +0800, Tristan Ye wrote:
> In ocfs2_dx_dir_rebalance(), we need to rejournal_acess the blocks after
> calling ocfs2_insert_extent() since growing an extent tree may trigger
> ocfs2_extend_trans(), which makes previous journal_access meaningless.
> 
> Signed-off-by: Tristan Ye <tristan.ye@oracle.com>

	This patch is now in the fixes branch of ocfs2.git.

Joel

-- 

Life's Little Instruction Book #173

	"Be kinder than necessary."

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