Skip to contentSkip to conversation

ocfs2: Commit transactions in error cases.

PATCH New Wengang Wang 2 participants 3 messages Sep 25, 2011 – Oct 12, 2011 OCFS2

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ocfs2: Commit transactions in error cases.

Commit transactions in error cases.

There are three cases found that in error cases, journal transactions are not committed nor aborted. We should take care of these case by committing the transactions. Otherwise, there would left a journal handle which will lead to , in same process context, the comming ocfs2_start_trans() gets wrong credits.

---
 fs/ocfs2/alloc.c |    2 +-
 fs/ocfs2/ioctl.c |    3 ++-
 fs/ocfs2/xattr.c |    2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)
fs/ocfs2/alloc.c +1 -1
index ed553c6..3165aeb 100644
@@ -5699,7 +5699,7 @@ int ocfs2_remove_btree_range(struct inode *inode,
 					   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
-		goto out;
+		goto out_commit;
 	}
 
 	dquot_free_space_nodirty(inode,
fs/ocfs2/ioctl.c +2 -1
index bc91072..5612c3d 100644
@@ -122,7 +122,7 @@ static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags,
 	if ((oldflags & OCFS2_IMMUTABLE_FL) || ((flags ^ oldflags) &
 		(OCFS2_APPEND_FL | OCFS2_IMMUTABLE_FL))) {
 		if (!capable(CAP_LINUX_IMMUTABLE))
-			goto bail_unlock;
+			goto bail_commit;
 	}
 
 	ocfs2_inode->ip_attr = flags;
@@ -132,6 +132,7 @@ static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags,
 	if (status < 0)
 		mlog_errno(status);
 
+bail_commit:
 	ocfs2_commit_trans(osb, handle);
 bail_unlock:
 	ocfs2_inode_unlock(inode, 1);
fs/ocfs2/xattr.c +1 -1
index 81ecf9c..6eb01c5 100644
@@ -2376,12 +2376,12 @@ static int ocfs2_remove_value_outside(struct inode*inode,
 		}
 
 		ret = ocfs2_xattr_value_truncate(inode, vb, 0, &ctxt);
+		ocfs2_commit_trans(osb, ctxt.handle);
 		if (ret < 0) {
 			mlog_errno(ret);
 			break;
 		}
 
-		ocfs2_commit_trans(osb, ctxt.handle);
 		if (ctxt.meta_ac) {
 			ocfs2_free_alloc_context(ctxt.meta_ac);
 			ctxt.meta_ac = NULL;
Signature · 1 line
1.7.5.2
#1 lore raw

PATCH 1/1 ocfs2: Commit transactions in error cases.

The first two are ok. Have a comment for the last one.

Quoted 56 lines · Wengang Wang

Commit transactions in error cases.

There are three cases found that in error cases, journal transactions are not committed nor aborted. We should take care of these case by committing the transactions. Otherwise, there would left a journal handle which will lead to , in same process context, the comming ocfs2_start_trans() gets wrong credits.

---
  fs/ocfs2/alloc.c |    2 +-
  fs/ocfs2/ioctl.c |    3 ++-
  fs/ocfs2/xattr.c |    2 +-
  3 files changed, 4 insertions(+), 3 deletions(-)
fs/ocfs2/alloc.c +1 -1
index ed553c6..3165aeb 100644
@@ -5699,7 +5699,7 @@ int ocfs2_remove_btree_range(struct inode *inode,
  					   OCFS2_JOURNAL_ACCESS_WRITE);
  	if (ret) {
  		mlog_errno(ret);
-		goto out;
+		goto out_commit;
  	}
 
  	dquot_free_space_nodirty(inode,
fs/ocfs2/ioctl.c +2 -1
index bc91072..5612c3d 100644
@@ -122,7 +122,7 @@ static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags,
  	if ((oldflags&  OCFS2_IMMUTABLE_FL) || ((flags ^ oldflags)&
  		(OCFS2_APPEND_FL | OCFS2_IMMUTABLE_FL))) {
  		if (!capable(CAP_LINUX_IMMUTABLE))
-			goto bail_unlock;
+			goto bail_commit;
  	}
 
  	ocfs2_inode->ip_attr = flags;
@@ -132,6 +132,7 @@ static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags,
  	if (status<  0)
  		mlog_errno(status);
 
+bail_commit:
  	ocfs2_commit_trans(osb, handle);
  bail_unlock:
  	ocfs2_inode_unlock(inode, 1);
fs/ocfs2/xattr.c +0 -0
index 81ecf9c..6eb01c5 100644
@@ -2376,12 +2376,12 @@ static int ocfs2_remove_value_outside(struct inode*inode,
  		}
 
  		ret = ocfs2_xattr_value_truncate(inode, vb, 0,&ctxt);

fs/ocfs2/xattr.c static int ocfs2_remove_value_outside(struct inode*inode, · line 2387

Quoted 10 lines
fs/ocfs2/xattr.c +1 -1
+		ocfs2_commit_trans(osb, ctxt.handle);
  		if (ret<  0) {
  			mlog_errno(ret);
  			break;
  		}
 
-		ocfs2_commit_trans(osb, ctxt.handle);
  		if (ctxt.meta_ac) {
  			ocfs2_free_alloc_context(ctxt.meta_ac);
  			ctxt.meta_ac = NULL;

ret = ocfs2_xattr_value_truncate(inode, vb, 0,&ctxt);
- if (ret< 0) {
- mlog_errno(ret);
- break;
- }

		ocfs2_commit_trans(osb, ctxt.handle);

if (ctxt.meta_ac) {
ocfs2_free_alloc_context(ctxt.meta_ac); ctxt.meta_ac = NULL;

+ if (ret< 0) {
+ mlog_errno(ret);
+ break;
+ }

I don't like the commit_trans() shoe horned. The above reads cleaner imo.

#2 lore raw

PATCH 1/1 ocfs2: Commit transactions in error cases.

Quoted 11 lines · Sunil Mushran

The first two are ok. Have a comment for the last one.

Quoted 8 lines · Wengang Wang

Commit transactions in error cases.

There are three cases found that in error cases, journal transactions are not committed nor aborted. We should take care of these case by committing the transactions. Otherwise, there would left a journal handle which will lead to , in same process context, the comming ocfs2_start_trans() gets wrong credits.

<snip>

Quoted 32 lines
Quoted 14 lines
fs/ocfs2/xattr.c +1 -1
@@ -2376,12 +2376,12 @@ static int ocfs2_remove_value_outside(struct inode*inode,
 		}
 
 		ret = ocfs2_xattr_value_truncate(inode, vb, 0,&ctxt);
+		ocfs2_commit_trans(osb, ctxt.handle);
 		if (ret<  0) {
 			mlog_errno(ret);
 			break;
 		}
 
-		ocfs2_commit_trans(osb, ctxt.handle);
 		if (ctxt.meta_ac) {
 			ocfs2_free_alloc_context(ctxt.meta_ac);
 			ctxt.meta_ac = NULL;
(quoted diff) +0 -4
 		ret = ocfs2_xattr_value_truncate(inode, vb, 0,&ctxt);
- 		if (ret<  0) {
- 			mlog_errno(ret);
- 			break;
- 		}
		ocfs2_commit_trans(osb, ctxt.handle);
(quoted diff) +4 -0
 		if (ctxt.meta_ac) {
 			ocfs2_free_alloc_context(ctxt.meta_ac);
 			ctxt.meta_ac = NULL;
 
+ 		if (ret<  0) {
+ 			mlog_errno(ret);
+ 			break;
+ 		}
Quoted 1 line

I don't like the commit_trans() shoe horned. The above reads cleaner imo.

Agree.

thanks,
wengang.