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/segment.c b/fs/nilfs2/segment.c
index ad65a73..6d66c5c 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -163,8 +163,8 @@
else {
/*
* If journal_info field is occupied by other FS,
- * we save it and restore on nilfs_transaction_end().
- * But this should never happen.
+ * it is saved and will be restored on
+ * nilfs_transaction_commit().
*/
printk(KERN_WARNING
"NILFS warning: journal info from a different "
@@ -195,7 +195,7 @@
*
* nilfs_transaction_begin() acquires a reader/writer semaphore, called
* the segment semaphore, to make a segment construction and write tasks
- * exclusive. The function is used with nilfs_transaction_end() in pairs.
+ * exclusive. The function is used with nilfs_transaction_commit() in pairs.
* The region enclosed by these two functions can be nested. To avoid a
* deadlock, the semaphore is only acquired or released in the outermost call.
*
@@ -212,8 +212,6 @@
*
* %-ENOMEM - Insufficient memory available.
*
- * %-ERESTARTSYS - Interrupted
- *
* %-ENOSPC - No space left on device
*/
int nilfs_transaction_begin(struct super_block *sb,
@@ -248,16 +246,17 @@
}
/**
- * nilfs_transaction_end - end indivisible file operations.
+ * nilfs_transaction_commit - commit indivisible file operations.
* @sb: super block
- * @commit: commit flag (0 for no change)
*
- * nilfs_transaction_end() releases the read semaphore which is
- * acquired by nilfs_transaction_begin(). Its releasing is only done
- * in outermost call of this function. If the nilfs_transaction_info
- * was allocated dynamically, it is given back to a slab cache.
+ * nilfs_transaction_commit() releases the read semaphore which is
+ * acquired by nilfs_transaction_begin(). This is only performed
+ * in outermost call of this function. If a commit flag is set,
+ * nilfs_transaction_commit() sets a timer to start the segment
+ * constructor. If a sync flag is set, it starts construction
+ * directly.
*/
-int nilfs_transaction_end(struct super_block *sb, int commit)
+int nilfs_transaction_commit(struct super_block *sb)
{
struct nilfs_transaction_info *ti = current->journal_info;
struct nilfs_sb_info *sbi;
@@ -265,9 +264,7 @@
int err = 0;
BUG_ON(ti == NULL || ti->ti_magic != NILFS_TI_MAGIC);
-
- if (commit)
- ti->ti_flags |= NILFS_TI_COMMIT;
+ ti->ti_flags |= NILFS_TI_COMMIT;
if (ti->ti_count > 0) {
ti->ti_count--;
return 0;
@@ -291,6 +288,22 @@
return err;
}
+void nilfs_transaction_abort(struct super_block *sb)
+{
+ struct nilfs_transaction_info *ti = current->journal_info;
+
+ BUG_ON(ti == NULL || ti->ti_magic != NILFS_TI_MAGIC);
+ if (ti->ti_count > 0) {
+ ti->ti_count--;
+ return;
+ }
+ up_read(&NILFS_SB(sb)->s_nilfs->ns_segctor_sem);
+
+ current->journal_info = ti->ti_save;
+ if (ti->ti_flags & NILFS_TI_DYNAMIC_ALLOC)
+ kmem_cache_free(nilfs_transaction_cachep, ti);
+}
+
void nilfs_relax_pressure_in_lock(struct super_block *sb)
{
struct nilfs_sb_info *sbi = NILFS_SB(sb);