nilfs2: avoid double error caused by nilfs_transaction_end

Pekka Enberg pointed out that double error handlings found after
nilfs_transaction_end() can be avoided by separating abort operation:

 OK, I don't understand this. The only way nilfs_transaction_end() can
 fail is if we have NILFS_TI_SYNC set and we fail to construct the
 segment. But why do we want to construct a segment if we don't commit?

 I guess what I'm asking is why don't we have a separate
 nilfs_transaction_abort() function that can't fail for the erroneous
 case to avoid this double error value tracking thing?

This does the separation and renames nilfs_transaction_end() to
nilfs_transaction_commit() for clarification.

Since, some calls of these functions were used just for exclusion control
against the segment constructor, they are replaced with semaphore
operations.

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

1 files changed, 51 insertions, 23 deletions

diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
index 95d1b29..df70dad 100644
--- a/fs/nilfs2/namei.c
+++ b/fs/nilfs2/namei.c
@@ -109,7 +109,7 @@ static int nilfs_create(struct inode *dir, struct dentry *dentry, int mode,
 {
 	struct inode *inode;
 	struct nilfs_transaction_info ti;
-	int err, err2;
+	int err;
 
 	err = nilfs_transaction_begin(dir->i_sb, &ti, 1);
 	if (err)
@@ -123,8 +123,12 @@ static int nilfs_create(struct inode *dir, struct dentry *dentry, int mode,
 		mark_inode_dirty(inode);
 		err = nilfs_add_nondir(dentry, inode);
 	}
-	err2 = nilfs_transaction_end(dir->i_sb, !err);
-	return err ? : err2;
+	if (!err)
+		err = nilfs_transaction_commit(dir->i_sb);
+	else
+		nilfs_transaction_abort(dir->i_sb);
+
+	return err;
 }
 
 static int
@@ -132,7 +136,7 @@ nilfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t rdev)
 {
 	struct inode *inode;
 	struct nilfs_transaction_info ti;
-	int err, err2;
+	int err;
 
 	if (!new_valid_dev(rdev))
 		return -EINVAL;
@@ -147,8 +151,12 @@ nilfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t rdev)
 		mark_inode_dirty(inode);
 		err = nilfs_add_nondir(dentry, inode);
 	}
-	err2 = nilfs_transaction_end(dir->i_sb, !err);
-	return err ? : err2;
+	if (!err)
+		err = nilfs_transaction_commit(dir->i_sb);
+	else
+		nilfs_transaction_abort(dir->i_sb);
+
+	return err;
 }
 
 static int nilfs_symlink(struct inode *dir, struct dentry *dentry,
@@ -158,7 +166,7 @@ static int nilfs_symlink(struct inode *dir, struct dentry *dentry,
 	struct super_block *sb = dir->i_sb;
 	unsigned l = strlen(symname)+1;
 	struct inode *inode;
-	int err, err2;
+	int err;
 
 	if (l > sb->s_blocksize)
 		return -ENAMETOOLONG;
@@ -184,8 +192,12 @@ static int nilfs_symlink(struct inode *dir, struct dentry *dentry,
 
 	err = nilfs_add_nondir(dentry, inode);
 out:
-	err2 = nilfs_transaction_end(dir->i_sb, !err);
-	return err ? : err2;
+	if (!err)
+		err = nilfs_transaction_commit(dir->i_sb);
+	else
+		nilfs_transaction_abort(dir->i_sb);
+
+	return err;
 
 out_fail:
 	inode_dec_link_count(inode);
@@ -198,7 +210,7 @@ static int nilfs_link(struct dentry *old_dentry, struct inode *dir,
 {
 	struct inode *inode = old_dentry->d_inode;
 	struct nilfs_transaction_info ti;
-	int err, err2;
+	int err;
 
 	if (inode->i_nlink >= NILFS_LINK_MAX)
 		return -EMLINK;
@@ -212,15 +224,19 @@ static int nilfs_link(struct dentry *old_dentry, struct inode *dir,
 	atomic_inc(&inode->i_count);
 
 	err = nilfs_add_nondir(dentry, inode);
-	err2 = nilfs_transaction_end(dir->i_sb, !err);
-	return err ? : err2;
+	if (!err)
+		err = nilfs_transaction_commit(dir->i_sb);
+	else
+		nilfs_transaction_abort(dir->i_sb);
+
+	return err;
 }
 
 static int nilfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 {
 	struct inode *inode;
 	struct nilfs_transaction_info ti;
-	int err, err2;
+	int err;
 
 	if (dir->i_nlink >= NILFS_LINK_MAX)
 		return -EMLINK;
@@ -252,8 +268,12 @@ static int nilfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 
 	d_instantiate(dentry, inode);
 out:
-	err2 = nilfs_transaction_end(dir->i_sb, !err);
-	return err ? : err2;
+	if (!err)
+		err = nilfs_transaction_commit(dir->i_sb);
+	else
+		nilfs_transaction_abort(dir->i_sb);
+
+	return err;
 
 out_fail:
 	inode_dec_link_count(inode);
@@ -270,7 +290,7 @@ static int nilfs_unlink(struct inode *dir, struct dentry *dentry)
 	struct nilfs_dir_entry *de;
 	struct page *page;
 	struct nilfs_transaction_info ti;
-	int err, err2;
+	int err;
 
 	err = nilfs_transaction_begin(dir->i_sb, &ti, 0);
 	if (err)
@@ -300,15 +320,19 @@ static int nilfs_unlink(struct inode *dir, struct dentry *dentry)
 	inode_dec_link_count(inode);
 	err = 0;
 out:
-	err2 = nilfs_transaction_end(dir->i_sb, !err);
-	return err ? : err2;
+	if (!err)
+		err = nilfs_transaction_commit(dir->i_sb);
+	else
+		nilfs_transaction_abort(dir->i_sb);
+
+	return err;
 }
 
 static int nilfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
 	struct nilfs_transaction_info ti;
-	int err, err2;
+	int err;
 
 	err = nilfs_transaction_begin(dir->i_sb, &ti, 0);
 	if (err)
@@ -323,8 +347,12 @@ static int nilfs_rmdir(struct inode *dir, struct dentry *dentry)
 			inode_dec_link_count(dir);
 		}
 	}
-	err2 = nilfs_transaction_end(dir->i_sb, !err);
-	return err ? : err2;
+	if (!err)
+		err = nilfs_transaction_commit(dir->i_sb);
+	else
+		nilfs_transaction_abort(dir->i_sb);
+
+	return err;
 }
 
 static int nilfs_rename(struct inode *old_dir, struct dentry *old_dentry,
@@ -404,7 +432,7 @@ static int nilfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		inode_dec_link_count(old_dir);
 	}
 
-	err = nilfs_transaction_end(old_dir->i_sb, 1);
+	err = nilfs_transaction_commit(old_dir->i_sb);
 	return err;
 
 out_dir:
@@ -416,7 +444,7 @@ out_old:
 	kunmap(old_page);
 	page_cache_release(old_page);
 out:
-	nilfs_transaction_end(old_dir->i_sb, 0);
+	nilfs_transaction_abort(old_dir->i_sb);
 	return err;
 }