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/inode.c b/fs/nilfs2/inode.c
index 289d179..4bf1e2c 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -77,7 +77,6 @@
goto out;
err = nilfs_bmap_insert(ii->i_bmap, (unsigned long)blkoff,
(unsigned long)bh_result);
- nilfs_transaction_end(inode->i_sb, !err);
if (unlikely(err != 0)) {
if (err == -EEXIST) {
/*
@@ -100,8 +99,10 @@
inode->i_ino);
err = -EIO;
}
+ nilfs_transaction_abort(inode->i_sb);
goto out;
}
+ nilfs_transaction_commit(inode->i_sb); /* never fails */
/* Error handling should be detailed */
set_buffer_new(bh_result);
map_bh(bh_result, inode->i_sb, 0); /* dbn must be changed
@@ -203,7 +204,7 @@
err = block_write_begin(file, mapping, pos, len, flags, pagep,
fsdata, nilfs_get_block);
if (unlikely(err))
- nilfs_transaction_end(inode->i_sb, 0);
+ nilfs_transaction_abort(inode->i_sb);
return err;
}
@@ -221,7 +222,7 @@
copied = generic_write_end(file, mapping, pos, len, copied, page,
fsdata);
nilfs_set_file_dirty(NILFS_SB(inode->i_sb), inode, nr_dirty);
- err = nilfs_transaction_end(inode->i_sb, 1);
+ err = nilfs_transaction_commit(inode->i_sb);
return err ? : copied;
}
@@ -641,7 +642,7 @@
nilfs_set_transaction_flag(NILFS_TI_SYNC);
nilfs_set_file_dirty(NILFS_SB(sb), inode, 0);
- nilfs_transaction_end(sb, 1);
+ nilfs_transaction_commit(sb);
/* May construct a logical segment and may fail in sync mode.
But truncate has no return value. */
}
@@ -669,7 +670,7 @@
/* nilfs_free_inode() marks inode buffer dirty */
if (IS_SYNC(inode))
nilfs_set_transaction_flag(NILFS_TI_SYNC);
- nilfs_transaction_end(sb, 1);
+ nilfs_transaction_commit(sb);
/* May construct a logical segment and may fail in sync mode.
But delete_inode has no return value. */
}
@@ -679,7 +680,7 @@
struct nilfs_transaction_info ti;
struct inode *inode = dentry->d_inode;
struct super_block *sb = inode->i_sb;
- int err, err2;
+ int err;
err = inode_change_ok(inode, iattr);
if (err)
@@ -691,8 +692,12 @@
err = inode_setattr(inode, iattr);
if (!err && (iattr->ia_valid & ATTR_MODE))
err = nilfs_acl_chmod(inode);
- err2 = nilfs_transaction_end(sb, 1);
- return err ? : err2;
+ if (likely(!err))
+ err = nilfs_transaction_commit(sb);
+ else
+ nilfs_transaction_abort(sb);
+
+ return err;
}
int nilfs_load_inode_block(struct nilfs_sb_info *sbi, struct inode *inode,
@@ -817,5 +822,5 @@
nilfs_transaction_begin(inode->i_sb, &ti, 0);
if (likely(inode->i_ino != NILFS_SKETCH_INO))
nilfs_mark_inode_dirty(inode);
- nilfs_transaction_end(inode->i_sb, 1); /* never fails */
+ nilfs_transaction_commit(inode->i_sb); /* never fails */
}