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>
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 @@
{
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 @@
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 @@
{
struct inode *inode;
struct nilfs_transaction_info ti;
- int err, err2;
+ int err;
if (!new_valid_dev(rdev))
return -EINVAL;
@@ -147,8 +151,12 @@
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 @@
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 @@
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 @@
{
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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;
}