[PATCHv4 0/2] nilfs2: asynchronous sb update

[PATCHv4 0/2] nilfs2: asynchronous sb update

[Jiro SEKIBA]

Hi,

This is v4 patch series to update super block asynchronously.

First patch separates super block validity check from nilfs_commit_super,
and moved the function into callers.

Second patch introduces swapping code in nilfs_prepare_super.
Caller of the function must specify whether swapping super blocks or not
explicitly.  To advance log pointer, the caller uses nilfs_set_log_cursor
to advance specified super block after calling nilfs_prepare_super.

changes from v3
- move swapping code to nilfs_prepare_super from nilfs_sync_super
- check last_cno of super blocks to advance protection period
- introduce nilfs_sb_need_swap function

changes from v2
- restore super block in nilfs_prepare_super
- introduce nilfs_set_error
 * used in nilfs_error
- introduce nilfs_set_log_cursor instead of write_log_cursor
 * instead of writing error, set log cursor to specified sb pointer
 * used followed by nilfs_commit_super
- introduce ns_sbwcount
 * swap super blocks based on the counter
 * swap all most all the time, still asymmetric(9:7 ratio)
- delete ns_sbwtime[2]
 * introduce ns_sbwtime instead of array of both super blocks
- synchronize super blocks when mount/remount
- abandon NILFS_SB_SET_FN
 * only used in nilfs_set_error, so assign directly in the function
- misc style fix
- misc comment fix

changes from v1
- change macro name NILFS_SUPER_SET_FN -> NILFS_SB_SET_FN
 * change inline function name as nilfs_sb_set_##name
- introduce enum for nilfs_commit_super and nilfs_sync_super
 * NILFS_SB_COMMIT and NILFS_SB_COMMIT_ALL
 * use flag instead of dupsb in nilfs_commit_super/sync_super

 fs/nilfs2/nilfs.h     |   12 +++
 fs/nilfs2/segment.c   |   10 ++-
 fs/nilfs2/super.c     |  182 ++++++++++++++++++++++++++++++++++---------------
 fs/nilfs2/the_nilfs.c |   17 +++--
 fs/nilfs2/the_nilfs.h |   17 ++---
 5 files changed, 166 insertions(+), 72 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 1/2] nilfs2: introduce nilfs_prepare_super

[Jiro SEKIBA]

This function checks validity of super block pointers.
If first super block is invalid, it will swap the super blocks.
The function should be called before any super block information updates.
Caller must obtain nilfs->ns_sem.

Signed-off-by: Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
---
 fs/nilfs2/nilfs.h     |    1 +
 fs/nilfs2/segment.c   |    6 ++-
 fs/nilfs2/super.c     |   87 ++++++++++++++++++++++++++++++++----------------
 fs/nilfs2/the_nilfs.c |   11 ++++--
 4 files changed, 71 insertions(+), 34 deletions(-)

diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 47d6d79..649e079 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -270,6 +270,7 @@ extern struct nilfs_super_block *
 nilfs_read_super_block(struct super_block *, u64, int, struct buffer_head **);
 extern int nilfs_store_magic_and_option(struct super_block *,
 					struct nilfs_super_block *, char *);
+extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *);
 extern int nilfs_commit_super(struct nilfs_sb_info *, int);
 extern int nilfs_attach_checkpoint(struct nilfs_sb_info *, __u64);
 extern void nilfs_detach_checkpoint(struct nilfs_sb_info *);
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index 1f7881c..075d7b0 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2423,8 +2423,10 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
 		if (test_bit(NILFS_SC_SUPER_ROOT, &sci->sc_flags) &&
 		    nilfs_discontinued(nilfs)) {
 			down_write(&nilfs->ns_sem);
-			err = nilfs_commit_super(
-				sbi, nilfs_altsb_need_update(nilfs));
+			err = -EIO;
+			if (likely(nilfs_prepare_super(sbi)))
+				err = nilfs_commit_super(
+					sbi, nilfs_altsb_need_update(nilfs));
 			up_write(&nilfs->ns_sem);
 		}
 	}
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 414ef68..045b8d7 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -90,6 +90,7 @@ void nilfs_error(struct super_block *sb, const char *function,
 		 const char *fmt, ...)
 {
 	struct nilfs_sb_info *sbi = NILFS_SB(sb);
+	struct nilfs_super_block **sbp;
 	va_list args;
 
 	va_start(args, fmt);
@@ -104,9 +105,11 @@ void nilfs_error(struct super_block *sb, const char *function,
 		down_write(&nilfs->ns_sem);
 		if (!(nilfs->ns_mount_state & NILFS_ERROR_FS)) {
 			nilfs->ns_mount_state |= NILFS_ERROR_FS;
-			nilfs->ns_sbp[0]->s_state |=
-				cpu_to_le16(NILFS_ERROR_FS);
-			nilfs_commit_super(sbi, 1);
+			sbp = nilfs_prepare_super(sbi);
+			if (likely(sbp)) {
+				sbp[0]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
+				nilfs_commit_super(sbi, 1);
+			}
 		}
 		up_write(&nilfs->ns_sem);
 
@@ -233,24 +236,34 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
 	return err;
 }
 
-int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
+struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
 {
 	struct the_nilfs *nilfs = sbi->s_nilfs;
 	struct nilfs_super_block **sbp = nilfs->ns_sbp;
-	sector_t nfreeblocks;
-	time_t t;
-	int err;
 
-	/* nilfs->sem must be locked by the caller. */
+	/* nilfs->ns_sem must be locked by the caller. */
 	if (sbp[0]->s_magic != cpu_to_le16(NILFS_SUPER_MAGIC)) {
-		if (sbp[1] && sbp[1]->s_magic == cpu_to_le16(NILFS_SUPER_MAGIC))
+		if (sbp[1] &&
+		    sbp[1]->s_magic == cpu_to_le16(NILFS_SUPER_MAGIC)) {
 			nilfs_swap_super_block(nilfs);
-		else {
+		} else {
 			printk(KERN_CRIT "NILFS: superblock broke on dev %s\n",
 			       sbi->s_super->s_id);
-			return -EIO;
+			return NULL;
 		}
 	}
+	return sbp;
+}
+
+int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
+{
+	struct the_nilfs *nilfs = sbi->s_nilfs;
+	struct nilfs_super_block **sbp = nilfs->ns_sbp;
+	sector_t nfreeblocks;
+	time_t t;
+	int err;
+
+	/* nilfs->ns_sem must be locked by the caller. */
 	err = nilfs_count_free_blocks(nilfs, &nfreeblocks);
 	if (unlikely(err)) {
 		printk(KERN_ERR "NILFS: failed to count free blocks\n");
@@ -282,6 +295,7 @@ static void nilfs_put_super(struct super_block *sb)
 {
 	struct nilfs_sb_info *sbi = NILFS_SB(sb);
 	struct the_nilfs *nilfs = sbi->s_nilfs;
+	struct nilfs_super_block **sbp;
 
 	lock_kernel();
 
@@ -289,8 +303,11 @@ static void nilfs_put_super(struct super_block *sb)
 
 	if (!(sb->s_flags & MS_RDONLY)) {
 		down_write(&nilfs->ns_sem);
-		nilfs->ns_sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
-		nilfs_commit_super(sbi, 1);
+		sbp = nilfs_prepare_super(sbi);
+		if (likely(sbp)) {
+			sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
+			nilfs_commit_super(sbi, 1);
+		}
 		up_write(&nilfs->ns_sem);
 	}
 	down_write(&nilfs->ns_super_sem);
@@ -318,7 +335,7 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
 		err = nilfs_construct_segment(sb);
 
 	down_write(&nilfs->ns_sem);
-	if (nilfs_sb_dirty(nilfs))
+	if (nilfs_sb_dirty(nilfs) && nilfs_prepare_super(sbi))
 		nilfs_commit_super(sbi, 1);
 	up_write(&nilfs->ns_sem);
 
@@ -613,11 +630,19 @@ nilfs_set_default_options(struct nilfs_sb_info *sbi,
 static int nilfs_setup_super(struct nilfs_sb_info *sbi)
 {
 	struct the_nilfs *nilfs = sbi->s_nilfs;
-	struct nilfs_super_block *sbp = nilfs->ns_sbp[0];
-	int max_mnt_count = le16_to_cpu(sbp->s_max_mnt_count);
-	int mnt_count = le16_to_cpu(sbp->s_mnt_count);
+	struct nilfs_super_block **sbp;
+	int max_mnt_count;
+	int mnt_count;
+
+	/* nilfs->ns_sem must be locked by the caller. */
+	sbp = nilfs_prepare_super(sbi);
+	if (!sbp)
+		return -EIO;
+
+	max_mnt_count = le16_to_cpu(sbp[0]->s_max_mnt_count);
+	mnt_count = le16_to_cpu(sbp[0]->s_mnt_count);
 
-	/* nilfs->sem must be locked by the caller. */
+	/* nilfs->ns_sem must be locked by the caller. */
 	if (nilfs->ns_mount_state & NILFS_ERROR_FS) {
 		printk(KERN_WARNING
 		       "NILFS warning: mounting fs with errors\n");
@@ -628,11 +653,12 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
 #endif
 	}
 	if (!max_mnt_count)
-		sbp->s_max_mnt_count = cpu_to_le16(NILFS_DFL_MAX_MNT_COUNT);
+		sbp[0]->s_max_mnt_count = cpu_to_le16(NILFS_DFL_MAX_MNT_COUNT);
 
-	sbp->s_mnt_count = cpu_to_le16(mnt_count + 1);
-	sbp->s_state = cpu_to_le16(le16_to_cpu(sbp->s_state) & ~NILFS_VALID_FS);
-	sbp->s_mtime = cpu_to_le64(get_seconds());
+	sbp[0]->s_mnt_count = cpu_to_le16(mnt_count + 1);
+	sbp[0]->s_state =
+		cpu_to_le16(le16_to_cpu(sbp[0]->s_state) & ~NILFS_VALID_FS);
+	sbp[0]->s_mtime = cpu_to_le64(get_seconds());
 	return nilfs_commit_super(sbi, 1);
 }
 
@@ -819,7 +845,7 @@ nilfs_fill_super(struct super_block *sb, void *data, int silent,
 static int nilfs_remount(struct super_block *sb, int *flags, char *data)
 {
 	struct nilfs_sb_info *sbi = NILFS_SB(sb);
-	struct nilfs_super_block *sbp;
+	struct nilfs_super_block **sbp;
 	struct the_nilfs *nilfs = sbi->s_nilfs;
 	unsigned long old_sb_flags;
 	struct nilfs_mount_options old_opts;
@@ -880,12 +906,15 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
 		 * the RDONLY flag and then mark the partition as valid again.
 		 */
 		down_write(&nilfs->ns_sem);
-		sbp = nilfs->ns_sbp[0];
-		if (!(sbp->s_state & le16_to_cpu(NILFS_VALID_FS)) &&
-		    (nilfs->ns_mount_state & NILFS_VALID_FS))
-			sbp->s_state = cpu_to_le16(nilfs->ns_mount_state);
-		sbp->s_mtime = cpu_to_le64(get_seconds());
-		nilfs_commit_super(sbi, 1);
+		sbp = nilfs_prepare_super(sbi);
+		if (likely(sbp)) {
+			if (!(sbp[0]->s_state & le16_to_cpu(NILFS_VALID_FS)) &&
+			    (nilfs->ns_mount_state & NILFS_VALID_FS))
+				sbp[0]->s_state =
+					cpu_to_le16(nilfs->ns_mount_state);
+			sbp[0]->s_mtime = cpu_to_le64(get_seconds());
+			nilfs_commit_super(sbi, 1);
+		}
 		up_write(&nilfs->ns_sem);
 	} else {
 		/*
diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
index 9f2cb01..74b0480 100644
--- a/fs/nilfs2/the_nilfs.c
+++ b/fs/nilfs2/the_nilfs.c
@@ -261,6 +261,7 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
 	unsigned int s_flags = sbi->s_super->s_flags;
 	int really_read_only = bdev_read_only(nilfs->ns_bdev);
 	int valid_fs = nilfs_valid_fs(nilfs);
+	struct nilfs_super_block **sbp;
 	int err;
 
 	if (nilfs_loaded(nilfs)) {
@@ -324,9 +325,13 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
 		goto failed_unload;
 
 	down_write(&nilfs->ns_sem);
-	nilfs->ns_mount_state |= NILFS_VALID_FS;
-	nilfs->ns_sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
-	err = nilfs_commit_super(sbi, 1);
+	err = -EIO;
+	sbp = nilfs_prepare_super(sbi);
+	if (likely(sbp)) {
+		nilfs->ns_mount_state |= NILFS_VALID_FS;
+		sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
+		err = nilfs_commit_super(sbi, 1);
+	}
 	up_write(&nilfs->ns_sem);
 
 	if (err) {
-- 
1.5.6.5

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 2/2] nilfs2: sync super blocks in turns

[Jiro SEKIBA]

This will sync super blocks in turns instead of syncing duplicate
super blocks at the time.  This will help searching valid super root when
super block is written into disk before log is written, which is happen when
barrier-less block devices are unmounted uncleanly.
In the situation, old super block likely points to valid log.

This patch introduces ns_sbwcount member, which counts how many times super
blocks write back to the disk.  Newly introduced nilfs_sb_need_swap() function
decides whether flipping required or not based on the ns_sbwcount to sync
super blocks asymmetrically.

nilfs_prepare_super swaps super blocks according to the argument.
The argument is calculated by nilfs_sb_need_swap() function.

The patch also introduces new function nilfs_set_log_cursor to advance
log cursor for specified super block.  To update both of super block
information, caller of nilfs_commit_super must set the information on both
super blocks.

Signed-off-by: Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
---
 fs/nilfs2/nilfs.h     |   13 +++++-
 fs/nilfs2/segment.c   |   10 +++-
 fs/nilfs2/super.c     |  131 ++++++++++++++++++++++++++++++++-----------------
 fs/nilfs2/the_nilfs.c |   10 ++--
 fs/nilfs2/the_nilfs.h |   17 +++---
 5 files changed, 119 insertions(+), 62 deletions(-)

diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 649e079..63c8fd4 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -107,6 +107,14 @@ enum {
 };
 
 /*
+ * commit flags for nilfs_commit_super and nilfs_sync_super
+ */
+enum {
+	NILFS_SB_COMMIT = 0,	/* Commit a super block alternately */
+	NILFS_SB_COMMIT_ALL	/* Commit both super blocks */
+};
+
+/*
  * Macros to check inode numbers
  */
 #define NILFS_MDT_INO_BITS   \
@@ -270,7 +278,10 @@ extern struct nilfs_super_block *
 nilfs_read_super_block(struct super_block *, u64, int, struct buffer_head **);
 extern int nilfs_store_magic_and_option(struct super_block *,
 					struct nilfs_super_block *, char *);
-extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *);
+extern void nilfs_set_log_cursor(struct nilfs_super_block *,
+				 struct the_nilfs *);
+extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *,
+						      int flip);
 extern int nilfs_commit_super(struct nilfs_sb_info *, int);
 extern int nilfs_attach_checkpoint(struct nilfs_sb_info *, __u64);
 extern void nilfs_detach_checkpoint(struct nilfs_sb_info *);
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index 075d7b0..844743e 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2408,6 +2408,7 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
 {
 	struct nilfs_sb_info *sbi = sci->sc_sbi;
 	struct the_nilfs *nilfs = sbi->s_nilfs;
+	struct nilfs_super_block **sbp;
 	int err = 0;
 
 	nilfs_segctor_accept(sci);
@@ -2424,9 +2425,12 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
 		    nilfs_discontinued(nilfs)) {
 			down_write(&nilfs->ns_sem);
 			err = -EIO;
-			if (likely(nilfs_prepare_super(sbi)))
-				err = nilfs_commit_super(
-					sbi, nilfs_altsb_need_update(nilfs));
+			sbp = nilfs_prepare_super(sbi,
+						  nilfs_sb_need_swap(nilfs));
+			if (likely(sbp)) {
+				nilfs_set_log_cursor(sbp[0], nilfs);
+				err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
+			}
 			up_write(&nilfs->ns_sem);
 		}
 	}
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 045b8d7..cd35f73 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -74,6 +74,25 @@ struct kmem_cache *nilfs_btree_path_cache;
 
 static int nilfs_remount(struct super_block *sb, int *flags, char *data);
 
+static void nilfs_set_error(struct nilfs_sb_info *sbi)
+{
+	struct the_nilfs *nilfs = sbi->s_nilfs;
+	struct nilfs_super_block **sbp;
+
+	down_write(&nilfs->ns_sem);
+	if (!(nilfs->ns_mount_state & NILFS_ERROR_FS)) {
+		nilfs->ns_mount_state |= NILFS_ERROR_FS;
+		sbp = nilfs_prepare_super(sbi, 0);
+		if (likely(sbp)) {
+			sbp[0]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
+			if (sbp[1])
+				sbp[1]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
+			nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
+		}
+	}
+	up_write(&nilfs->ns_sem);
+}
+
 /**
  * nilfs_error() - report failure condition on a filesystem
  *
@@ -90,7 +109,6 @@ void nilfs_error(struct super_block *sb, const char *function,
 		 const char *fmt, ...)
 {
 	struct nilfs_sb_info *sbi = NILFS_SB(sb);
-	struct nilfs_super_block **sbp;
 	va_list args;
 
 	va_start(args, fmt);
@@ -100,18 +118,7 @@ void nilfs_error(struct super_block *sb, const char *function,
 	va_end(args);
 
 	if (!(sb->s_flags & MS_RDONLY)) {
-		struct the_nilfs *nilfs = sbi->s_nilfs;
-
-		down_write(&nilfs->ns_sem);
-		if (!(nilfs->ns_mount_state & NILFS_ERROR_FS)) {
-			nilfs->ns_mount_state |= NILFS_ERROR_FS;
-			sbp = nilfs_prepare_super(sbi);
-			if (likely(sbp)) {
-				sbp[0]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
-				nilfs_commit_super(sbi, 1);
-			}
-		}
-		up_write(&nilfs->ns_sem);
+		nilfs_set_error(sbi);
 
 		if (nilfs_test_opt(sbi, ERRORS_RO)) {
 			printk(KERN_CRIT "Remounting filesystem read-only\n");
@@ -179,7 +186,7 @@ static void nilfs_clear_inode(struct inode *inode)
 	nilfs_btnode_cache_clear(&ii->i_btnode_cache);
 }
 
-static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
+static int nilfs_sync_super(struct nilfs_sb_info *sbi, int flag)
 {
 	struct the_nilfs *nilfs = sbi->s_nilfs;
 	int err;
@@ -205,12 +212,20 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
 		printk(KERN_ERR
 		       "NILFS: unable to write superblock (err=%d)\n", err);
 		if (err == -EIO && nilfs->ns_sbh[1]) {
+			/*
+			 * sbp[0] points to newer log than sbp[1],
+			 * so copy sbp[0] to sbp[1] to take over sbp[0].
+			 */
+			memcpy(nilfs->ns_sbp[1], nilfs->ns_sbp[0],
+			       nilfs->ns_sbsize);
 			nilfs_fall_back_super_block(nilfs);
 			goto retry;
 		}
 	} else {
 		struct nilfs_super_block *sbp = nilfs->ns_sbp[0];
 
+		nilfs->ns_sbwcount++;
+
 		/*
 		 * The latest segment becomes trailable from the position
 		 * written in superblock.
@@ -220,11 +235,14 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
 		/* update GC protection for recent segments */
 		if (nilfs->ns_sbh[1]) {
 			sbp = NULL;
-			if (dupsb) {
+			if (flag == NILFS_SB_COMMIT_ALL) {
 				set_buffer_dirty(nilfs->ns_sbh[1]);
 				if (!sync_dirty_buffer(nilfs->ns_sbh[1]))
 					sbp = nilfs->ns_sbp[1];
 			}
+			if (sbp &&
+			    nilfs->ns_sbp[0]->s_last_cno < sbp->s_last_cno)
+				sbp = nilfs->ns_sbp[0];
 		}
 		if (sbp) {
 			spin_lock(&nilfs->ns_last_segment_lock);
@@ -236,7 +254,8 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
 	return err;
 }
 
-struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
+struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi,
+					       int flip)
 {
 	struct the_nilfs *nilfs = sbi->s_nilfs;
 	struct nilfs_super_block **sbp = nilfs->ns_sbp;
@@ -245,50 +264,62 @@ struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
 	if (sbp[0]->s_magic != cpu_to_le16(NILFS_SUPER_MAGIC)) {
 		if (sbp[1] &&
 		    sbp[1]->s_magic == cpu_to_le16(NILFS_SUPER_MAGIC)) {
-			nilfs_swap_super_block(nilfs);
+			memcpy(sbp[0], sbp[1], nilfs->ns_sbsize);
 		} else {
 			printk(KERN_CRIT "NILFS: superblock broke on dev %s\n",
 			       sbi->s_super->s_id);
 			return NULL;
 		}
+	} else if (sbp[1] &&
+		   sbp[1]->s_magic != cpu_to_le16(NILFS_SUPER_MAGIC)) {
+			memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
 	}
+
+	if (flip && sbp[1])
+		nilfs_swap_super_block(nilfs);
+
 	return sbp;
 }
 
-int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
+void nilfs_set_log_cursor(struct nilfs_super_block *sbp,
+			  struct the_nilfs *nilfs)
 {
-	struct the_nilfs *nilfs = sbi->s_nilfs;
-	struct nilfs_super_block **sbp = nilfs->ns_sbp;
 	sector_t nfreeblocks;
-	time_t t;
-	int err;
 
 	/* nilfs->ns_sem must be locked by the caller. */
-	err = nilfs_count_free_blocks(nilfs, &nfreeblocks);
-	if (unlikely(err)) {
-		printk(KERN_ERR "NILFS: failed to count free blocks\n");
-		return err;
-	}
+	nilfs_count_free_blocks(nilfs, &nfreeblocks);
+	sbp->s_free_blocks_count = cpu_to_le64(nfreeblocks);
+
 	spin_lock(&nilfs->ns_last_segment_lock);
-	sbp[0]->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
-	sbp[0]->s_last_pseg = cpu_to_le64(nilfs->ns_last_pseg);
-	sbp[0]->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
+	sbp->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
+	sbp->s_last_pseg = cpu_to_le64(nilfs->ns_last_pseg);
+	sbp->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
 	spin_unlock(&nilfs->ns_last_segment_lock);
+}
+
+int nilfs_commit_super(struct nilfs_sb_info *sbi, int flag)
+{
+	struct the_nilfs *nilfs = sbi->s_nilfs;
+	struct nilfs_super_block **sbp = nilfs->ns_sbp;
+	time_t t;
 
+	/* nilfs->ns_sem must be locked by the caller. */
 	t = get_seconds();
-	nilfs->ns_sbwtime[0] = t;
-	sbp[0]->s_free_blocks_count = cpu_to_le64(nfreeblocks);
+	nilfs->ns_sbwtime = t;
 	sbp[0]->s_wtime = cpu_to_le64(t);
 	sbp[0]->s_sum = 0;
 	sbp[0]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
 					     (unsigned char *)sbp[0],
 					     nilfs->ns_sbsize));
-	if (dupsb && sbp[1]) {
-		memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
-		nilfs->ns_sbwtime[1] = t;
+	if (flag == NILFS_SB_COMMIT_ALL && sbp[1]) {
+		sbp[1]->s_wtime = sbp[0]->s_wtime;
+		sbp[1]->s_sum = 0;
+		sbp[1]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
+					    (unsigned char *)sbp[1],
+					    nilfs->ns_sbsize));
 	}
 	clear_nilfs_sb_dirty(nilfs);
-	return nilfs_sync_super(sbi, dupsb);
+	return nilfs_sync_super(sbi, flag);
 }
 
 static void nilfs_put_super(struct super_block *sb)
@@ -303,10 +334,12 @@ static void nilfs_put_super(struct super_block *sb)
 
 	if (!(sb->s_flags & MS_RDONLY)) {
 		down_write(&nilfs->ns_sem);
-		sbp = nilfs_prepare_super(sbi);
+		sbp = nilfs_prepare_super(sbi, 0);
 		if (likely(sbp)) {
+			/* set state only for newer super block */
 			sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
-			nilfs_commit_super(sbi, 1);
+			nilfs_set_log_cursor(sbp[0], nilfs);
+			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
 		}
 		up_write(&nilfs->ns_sem);
 	}
@@ -328,6 +361,7 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
 {
 	struct nilfs_sb_info *sbi = NILFS_SB(sb);
 	struct the_nilfs *nilfs = sbi->s_nilfs;
+	struct nilfs_super_block **sbp;
 	int err = 0;
 
 	/* This function is called when super block should be written back */
@@ -335,8 +369,13 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
 		err = nilfs_construct_segment(sb);
 
 	down_write(&nilfs->ns_sem);
-	if (nilfs_sb_dirty(nilfs) && nilfs_prepare_super(sbi))
-		nilfs_commit_super(sbi, 1);
+	if (nilfs_sb_dirty(nilfs)) {
+		sbp = nilfs_prepare_super(sbi, nilfs_sb_need_swap(nilfs));
+		if (likely(sbp)) {
+			nilfs_set_log_cursor(sbp[0], nilfs);
+			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
+		}
+	}
 	up_write(&nilfs->ns_sem);
 
 	return err;
@@ -635,14 +674,13 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
 	int mnt_count;
 
 	/* nilfs->ns_sem must be locked by the caller. */
-	sbp = nilfs_prepare_super(sbi);
+	sbp = nilfs_prepare_super(sbi, 0);
 	if (!sbp)
 		return -EIO;
 
 	max_mnt_count = le16_to_cpu(sbp[0]->s_max_mnt_count);
 	mnt_count = le16_to_cpu(sbp[0]->s_mnt_count);
 
-	/* nilfs->ns_sem must be locked by the caller. */
 	if (nilfs->ns_mount_state & NILFS_ERROR_FS) {
 		printk(KERN_WARNING
 		       "NILFS warning: mounting fs with errors\n");
@@ -659,7 +697,9 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
 	sbp[0]->s_state =
 		cpu_to_le16(le16_to_cpu(sbp[0]->s_state) & ~NILFS_VALID_FS);
 	sbp[0]->s_mtime = cpu_to_le64(get_seconds());
-	return nilfs_commit_super(sbi, 1);
+	/* synchronize sbp[1] with sbp[0] */
+	memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
+	return nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
 }
 
 struct nilfs_super_block *nilfs_read_super_block(struct super_block *sb,
@@ -906,14 +946,15 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
 		 * the RDONLY flag and then mark the partition as valid again.
 		 */
 		down_write(&nilfs->ns_sem);
-		sbp = nilfs_prepare_super(sbi);
+		sbp = nilfs_prepare_super(sbi, 0);
 		if (likely(sbp)) {
 			if (!(sbp[0]->s_state & le16_to_cpu(NILFS_VALID_FS)) &&
 			    (nilfs->ns_mount_state & NILFS_VALID_FS))
 				sbp[0]->s_state =
 					cpu_to_le16(nilfs->ns_mount_state);
 			sbp[0]->s_mtime = cpu_to_le64(get_seconds());
-			nilfs_commit_super(sbi, 1);
+			nilfs_set_log_cursor(sbp[0], nilfs);
+			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
 		}
 		up_write(&nilfs->ns_sem);
 	} else {
diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
index 74b0480..7fdb780 100644
--- a/fs/nilfs2/the_nilfs.c
+++ b/fs/nilfs2/the_nilfs.c
@@ -326,11 +326,13 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
 
 	down_write(&nilfs->ns_sem);
 	err = -EIO;
-	sbp = nilfs_prepare_super(sbi);
+	sbp = nilfs_prepare_super(sbi, 0);
 	if (likely(sbp)) {
 		nilfs->ns_mount_state |= NILFS_VALID_FS;
+		/* set the flag only for newer super block */
 		sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
-		err = nilfs_commit_super(sbi, 1);
+		nilfs_set_log_cursor(sbp[0], nilfs);
+		err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
 	}
 	up_write(&nilfs->ns_sem);
 
@@ -519,8 +521,8 @@ static int nilfs_load_super_block(struct the_nilfs *nilfs,
 		nilfs_swap_super_block(nilfs);
 	}
 
-	nilfs->ns_sbwtime[0] = le64_to_cpu(sbp[0]->s_wtime);
-	nilfs->ns_sbwtime[1] = valid[!swp] ? le64_to_cpu(sbp[1]->s_wtime) : 0;
+	nilfs->ns_sbwcount = 0;
+	nilfs->ns_sbwtime = le64_to_cpu(sbp[0]->s_wtime);
 	nilfs->ns_prot_seq = le64_to_cpu(sbp[valid[1] & !swp]->s_last_seq);
 	*sbpp = sbp[0];
 	return 0;
diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
index 85df47f..0a44688 100644
--- a/fs/nilfs2/the_nilfs.h
+++ b/fs/nilfs2/the_nilfs.h
@@ -57,7 +57,8 @@ enum {
  * @ns_current: back pointer to current mount
  * @ns_sbh: buffer heads of on-disk super blocks
  * @ns_sbp: pointers to super block data
- * @ns_sbwtime: previous write time of super blocks
+ * @ns_sbwtime: previous write time of super block
+ * @ns_sbwcount: write count of super block
  * @ns_sbsize: size of valid data in super block
  * @ns_supers: list of nilfs super block structs
  * @ns_seg_seq: segment sequence counter
@@ -120,7 +121,8 @@ struct the_nilfs {
 	 */
 	struct buffer_head     *ns_sbh[2];
 	struct nilfs_super_block *ns_sbp[2];
-	time_t			ns_sbwtime[2];
+	time_t			ns_sbwtime;
+	unsigned		ns_sbwcount;
 	unsigned		ns_sbsize;
 	unsigned		ns_mount_state;
 
@@ -205,20 +207,17 @@ THE_NILFS_FNS(SB_DIRTY, sb_dirty)
 
 /* Minimum interval of periodical update of superblocks (in seconds) */
 #define NILFS_SB_FREQ		10
-#define NILFS_ALTSB_FREQ	60  /* spare superblock */
 
 static inline int nilfs_sb_need_update(struct the_nilfs *nilfs)
 {
 	u64 t = get_seconds();
-	return t < nilfs->ns_sbwtime[0] ||
-		 t > nilfs->ns_sbwtime[0] + NILFS_SB_FREQ;
+	return t < nilfs->ns_sbwtime || t > nilfs->ns_sbwtime + NILFS_SB_FREQ;
 }
 
-static inline int nilfs_altsb_need_update(struct the_nilfs *nilfs)
+static inline int nilfs_sb_need_swap(struct the_nilfs *nilfs)
 {
-	u64 t = get_seconds();
-	struct nilfs_super_block **sbp = nilfs->ns_sbp;
-	return sbp[1] && t > nilfs->ns_sbwtime[1] + NILFS_ALTSB_FREQ;
+	int flip_bits = nilfs->ns_sbwcount & 0x0FL;
+	return (flip_bits != 0x08 && flip_bits != 0x0F);
 }
 
 void nilfs_set_last_segment(struct the_nilfs *, sector_t, u64, __u64);
-- 
1.5.6.5

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 0/2] nilfs2: asynchronous sb update

[Ryusuke Konishi]

Hi,
On Fri, 25 Jun 2010 00:27:38 +0900, Jiro SEKIBA wrote:
> Hi,
> 
> This is v4 patch series to update super block asynchronously.
> 
> First patch separates super block validity check from nilfs_commit_super,
> and moved the function into callers.
> 
> Second patch introduces swapping code in nilfs_prepare_super.
> Caller of the function must specify whether swapping super blocks or not
> explicitly.  To advance log pointer, the caller uses nilfs_set_log_cursor
> to advance specified super block after calling nilfs_prepare_super.
> 
> changes from v3
> - move swapping code to nilfs_prepare_super from nilfs_sync_super
> - check last_cno of super blocks to advance protection period
> - introduce nilfs_sb_need_swap function
> 
> changes from v2
> - restore super block in nilfs_prepare_super
> - introduce nilfs_set_error
>  * used in nilfs_error
> - introduce nilfs_set_log_cursor instead of write_log_cursor
>  * instead of writing error, set log cursor to specified sb pointer
>  * used followed by nilfs_commit_super
> - introduce ns_sbwcount
>  * swap super blocks based on the counter
>  * swap all most all the time, still asymmetric(9:7 ratio)
> - delete ns_sbwtime[2]
>  * introduce ns_sbwtime instead of array of both super blocks
> - synchronize super blocks when mount/remount
> - abandon NILFS_SB_SET_FN
>  * only used in nilfs_set_error, so assign directly in the function
> - misc style fix
> - misc comment fix
> 
> changes from v1
> - change macro name NILFS_SUPER_SET_FN -> NILFS_SB_SET_FN
>  * change inline function name as nilfs_sb_set_##name
> - introduce enum for nilfs_commit_super and nilfs_sync_super
>  * NILFS_SB_COMMIT and NILFS_SB_COMMIT_ALL
>  * use flag instead of dupsb in nilfs_commit_super/sync_super
> 
>  fs/nilfs2/nilfs.h     |   12 +++
>  fs/nilfs2/segment.c   |   10 ++-
>  fs/nilfs2/super.c     |  182 ++++++++++++++++++++++++++++++++++---------------
>  fs/nilfs2/the_nilfs.c |   17 +++--
>  fs/nilfs2/the_nilfs.h |   17 ++---
>  5 files changed, 166 insertions(+), 72 deletions(-)

Thank you for the post.

I've just sent a complementary patchset for your previous series.

I will review this new one tomorrow.

Regards,
Ryusuke Konishi

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 2/2] nilfs2: sync super blocks in turns

[Ryusuke Konishi]

On Fri, 25 Jun 2010 00:27:40 +0900, Jiro SEKIBA wrote:
> This will sync super blocks in turns instead of syncing duplicate
> super blocks at the time.  This will help searching valid super root when
> super block is written into disk before log is written, which is happen when
> barrier-less block devices are unmounted uncleanly.
> In the situation, old super block likely points to valid log.
> 
> This patch introduces ns_sbwcount member, which counts how many times super
> blocks write back to the disk.  Newly introduced nilfs_sb_need_swap() function
> decides whether flipping required or not based on the ns_sbwcount to sync
> super blocks asymmetrically.
> 
> nilfs_prepare_super swaps super blocks according to the argument.
> The argument is calculated by nilfs_sb_need_swap() function.
> 
> The patch also introduces new function nilfs_set_log_cursor to advance
> log cursor for specified super block.  To update both of super block
> information, caller of nilfs_commit_super must set the information on both
> super blocks.
> 
> Signed-off-by: Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>

May I change the function name of "nilfs_sb_need_swap" to
"nilfs_sb_will_flip" or something?

"need swap" looks confusing at first glance.

Regards,
Ryusuke Konishi

> ---
>  fs/nilfs2/nilfs.h     |   13 +++++-
>  fs/nilfs2/segment.c   |   10 +++-
>  fs/nilfs2/super.c     |  131 ++++++++++++++++++++++++++++++++-----------------
>  fs/nilfs2/the_nilfs.c |   10 ++--
>  fs/nilfs2/the_nilfs.h |   17 +++---
>  5 files changed, 119 insertions(+), 62 deletions(-)
> 
> diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
> index 649e079..63c8fd4 100644
> --- a/fs/nilfs2/nilfs.h
> +++ b/fs/nilfs2/nilfs.h
> @@ -107,6 +107,14 @@ enum {
>  };
>  
>  /*
> + * commit flags for nilfs_commit_super and nilfs_sync_super
> + */
> +enum {
> +	NILFS_SB_COMMIT = 0,	/* Commit a super block alternately */
> +	NILFS_SB_COMMIT_ALL	/* Commit both super blocks */
> +};
> +
> +/*
>   * Macros to check inode numbers
>   */
>  #define NILFS_MDT_INO_BITS   \
> @@ -270,7 +278,10 @@ extern struct nilfs_super_block *
>  nilfs_read_super_block(struct super_block *, u64, int, struct buffer_head **);
>  extern int nilfs_store_magic_and_option(struct super_block *,
>  					struct nilfs_super_block *, char *);
> -extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *);
> +extern void nilfs_set_log_cursor(struct nilfs_super_block *,
> +				 struct the_nilfs *);
> +extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *,
> +						      int flip);
>  extern int nilfs_commit_super(struct nilfs_sb_info *, int);
>  extern int nilfs_attach_checkpoint(struct nilfs_sb_info *, __u64);
>  extern void nilfs_detach_checkpoint(struct nilfs_sb_info *);
> diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
> index 075d7b0..844743e 100644
> --- a/fs/nilfs2/segment.c
> +++ b/fs/nilfs2/segment.c
> @@ -2408,6 +2408,7 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
>  {
>  	struct nilfs_sb_info *sbi = sci->sc_sbi;
>  	struct the_nilfs *nilfs = sbi->s_nilfs;
> +	struct nilfs_super_block **sbp;
>  	int err = 0;
>  
>  	nilfs_segctor_accept(sci);
> @@ -2424,9 +2425,12 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
>  		    nilfs_discontinued(nilfs)) {
>  			down_write(&nilfs->ns_sem);
>  			err = -EIO;
> -			if (likely(nilfs_prepare_super(sbi)))
> -				err = nilfs_commit_super(
> -					sbi, nilfs_altsb_need_update(nilfs));
> +			sbp = nilfs_prepare_super(sbi,
> +						  nilfs_sb_need_swap(nilfs));
> +			if (likely(sbp)) {
> +				nilfs_set_log_cursor(sbp[0], nilfs);
> +				err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> +			}
>  			up_write(&nilfs->ns_sem);
>  		}
>  	}
> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> index 045b8d7..cd35f73 100644
> --- a/fs/nilfs2/super.c
> +++ b/fs/nilfs2/super.c
> @@ -74,6 +74,25 @@ struct kmem_cache *nilfs_btree_path_cache;
>  
>  static int nilfs_remount(struct super_block *sb, int *flags, char *data);
>  
> +static void nilfs_set_error(struct nilfs_sb_info *sbi)
> +{
> +	struct the_nilfs *nilfs = sbi->s_nilfs;
> +	struct nilfs_super_block **sbp;
> +
> +	down_write(&nilfs->ns_sem);
> +	if (!(nilfs->ns_mount_state & NILFS_ERROR_FS)) {
> +		nilfs->ns_mount_state |= NILFS_ERROR_FS;
> +		sbp = nilfs_prepare_super(sbi, 0);
> +		if (likely(sbp)) {
> +			sbp[0]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> +			if (sbp[1])
> +				sbp[1]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> +			nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
> +		}
> +	}
> +	up_write(&nilfs->ns_sem);
> +}
> +
>  /**
>   * nilfs_error() - report failure condition on a filesystem
>   *
> @@ -90,7 +109,6 @@ void nilfs_error(struct super_block *sb, const char *function,
>  		 const char *fmt, ...)
>  {
>  	struct nilfs_sb_info *sbi = NILFS_SB(sb);
> -	struct nilfs_super_block **sbp;
>  	va_list args;
>  
>  	va_start(args, fmt);
> @@ -100,18 +118,7 @@ void nilfs_error(struct super_block *sb, const char *function,
>  	va_end(args);
>  
>  	if (!(sb->s_flags & MS_RDONLY)) {
> -		struct the_nilfs *nilfs = sbi->s_nilfs;
> -
> -		down_write(&nilfs->ns_sem);
> -		if (!(nilfs->ns_mount_state & NILFS_ERROR_FS)) {
> -			nilfs->ns_mount_state |= NILFS_ERROR_FS;
> -			sbp = nilfs_prepare_super(sbi);
> -			if (likely(sbp)) {
> -				sbp[0]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> -				nilfs_commit_super(sbi, 1);
> -			}
> -		}
> -		up_write(&nilfs->ns_sem);
> +		nilfs_set_error(sbi);
>  
>  		if (nilfs_test_opt(sbi, ERRORS_RO)) {
>  			printk(KERN_CRIT "Remounting filesystem read-only\n");
> @@ -179,7 +186,7 @@ static void nilfs_clear_inode(struct inode *inode)
>  	nilfs_btnode_cache_clear(&ii->i_btnode_cache);
>  }
>  
> -static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> +static int nilfs_sync_super(struct nilfs_sb_info *sbi, int flag)
>  {
>  	struct the_nilfs *nilfs = sbi->s_nilfs;
>  	int err;
> @@ -205,12 +212,20 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
>  		printk(KERN_ERR
>  		       "NILFS: unable to write superblock (err=%d)\n", err);
>  		if (err == -EIO && nilfs->ns_sbh[1]) {
> +			/*
> +			 * sbp[0] points to newer log than sbp[1],
> +			 * so copy sbp[0] to sbp[1] to take over sbp[0].
> +			 */
> +			memcpy(nilfs->ns_sbp[1], nilfs->ns_sbp[0],
> +			       nilfs->ns_sbsize);
>  			nilfs_fall_back_super_block(nilfs);
>  			goto retry;
>  		}
>  	} else {
>  		struct nilfs_super_block *sbp = nilfs->ns_sbp[0];
>  
> +		nilfs->ns_sbwcount++;
> +
>  		/*
>  		 * The latest segment becomes trailable from the position
>  		 * written in superblock.
> @@ -220,11 +235,14 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
>  		/* update GC protection for recent segments */
>  		if (nilfs->ns_sbh[1]) {
>  			sbp = NULL;
> -			if (dupsb) {
> +			if (flag == NILFS_SB_COMMIT_ALL) {
>  				set_buffer_dirty(nilfs->ns_sbh[1]);
>  				if (!sync_dirty_buffer(nilfs->ns_sbh[1]))
>  					sbp = nilfs->ns_sbp[1];
>  			}
> +			if (sbp &&
> +			    nilfs->ns_sbp[0]->s_last_cno < sbp->s_last_cno)
> +				sbp = nilfs->ns_sbp[0];
>  		}
>  		if (sbp) {
>  			spin_lock(&nilfs->ns_last_segment_lock);
> @@ -236,7 +254,8 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
>  	return err;
>  }
>  
> -struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
> +struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi,
> +					       int flip)
>  {
>  	struct the_nilfs *nilfs = sbi->s_nilfs;
>  	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> @@ -245,50 +264,62 @@ struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
>  	if (sbp[0]->s_magic != cpu_to_le16(NILFS_SUPER_MAGIC)) {
>  		if (sbp[1] &&
>  		    sbp[1]->s_magic == cpu_to_le16(NILFS_SUPER_MAGIC)) {
> -			nilfs_swap_super_block(nilfs);
> +			memcpy(sbp[0], sbp[1], nilfs->ns_sbsize);
>  		} else {
>  			printk(KERN_CRIT "NILFS: superblock broke on dev %s\n",
>  			       sbi->s_super->s_id);
>  			return NULL;
>  		}
> +	} else if (sbp[1] &&
> +		   sbp[1]->s_magic != cpu_to_le16(NILFS_SUPER_MAGIC)) {
> +			memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
>  	}
> +
> +	if (flip && sbp[1])
> +		nilfs_swap_super_block(nilfs);
> +
>  	return sbp;
>  }
>  
> -int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
> +void nilfs_set_log_cursor(struct nilfs_super_block *sbp,
> +			  struct the_nilfs *nilfs)
>  {
> -	struct the_nilfs *nilfs = sbi->s_nilfs;
> -	struct nilfs_super_block **sbp = nilfs->ns_sbp;
>  	sector_t nfreeblocks;
> -	time_t t;
> -	int err;
>  
>  	/* nilfs->ns_sem must be locked by the caller. */
> -	err = nilfs_count_free_blocks(nilfs, &nfreeblocks);
> -	if (unlikely(err)) {
> -		printk(KERN_ERR "NILFS: failed to count free blocks\n");
> -		return err;
> -	}
> +	nilfs_count_free_blocks(nilfs, &nfreeblocks);
> +	sbp->s_free_blocks_count = cpu_to_le64(nfreeblocks);
> +
>  	spin_lock(&nilfs->ns_last_segment_lock);
> -	sbp[0]->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
> -	sbp[0]->s_last_pseg = cpu_to_le64(nilfs->ns_last_pseg);
> -	sbp[0]->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
> +	sbp->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
> +	sbp->s_last_pseg = cpu_to_le64(nilfs->ns_last_pseg);
> +	sbp->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
>  	spin_unlock(&nilfs->ns_last_segment_lock);
> +}
> +
> +int nilfs_commit_super(struct nilfs_sb_info *sbi, int flag)
> +{
> +	struct the_nilfs *nilfs = sbi->s_nilfs;
> +	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> +	time_t t;
>  
> +	/* nilfs->ns_sem must be locked by the caller. */
>  	t = get_seconds();
> -	nilfs->ns_sbwtime[0] = t;
> -	sbp[0]->s_free_blocks_count = cpu_to_le64(nfreeblocks);
> +	nilfs->ns_sbwtime = t;
>  	sbp[0]->s_wtime = cpu_to_le64(t);
>  	sbp[0]->s_sum = 0;
>  	sbp[0]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
>  					     (unsigned char *)sbp[0],
>  					     nilfs->ns_sbsize));
> -	if (dupsb && sbp[1]) {
> -		memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> -		nilfs->ns_sbwtime[1] = t;
> +	if (flag == NILFS_SB_COMMIT_ALL && sbp[1]) {
> +		sbp[1]->s_wtime = sbp[0]->s_wtime;
> +		sbp[1]->s_sum = 0;
> +		sbp[1]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
> +					    (unsigned char *)sbp[1],
> +					    nilfs->ns_sbsize));
>  	}
>  	clear_nilfs_sb_dirty(nilfs);
> -	return nilfs_sync_super(sbi, dupsb);
> +	return nilfs_sync_super(sbi, flag);
>  }
>  
>  static void nilfs_put_super(struct super_block *sb)
> @@ -303,10 +334,12 @@ static void nilfs_put_super(struct super_block *sb)
>  
>  	if (!(sb->s_flags & MS_RDONLY)) {
>  		down_write(&nilfs->ns_sem);
> -		sbp = nilfs_prepare_super(sbi);
> +		sbp = nilfs_prepare_super(sbi, 0);
>  		if (likely(sbp)) {
> +			/* set state only for newer super block */
>  			sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
> -			nilfs_commit_super(sbi, 1);
> +			nilfs_set_log_cursor(sbp[0], nilfs);
> +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
>  		}
>  		up_write(&nilfs->ns_sem);
>  	}
> @@ -328,6 +361,7 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
>  {
>  	struct nilfs_sb_info *sbi = NILFS_SB(sb);
>  	struct the_nilfs *nilfs = sbi->s_nilfs;
> +	struct nilfs_super_block **sbp;
>  	int err = 0;
>  
>  	/* This function is called when super block should be written back */
> @@ -335,8 +369,13 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
>  		err = nilfs_construct_segment(sb);
>  
>  	down_write(&nilfs->ns_sem);
> -	if (nilfs_sb_dirty(nilfs) && nilfs_prepare_super(sbi))
> -		nilfs_commit_super(sbi, 1);
> +	if (nilfs_sb_dirty(nilfs)) {
> +		sbp = nilfs_prepare_super(sbi, nilfs_sb_need_swap(nilfs));
> +		if (likely(sbp)) {
> +			nilfs_set_log_cursor(sbp[0], nilfs);
> +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> +		}
> +	}
>  	up_write(&nilfs->ns_sem);
>  
>  	return err;
> @@ -635,14 +674,13 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
>  	int mnt_count;
>  
>  	/* nilfs->ns_sem must be locked by the caller. */
> -	sbp = nilfs_prepare_super(sbi);
> +	sbp = nilfs_prepare_super(sbi, 0);
>  	if (!sbp)
>  		return -EIO;
>  
>  	max_mnt_count = le16_to_cpu(sbp[0]->s_max_mnt_count);
>  	mnt_count = le16_to_cpu(sbp[0]->s_mnt_count);
>  
> -	/* nilfs->ns_sem must be locked by the caller. */
>  	if (nilfs->ns_mount_state & NILFS_ERROR_FS) {
>  		printk(KERN_WARNING
>  		       "NILFS warning: mounting fs with errors\n");
> @@ -659,7 +697,9 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
>  	sbp[0]->s_state =
>  		cpu_to_le16(le16_to_cpu(sbp[0]->s_state) & ~NILFS_VALID_FS);
>  	sbp[0]->s_mtime = cpu_to_le64(get_seconds());
> -	return nilfs_commit_super(sbi, 1);
> +	/* synchronize sbp[1] with sbp[0] */
> +	memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> +	return nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
>  }
>  
>  struct nilfs_super_block *nilfs_read_super_block(struct super_block *sb,
> @@ -906,14 +946,15 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
>  		 * the RDONLY flag and then mark the partition as valid again.
>  		 */
>  		down_write(&nilfs->ns_sem);
> -		sbp = nilfs_prepare_super(sbi);
> +		sbp = nilfs_prepare_super(sbi, 0);
>  		if (likely(sbp)) {
>  			if (!(sbp[0]->s_state & le16_to_cpu(NILFS_VALID_FS)) &&
>  			    (nilfs->ns_mount_state & NILFS_VALID_FS))
>  				sbp[0]->s_state =
>  					cpu_to_le16(nilfs->ns_mount_state);
>  			sbp[0]->s_mtime = cpu_to_le64(get_seconds());
> -			nilfs_commit_super(sbi, 1);
> +			nilfs_set_log_cursor(sbp[0], nilfs);
> +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
>  		}
>  		up_write(&nilfs->ns_sem);
>  	} else {
> diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
> index 74b0480..7fdb780 100644
> --- a/fs/nilfs2/the_nilfs.c
> +++ b/fs/nilfs2/the_nilfs.c
> @@ -326,11 +326,13 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
>  
>  	down_write(&nilfs->ns_sem);
>  	err = -EIO;
> -	sbp = nilfs_prepare_super(sbi);
> +	sbp = nilfs_prepare_super(sbi, 0);
>  	if (likely(sbp)) {
>  		nilfs->ns_mount_state |= NILFS_VALID_FS;
> +		/* set the flag only for newer super block */
>  		sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
> -		err = nilfs_commit_super(sbi, 1);
> +		nilfs_set_log_cursor(sbp[0], nilfs);
> +		err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
>  	}
>  	up_write(&nilfs->ns_sem);
>  
> @@ -519,8 +521,8 @@ static int nilfs_load_super_block(struct the_nilfs *nilfs,
>  		nilfs_swap_super_block(nilfs);
>  	}
>  
> -	nilfs->ns_sbwtime[0] = le64_to_cpu(sbp[0]->s_wtime);
> -	nilfs->ns_sbwtime[1] = valid[!swp] ? le64_to_cpu(sbp[1]->s_wtime) : 0;
> +	nilfs->ns_sbwcount = 0;
> +	nilfs->ns_sbwtime = le64_to_cpu(sbp[0]->s_wtime);
>  	nilfs->ns_prot_seq = le64_to_cpu(sbp[valid[1] & !swp]->s_last_seq);
>  	*sbpp = sbp[0];
>  	return 0;
> diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
> index 85df47f..0a44688 100644
> --- a/fs/nilfs2/the_nilfs.h
> +++ b/fs/nilfs2/the_nilfs.h
> @@ -57,7 +57,8 @@ enum {
>   * @ns_current: back pointer to current mount
>   * @ns_sbh: buffer heads of on-disk super blocks
>   * @ns_sbp: pointers to super block data
> - * @ns_sbwtime: previous write time of super blocks
> + * @ns_sbwtime: previous write time of super block
> + * @ns_sbwcount: write count of super block
>   * @ns_sbsize: size of valid data in super block
>   * @ns_supers: list of nilfs super block structs
>   * @ns_seg_seq: segment sequence counter
> @@ -120,7 +121,8 @@ struct the_nilfs {
>  	 */
>  	struct buffer_head     *ns_sbh[2];
>  	struct nilfs_super_block *ns_sbp[2];
> -	time_t			ns_sbwtime[2];
> +	time_t			ns_sbwtime;
> +	unsigned		ns_sbwcount;
>  	unsigned		ns_sbsize;
>  	unsigned		ns_mount_state;
>  
> @@ -205,20 +207,17 @@ THE_NILFS_FNS(SB_DIRTY, sb_dirty)
>  
>  /* Minimum interval of periodical update of superblocks (in seconds) */
>  #define NILFS_SB_FREQ		10
> -#define NILFS_ALTSB_FREQ	60  /* spare superblock */
>  
>  static inline int nilfs_sb_need_update(struct the_nilfs *nilfs)
>  {
>  	u64 t = get_seconds();
> -	return t < nilfs->ns_sbwtime[0] ||
> -		 t > nilfs->ns_sbwtime[0] + NILFS_SB_FREQ;
> +	return t < nilfs->ns_sbwtime || t > nilfs->ns_sbwtime + NILFS_SB_FREQ;
>  }
>  
> -static inline int nilfs_altsb_need_update(struct the_nilfs *nilfs)
> +static inline int nilfs_sb_need_swap(struct the_nilfs *nilfs)
>  {
> -	u64 t = get_seconds();
> -	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> -	return sbp[1] && t > nilfs->ns_sbwtime[1] + NILFS_ALTSB_FREQ;
> +	int flip_bits = nilfs->ns_sbwcount & 0x0FL;
> +	return (flip_bits != 0x08 && flip_bits != 0x0F);
>  }
>  
>  void nilfs_set_last_segment(struct the_nilfs *, sector_t, u64, __u64);
> -- 
> 1.5.6.5
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 2/2] nilfs2: sync super blocks in turns

[Jiro SEKIBA]

Hi, thank you for the comments.

At Fri, 25 Jun 2010 18:06:39 +0900 (JST),
Ryusuke Konishi wrote:
> 
> On Fri, 25 Jun 2010 00:27:40 +0900, Jiro SEKIBA wrote:
> > This will sync super blocks in turns instead of syncing duplicate
> > super blocks at the time.  This will help searching valid super root when
> > super block is written into disk before log is written, which is happen when
> > barrier-less block devices are unmounted uncleanly.
> > In the situation, old super block likely points to valid log.
> > 
> > This patch introduces ns_sbwcount member, which counts how many times super
> > blocks write back to the disk.  Newly introduced nilfs_sb_need_swap() function
> > decides whether flipping required or not based on the ns_sbwcount to sync
> > super blocks asymmetrically.
> > 
> > nilfs_prepare_super swaps super blocks according to the argument.
> > The argument is calculated by nilfs_sb_need_swap() function.
> > 
> > The patch also introduces new function nilfs_set_log_cursor to advance
> > log cursor for specified super block.  To update both of super block
> > information, caller of nilfs_commit_super must set the information on both
> > super blocks.
> > 
> > Signed-off-by: Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
> 
> May I change the function name of "nilfs_sb_need_swap" to
> "nilfs_sb_will_flip" or something?
> 
> "need swap" looks confusing at first glance.

OK, I'll change it nilfs_sb_will_flip.
if you have any further comments, please let me know

thanks,

regards,

> Regards,
> Ryusuke Konishi
> 
> > ---
> >  fs/nilfs2/nilfs.h     |   13 +++++-
> >  fs/nilfs2/segment.c   |   10 +++-
> >  fs/nilfs2/super.c     |  131 ++++++++++++++++++++++++++++++++-----------------
> >  fs/nilfs2/the_nilfs.c |   10 ++--
> >  fs/nilfs2/the_nilfs.h |   17 +++---
> >  5 files changed, 119 insertions(+), 62 deletions(-)
> > 
> > diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
> > index 649e079..63c8fd4 100644
> > --- a/fs/nilfs2/nilfs.h
> > +++ b/fs/nilfs2/nilfs.h
> > @@ -107,6 +107,14 @@ enum {
> >  };
> >  
> >  /*
> > + * commit flags for nilfs_commit_super and nilfs_sync_super
> > + */
> > +enum {
> > +	NILFS_SB_COMMIT = 0,	/* Commit a super block alternately */
> > +	NILFS_SB_COMMIT_ALL	/* Commit both super blocks */
> > +};
> > +
> > +/*
> >   * Macros to check inode numbers
> >   */
> >  #define NILFS_MDT_INO_BITS   \
> > @@ -270,7 +278,10 @@ extern struct nilfs_super_block *
> >  nilfs_read_super_block(struct super_block *, u64, int, struct buffer_head **);
> >  extern int nilfs_store_magic_and_option(struct super_block *,
> >  					struct nilfs_super_block *, char *);
> > -extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *);
> > +extern void nilfs_set_log_cursor(struct nilfs_super_block *,
> > +				 struct the_nilfs *);
> > +extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *,
> > +						      int flip);
> >  extern int nilfs_commit_super(struct nilfs_sb_info *, int);
> >  extern int nilfs_attach_checkpoint(struct nilfs_sb_info *, __u64);
> >  extern void nilfs_detach_checkpoint(struct nilfs_sb_info *);
> > diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
> > index 075d7b0..844743e 100644
> > --- a/fs/nilfs2/segment.c
> > +++ b/fs/nilfs2/segment.c
> > @@ -2408,6 +2408,7 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
> >  {
> >  	struct nilfs_sb_info *sbi = sci->sc_sbi;
> >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> > +	struct nilfs_super_block **sbp;
> >  	int err = 0;
> >  
> >  	nilfs_segctor_accept(sci);
> > @@ -2424,9 +2425,12 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
> >  		    nilfs_discontinued(nilfs)) {
> >  			down_write(&nilfs->ns_sem);
> >  			err = -EIO;
> > -			if (likely(nilfs_prepare_super(sbi)))
> > -				err = nilfs_commit_super(
> > -					sbi, nilfs_altsb_need_update(nilfs));
> > +			sbp = nilfs_prepare_super(sbi,
> > +						  nilfs_sb_need_swap(nilfs));
> > +			if (likely(sbp)) {
> > +				nilfs_set_log_cursor(sbp[0], nilfs);
> > +				err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > +			}
> >  			up_write(&nilfs->ns_sem);
> >  		}
> >  	}
> > diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> > index 045b8d7..cd35f73 100644
> > --- a/fs/nilfs2/super.c
> > +++ b/fs/nilfs2/super.c
> > @@ -74,6 +74,25 @@ struct kmem_cache *nilfs_btree_path_cache;
> >  
> >  static int nilfs_remount(struct super_block *sb, int *flags, char *data);
> >  
> > +static void nilfs_set_error(struct nilfs_sb_info *sbi)
> > +{
> > +	struct the_nilfs *nilfs = sbi->s_nilfs;
> > +	struct nilfs_super_block **sbp;
> > +
> > +	down_write(&nilfs->ns_sem);
> > +	if (!(nilfs->ns_mount_state & NILFS_ERROR_FS)) {
> > +		nilfs->ns_mount_state |= NILFS_ERROR_FS;
> > +		sbp = nilfs_prepare_super(sbi, 0);
> > +		if (likely(sbp)) {
> > +			sbp[0]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> > +			if (sbp[1])
> > +				sbp[1]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
> > +		}
> > +	}
> > +	up_write(&nilfs->ns_sem);
> > +}
> > +
> >  /**
> >   * nilfs_error() - report failure condition on a filesystem
> >   *
> > @@ -90,7 +109,6 @@ void nilfs_error(struct super_block *sb, const char *function,
> >  		 const char *fmt, ...)
> >  {
> >  	struct nilfs_sb_info *sbi = NILFS_SB(sb);
> > -	struct nilfs_super_block **sbp;
> >  	va_list args;
> >  
> >  	va_start(args, fmt);
> > @@ -100,18 +118,7 @@ void nilfs_error(struct super_block *sb, const char *function,
> >  	va_end(args);
> >  
> >  	if (!(sb->s_flags & MS_RDONLY)) {
> > -		struct the_nilfs *nilfs = sbi->s_nilfs;
> > -
> > -		down_write(&nilfs->ns_sem);
> > -		if (!(nilfs->ns_mount_state & NILFS_ERROR_FS)) {
> > -			nilfs->ns_mount_state |= NILFS_ERROR_FS;
> > -			sbp = nilfs_prepare_super(sbi);
> > -			if (likely(sbp)) {
> > -				sbp[0]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> > -				nilfs_commit_super(sbi, 1);
> > -			}
> > -		}
> > -		up_write(&nilfs->ns_sem);
> > +		nilfs_set_error(sbi);
> >  
> >  		if (nilfs_test_opt(sbi, ERRORS_RO)) {
> >  			printk(KERN_CRIT "Remounting filesystem read-only\n");
> > @@ -179,7 +186,7 @@ static void nilfs_clear_inode(struct inode *inode)
> >  	nilfs_btnode_cache_clear(&ii->i_btnode_cache);
> >  }
> >  
> > -static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> > +static int nilfs_sync_super(struct nilfs_sb_info *sbi, int flag)
> >  {
> >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> >  	int err;
> > @@ -205,12 +212,20 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> >  		printk(KERN_ERR
> >  		       "NILFS: unable to write superblock (err=%d)\n", err);
> >  		if (err == -EIO && nilfs->ns_sbh[1]) {
> > +			/*
> > +			 * sbp[0] points to newer log than sbp[1],
> > +			 * so copy sbp[0] to sbp[1] to take over sbp[0].
> > +			 */
> > +			memcpy(nilfs->ns_sbp[1], nilfs->ns_sbp[0],
> > +			       nilfs->ns_sbsize);
> >  			nilfs_fall_back_super_block(nilfs);
> >  			goto retry;
> >  		}
> >  	} else {
> >  		struct nilfs_super_block *sbp = nilfs->ns_sbp[0];
> >  
> > +		nilfs->ns_sbwcount++;
> > +
> >  		/*
> >  		 * The latest segment becomes trailable from the position
> >  		 * written in superblock.
> > @@ -220,11 +235,14 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> >  		/* update GC protection for recent segments */
> >  		if (nilfs->ns_sbh[1]) {
> >  			sbp = NULL;
> > -			if (dupsb) {
> > +			if (flag == NILFS_SB_COMMIT_ALL) {
> >  				set_buffer_dirty(nilfs->ns_sbh[1]);
> >  				if (!sync_dirty_buffer(nilfs->ns_sbh[1]))
> >  					sbp = nilfs->ns_sbp[1];
> >  			}
> > +			if (sbp &&
> > +			    nilfs->ns_sbp[0]->s_last_cno < sbp->s_last_cno)
> > +				sbp = nilfs->ns_sbp[0];
> >  		}
> >  		if (sbp) {
> >  			spin_lock(&nilfs->ns_last_segment_lock);
> > @@ -236,7 +254,8 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> >  	return err;
> >  }
> >  
> > -struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
> > +struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi,
> > +					       int flip)
> >  {
> >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> >  	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> > @@ -245,50 +264,62 @@ struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
> >  	if (sbp[0]->s_magic != cpu_to_le16(NILFS_SUPER_MAGIC)) {
> >  		if (sbp[1] &&
> >  		    sbp[1]->s_magic == cpu_to_le16(NILFS_SUPER_MAGIC)) {
> > -			nilfs_swap_super_block(nilfs);
> > +			memcpy(sbp[0], sbp[1], nilfs->ns_sbsize);
> >  		} else {
> >  			printk(KERN_CRIT "NILFS: superblock broke on dev %s\n",
> >  			       sbi->s_super->s_id);
> >  			return NULL;
> >  		}
> > +	} else if (sbp[1] &&
> > +		   sbp[1]->s_magic != cpu_to_le16(NILFS_SUPER_MAGIC)) {
> > +			memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> >  	}
> > +
> > +	if (flip && sbp[1])
> > +		nilfs_swap_super_block(nilfs);
> > +
> >  	return sbp;
> >  }
> >  
> > -int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
> > +void nilfs_set_log_cursor(struct nilfs_super_block *sbp,
> > +			  struct the_nilfs *nilfs)
> >  {
> > -	struct the_nilfs *nilfs = sbi->s_nilfs;
> > -	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> >  	sector_t nfreeblocks;
> > -	time_t t;
> > -	int err;
> >  
> >  	/* nilfs->ns_sem must be locked by the caller. */
> > -	err = nilfs_count_free_blocks(nilfs, &nfreeblocks);
> > -	if (unlikely(err)) {
> > -		printk(KERN_ERR "NILFS: failed to count free blocks\n");
> > -		return err;
> > -	}
> > +	nilfs_count_free_blocks(nilfs, &nfreeblocks);
> > +	sbp->s_free_blocks_count = cpu_to_le64(nfreeblocks);
> > +
> >  	spin_lock(&nilfs->ns_last_segment_lock);
> > -	sbp[0]->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
> > -	sbp[0]->s_last_pseg = cpu_to_le64(nilfs->ns_last_pseg);
> > -	sbp[0]->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
> > +	sbp->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
> > +	sbp->s_last_pseg = cpu_to_le64(nilfs->ns_last_pseg);
> > +	sbp->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
> >  	spin_unlock(&nilfs->ns_last_segment_lock);
> > +}
> > +
> > +int nilfs_commit_super(struct nilfs_sb_info *sbi, int flag)
> > +{
> > +	struct the_nilfs *nilfs = sbi->s_nilfs;
> > +	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> > +	time_t t;
> >  
> > +	/* nilfs->ns_sem must be locked by the caller. */
> >  	t = get_seconds();
> > -	nilfs->ns_sbwtime[0] = t;
> > -	sbp[0]->s_free_blocks_count = cpu_to_le64(nfreeblocks);
> > +	nilfs->ns_sbwtime = t;
> >  	sbp[0]->s_wtime = cpu_to_le64(t);
> >  	sbp[0]->s_sum = 0;
> >  	sbp[0]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
> >  					     (unsigned char *)sbp[0],
> >  					     nilfs->ns_sbsize));
> > -	if (dupsb && sbp[1]) {
> > -		memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> > -		nilfs->ns_sbwtime[1] = t;
> > +	if (flag == NILFS_SB_COMMIT_ALL && sbp[1]) {
> > +		sbp[1]->s_wtime = sbp[0]->s_wtime;
> > +		sbp[1]->s_sum = 0;
> > +		sbp[1]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
> > +					    (unsigned char *)sbp[1],
> > +					    nilfs->ns_sbsize));
> >  	}
> >  	clear_nilfs_sb_dirty(nilfs);
> > -	return nilfs_sync_super(sbi, dupsb);
> > +	return nilfs_sync_super(sbi, flag);
> >  }
> >  
> >  static void nilfs_put_super(struct super_block *sb)
> > @@ -303,10 +334,12 @@ static void nilfs_put_super(struct super_block *sb)
> >  
> >  	if (!(sb->s_flags & MS_RDONLY)) {
> >  		down_write(&nilfs->ns_sem);
> > -		sbp = nilfs_prepare_super(sbi);
> > +		sbp = nilfs_prepare_super(sbi, 0);
> >  		if (likely(sbp)) {
> > +			/* set state only for newer super block */
> >  			sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
> > -			nilfs_commit_super(sbi, 1);
> > +			nilfs_set_log_cursor(sbp[0], nilfs);
> > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> >  		}
> >  		up_write(&nilfs->ns_sem);
> >  	}
> > @@ -328,6 +361,7 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
> >  {
> >  	struct nilfs_sb_info *sbi = NILFS_SB(sb);
> >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> > +	struct nilfs_super_block **sbp;
> >  	int err = 0;
> >  
> >  	/* This function is called when super block should be written back */
> > @@ -335,8 +369,13 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
> >  		err = nilfs_construct_segment(sb);
> >  
> >  	down_write(&nilfs->ns_sem);
> > -	if (nilfs_sb_dirty(nilfs) && nilfs_prepare_super(sbi))
> > -		nilfs_commit_super(sbi, 1);
> > +	if (nilfs_sb_dirty(nilfs)) {
> > +		sbp = nilfs_prepare_super(sbi, nilfs_sb_need_swap(nilfs));
> > +		if (likely(sbp)) {
> > +			nilfs_set_log_cursor(sbp[0], nilfs);
> > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > +		}
> > +	}
> >  	up_write(&nilfs->ns_sem);
> >  
> >  	return err;
> > @@ -635,14 +674,13 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
> >  	int mnt_count;
> >  
> >  	/* nilfs->ns_sem must be locked by the caller. */
> > -	sbp = nilfs_prepare_super(sbi);
> > +	sbp = nilfs_prepare_super(sbi, 0);
> >  	if (!sbp)
> >  		return -EIO;
> >  
> >  	max_mnt_count = le16_to_cpu(sbp[0]->s_max_mnt_count);
> >  	mnt_count = le16_to_cpu(sbp[0]->s_mnt_count);
> >  
> > -	/* nilfs->ns_sem must be locked by the caller. */
> >  	if (nilfs->ns_mount_state & NILFS_ERROR_FS) {
> >  		printk(KERN_WARNING
> >  		       "NILFS warning: mounting fs with errors\n");
> > @@ -659,7 +697,9 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
> >  	sbp[0]->s_state =
> >  		cpu_to_le16(le16_to_cpu(sbp[0]->s_state) & ~NILFS_VALID_FS);
> >  	sbp[0]->s_mtime = cpu_to_le64(get_seconds());
> > -	return nilfs_commit_super(sbi, 1);
> > +	/* synchronize sbp[1] with sbp[0] */
> > +	memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> > +	return nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
> >  }
> >  
> >  struct nilfs_super_block *nilfs_read_super_block(struct super_block *sb,
> > @@ -906,14 +946,15 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
> >  		 * the RDONLY flag and then mark the partition as valid again.
> >  		 */
> >  		down_write(&nilfs->ns_sem);
> > -		sbp = nilfs_prepare_super(sbi);
> > +		sbp = nilfs_prepare_super(sbi, 0);
> >  		if (likely(sbp)) {
> >  			if (!(sbp[0]->s_state & le16_to_cpu(NILFS_VALID_FS)) &&
> >  			    (nilfs->ns_mount_state & NILFS_VALID_FS))
> >  				sbp[0]->s_state =
> >  					cpu_to_le16(nilfs->ns_mount_state);
> >  			sbp[0]->s_mtime = cpu_to_le64(get_seconds());
> > -			nilfs_commit_super(sbi, 1);
> > +			nilfs_set_log_cursor(sbp[0], nilfs);
> > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> >  		}
> >  		up_write(&nilfs->ns_sem);
> >  	} else {
> > diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
> > index 74b0480..7fdb780 100644
> > --- a/fs/nilfs2/the_nilfs.c
> > +++ b/fs/nilfs2/the_nilfs.c
> > @@ -326,11 +326,13 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
> >  
> >  	down_write(&nilfs->ns_sem);
> >  	err = -EIO;
> > -	sbp = nilfs_prepare_super(sbi);
> > +	sbp = nilfs_prepare_super(sbi, 0);
> >  	if (likely(sbp)) {
> >  		nilfs->ns_mount_state |= NILFS_VALID_FS;
> > +		/* set the flag only for newer super block */
> >  		sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
> > -		err = nilfs_commit_super(sbi, 1);
> > +		nilfs_set_log_cursor(sbp[0], nilfs);
> > +		err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> >  	}
> >  	up_write(&nilfs->ns_sem);
> >  
> > @@ -519,8 +521,8 @@ static int nilfs_load_super_block(struct the_nilfs *nilfs,
> >  		nilfs_swap_super_block(nilfs);
> >  	}
> >  
> > -	nilfs->ns_sbwtime[0] = le64_to_cpu(sbp[0]->s_wtime);
> > -	nilfs->ns_sbwtime[1] = valid[!swp] ? le64_to_cpu(sbp[1]->s_wtime) : 0;
> > +	nilfs->ns_sbwcount = 0;
> > +	nilfs->ns_sbwtime = le64_to_cpu(sbp[0]->s_wtime);
> >  	nilfs->ns_prot_seq = le64_to_cpu(sbp[valid[1] & !swp]->s_last_seq);
> >  	*sbpp = sbp[0];
> >  	return 0;
> > diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
> > index 85df47f..0a44688 100644
> > --- a/fs/nilfs2/the_nilfs.h
> > +++ b/fs/nilfs2/the_nilfs.h
> > @@ -57,7 +57,8 @@ enum {
> >   * @ns_current: back pointer to current mount
> >   * @ns_sbh: buffer heads of on-disk super blocks
> >   * @ns_sbp: pointers to super block data
> > - * @ns_sbwtime: previous write time of super blocks
> > + * @ns_sbwtime: previous write time of super block
> > + * @ns_sbwcount: write count of super block
> >   * @ns_sbsize: size of valid data in super block
> >   * @ns_supers: list of nilfs super block structs
> >   * @ns_seg_seq: segment sequence counter
> > @@ -120,7 +121,8 @@ struct the_nilfs {
> >  	 */
> >  	struct buffer_head     *ns_sbh[2];
> >  	struct nilfs_super_block *ns_sbp[2];
> > -	time_t			ns_sbwtime[2];
> > +	time_t			ns_sbwtime;
> > +	unsigned		ns_sbwcount;
> >  	unsigned		ns_sbsize;
> >  	unsigned		ns_mount_state;
> >  
> > @@ -205,20 +207,17 @@ THE_NILFS_FNS(SB_DIRTY, sb_dirty)
> >  
> >  /* Minimum interval of periodical update of superblocks (in seconds) */
> >  #define NILFS_SB_FREQ		10
> > -#define NILFS_ALTSB_FREQ	60  /* spare superblock */
> >  
> >  static inline int nilfs_sb_need_update(struct the_nilfs *nilfs)
> >  {
> >  	u64 t = get_seconds();
> > -	return t < nilfs->ns_sbwtime[0] ||
> > -		 t > nilfs->ns_sbwtime[0] + NILFS_SB_FREQ;
> > +	return t < nilfs->ns_sbwtime || t > nilfs->ns_sbwtime + NILFS_SB_FREQ;
> >  }
> >  
> > -static inline int nilfs_altsb_need_update(struct the_nilfs *nilfs)
> > +static inline int nilfs_sb_need_swap(struct the_nilfs *nilfs)
> >  {
> > -	u64 t = get_seconds();
> > -	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> > -	return sbp[1] && t > nilfs->ns_sbwtime[1] + NILFS_ALTSB_FREQ;
> > +	int flip_bits = nilfs->ns_sbwcount & 0x0FL;
> > +	return (flip_bits != 0x08 && flip_bits != 0x0F);
> >  }
> >  
> >  void nilfs_set_last_segment(struct the_nilfs *, sector_t, u64, __u64);
> > -- 
> > 1.5.6.5
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 


-- 
Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

CentOS 5 support for nilfs

[Martin, Nick]

Hello,		

I have been watching the list and web site for some time, hoping for an update to the CentOS 5 download.
It does not yet include the utilities released on 10 April.

I have a 2.0 TB nilfs file system allocated on an iSCSI SAN volume with about 1.5 TB used.  
It holds about 1.3 million current files and lots of snapshots.  It is being accessed using CentOS 5 (x86_64).
I have preserved daily snapshots since February.  The nilfs file system is working about as expected.
I cannot attribute any crashes or data loss to nilfs.

Although there is no shortage of available free space, the nilfs cleaner produces 400 to 800 IOPS all day long to this iSCSI SAN volume.
I would really appreciate the feature that the cleaner goes to sleep when there is no free space shortage.
This would lighten the load on the iSCSI SAN and leave more bandwidth for other SAN volumes.

I attempted to build the utilities myself from source on CentOS 5, but was not successful.
There seemed to be dependencies I could not resolve.
Can someone post the directions and dependencies to allow me to build it myself?
Alternatively, please let me know when such a build is likely to appear on the downloads page.

I manage the shared file servers for a group of about 20 developers building software that runs on Linux.
Despite the warning messages, I am using nilfs in a production environment as an archival/backup mechanism.
It is far superior to my previous solution which used symbolic links to create daily snapshots without duplicating unchanged files.
I am doing weekly full backups to tape as well, but the efficiency of nilfs allowed me to effectively keep a daily full backup on disk for every day for 4 months. I am now trimming snapshots older than two months down to 2 per month to delay running out of space.  

The next unreleased feature I anticipate needing is the ability to expand a nilfs file system.  
I would now increase my nilfs file system to 3 TB if I could.

Thanks to all who contribute to this useful, and in my experience stable project.
 
Thank you in advance for any help getting current utilities for CentOS. 

I would like to contribute to your project in some way.  
I have some ideas of how I might be able to help.  
I would be happy to explore this offline with someone.

Nick Martin

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 2/2] nilfs2: sync super blocks in turns

[Ryusuke Konishi]

Hi,
On Sat, 26 Jun 2010 14:05:31 +0900, Jiro SEKIBA wrote:
> Hi, thank you for the comments.
> 
> At Fri, 25 Jun 2010 18:06:39 +0900 (JST),
> Ryusuke Konishi wrote:
> > 
> > On Fri, 25 Jun 2010 00:27:40 +0900, Jiro SEKIBA wrote:
> > > This will sync super blocks in turns instead of syncing duplicate
> > > super blocks at the time.  This will help searching valid super root when
> > > super block is written into disk before log is written, which is happen when
> > > barrier-less block devices are unmounted uncleanly.
> > > In the situation, old super block likely points to valid log.
> > > 
> > > This patch introduces ns_sbwcount member, which counts how many times super
> > > blocks write back to the disk.  Newly introduced nilfs_sb_need_swap() function
> > > decides whether flipping required or not based on the ns_sbwcount to sync
> > > super blocks asymmetrically.
> > > 
> > > nilfs_prepare_super swaps super blocks according to the argument.
> > > The argument is calculated by nilfs_sb_need_swap() function.
> > > 
> > > The patch also introduces new function nilfs_set_log_cursor to advance
> > > log cursor for specified super block.  To update both of super block
> > > information, caller of nilfs_commit_super must set the information on both
> > > super blocks.
> > > 
> > > Signed-off-by: Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
> > 
> > May I change the function name of "nilfs_sb_need_swap" to
> > "nilfs_sb_will_flip" or something?
> > 
> > "need swap" looks confusing at first glance.
> 
> OK, I'll change it nilfs_sb_will_flip.
> if you have any further comments, please let me know

I found two issues.  Please see the following inline comment, and
correct them together.

> thanks,
> 
> regards,
> 
> > Regards,
> > Ryusuke Konishi
> > 
> > > ---
> > >  fs/nilfs2/nilfs.h     |   13 +++++-
> > >  fs/nilfs2/segment.c   |   10 +++-
> > >  fs/nilfs2/super.c     |  131 ++++++++++++++++++++++++++++++++-----------------
> > >  fs/nilfs2/the_nilfs.c |   10 ++--
> > >  fs/nilfs2/the_nilfs.h |   17 +++---
> > >  5 files changed, 119 insertions(+), 62 deletions(-)
> > > 
> > > diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
> > > index 649e079..63c8fd4 100644
> > > --- a/fs/nilfs2/nilfs.h
> > > +++ b/fs/nilfs2/nilfs.h
> > > @@ -107,6 +107,14 @@ enum {
> > >  };
> > >  
> > >  /*
> > > + * commit flags for nilfs_commit_super and nilfs_sync_super
> > > + */
> > > +enum {
> > > +	NILFS_SB_COMMIT = 0,	/* Commit a super block alternately */
> > > +	NILFS_SB_COMMIT_ALL	/* Commit both super blocks */
> > > +};
> > > +
> > > +/*
> > >   * Macros to check inode numbers
> > >   */
> > >  #define NILFS_MDT_INO_BITS   \
> > > @@ -270,7 +278,10 @@ extern struct nilfs_super_block *
> > >  nilfs_read_super_block(struct super_block *, u64, int, struct buffer_head **);
> > >  extern int nilfs_store_magic_and_option(struct super_block *,
> > >  					struct nilfs_super_block *, char *);
> > > -extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *);
> > > +extern void nilfs_set_log_cursor(struct nilfs_super_block *,
> > > +				 struct the_nilfs *);
> > > +extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *,
> > > +						      int flip);
> > >  extern int nilfs_commit_super(struct nilfs_sb_info *, int);
> > >  extern int nilfs_attach_checkpoint(struct nilfs_sb_info *, __u64);
> > >  extern void nilfs_detach_checkpoint(struct nilfs_sb_info *);
> > > diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
> > > index 075d7b0..844743e 100644
> > > --- a/fs/nilfs2/segment.c
> > > +++ b/fs/nilfs2/segment.c
> > > @@ -2408,6 +2408,7 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
> > >  {
> > >  	struct nilfs_sb_info *sbi = sci->sc_sbi;
> > >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > +	struct nilfs_super_block **sbp;
> > >  	int err = 0;
> > >  
> > >  	nilfs_segctor_accept(sci);
> > > @@ -2424,9 +2425,12 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
> > >  		    nilfs_discontinued(nilfs)) {
> > >  			down_write(&nilfs->ns_sem);
> > >  			err = -EIO;
> > > -			if (likely(nilfs_prepare_super(sbi)))
> > > -				err = nilfs_commit_super(
> > > -					sbi, nilfs_altsb_need_update(nilfs));
> > > +			sbp = nilfs_prepare_super(sbi,
> > > +						  nilfs_sb_need_swap(nilfs));
> > > +			if (likely(sbp)) {
> > > +				nilfs_set_log_cursor(sbp[0], nilfs);
> > > +				err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > > +			}
> > >  			up_write(&nilfs->ns_sem);
> > >  		}
> > >  	}
> > > diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> > > index 045b8d7..cd35f73 100644
> > > --- a/fs/nilfs2/super.c
> > > +++ b/fs/nilfs2/super.c
> > > @@ -74,6 +74,25 @@ struct kmem_cache *nilfs_btree_path_cache;
> > >  
> > >  static int nilfs_remount(struct super_block *sb, int *flags, char *data);
> > >  
> > > +static void nilfs_set_error(struct nilfs_sb_info *sbi)
> > > +{
> > > +	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > +	struct nilfs_super_block **sbp;
> > > +
> > > +	down_write(&nilfs->ns_sem);
> > > +	if (!(nilfs->ns_mount_state & NILFS_ERROR_FS)) {
> > > +		nilfs->ns_mount_state |= NILFS_ERROR_FS;
> > > +		sbp = nilfs_prepare_super(sbi, 0);
> > > +		if (likely(sbp)) {
> > > +			sbp[0]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> > > +			if (sbp[1])
> > > +				sbp[1]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> > > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
> > > +		}
> > > +	}
> > > +	up_write(&nilfs->ns_sem);
> > > +}
> > > +
> > >  /**
> > >   * nilfs_error() - report failure condition on a filesystem
> > >   *
> > > @@ -90,7 +109,6 @@ void nilfs_error(struct super_block *sb, const char *function,
> > >  		 const char *fmt, ...)
> > >  {
> > >  	struct nilfs_sb_info *sbi = NILFS_SB(sb);
> > > -	struct nilfs_super_block **sbp;
> > >  	va_list args;
> > >  
> > >  	va_start(args, fmt);
> > > @@ -100,18 +118,7 @@ void nilfs_error(struct super_block *sb, const char *function,
> > >  	va_end(args);
> > >  
> > >  	if (!(sb->s_flags & MS_RDONLY)) {
> > > -		struct the_nilfs *nilfs = sbi->s_nilfs;
> > > -
> > > -		down_write(&nilfs->ns_sem);
> > > -		if (!(nilfs->ns_mount_state & NILFS_ERROR_FS)) {
> > > -			nilfs->ns_mount_state |= NILFS_ERROR_FS;
> > > -			sbp = nilfs_prepare_super(sbi);
> > > -			if (likely(sbp)) {
> > > -				sbp[0]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> > > -				nilfs_commit_super(sbi, 1);
> > > -			}
> > > -		}
> > > -		up_write(&nilfs->ns_sem);
> > > +		nilfs_set_error(sbi);
> > >  
> > >  		if (nilfs_test_opt(sbi, ERRORS_RO)) {
> > >  			printk(KERN_CRIT "Remounting filesystem read-only\n");
> > > @@ -179,7 +186,7 @@ static void nilfs_clear_inode(struct inode *inode)
> > >  	nilfs_btnode_cache_clear(&ii->i_btnode_cache);
> > >  }
> > >  
> > > -static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> > > +static int nilfs_sync_super(struct nilfs_sb_info *sbi, int flag)
> > >  {
> > >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> > >  	int err;
> > > @@ -205,12 +212,20 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> > >  		printk(KERN_ERR
> > >  		       "NILFS: unable to write superblock (err=%d)\n", err);
> > >  		if (err == -EIO && nilfs->ns_sbh[1]) {
> > > +			/*
> > > +			 * sbp[0] points to newer log than sbp[1],
> > > +			 * so copy sbp[0] to sbp[1] to take over sbp[0].
> > > +			 */
> > > +			memcpy(nilfs->ns_sbp[1], nilfs->ns_sbp[0],
> > > +			       nilfs->ns_sbsize);
> > >  			nilfs_fall_back_super_block(nilfs);
> > >  			goto retry;
> > >  		}
> > >  	} else {
> > >  		struct nilfs_super_block *sbp = nilfs->ns_sbp[0];
> > >  
> > > +		nilfs->ns_sbwcount++;
> > > +
> > >  		/*
> > >  		 * The latest segment becomes trailable from the position
> > >  		 * written in superblock.
> > > @@ -220,11 +235,14 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> > >  		/* update GC protection for recent segments */
> > >  		if (nilfs->ns_sbh[1]) {
> > >  			sbp = NULL;
> > > -			if (dupsb) {
> > > +			if (flag == NILFS_SB_COMMIT_ALL) {
> > >  				set_buffer_dirty(nilfs->ns_sbh[1]);
> > >  				if (!sync_dirty_buffer(nilfs->ns_sbh[1]))
> > >  					sbp = nilfs->ns_sbp[1];
> > >  			}
> > > +			if (sbp &&
> > > +			    nilfs->ns_sbp[0]->s_last_cno < sbp->s_last_cno)
> > > +				sbp = nilfs->ns_sbp[0];
> > >  		}

Here, endian conversions are required for both s_last_cno references.

We can apply some bit operations without endian conversion (e.g. OR,
AND, XOR, and so on), but it's not true for arithmetic operations like
addition, subtraction and comparison.

And, the above code seems to never update ns_prot_seq when
NILFS_SB_COMMIT is specified.

How about simplifying this section as follows ?

		struct nilfs_super_block *sbp = nilfs->ns_sbp[0];
		...

		if (nilfs->ns_sbh[1]) {
			if (flag == NILFS_SB_COMMIT_ALL) {
				set_buffer_dirty(nilfs->ns_sbh[1]);
				if (sync_dirty_buffer(nilfs->ns_sbh[1]) < 0)
					goto out;
			}
			if (le64_to_cpu(nilfs->ns_sbp[1]->s_last_cno) <
			    le64_to_cpu(nilfs->ns_sbp[0]->s_last_cno))
				sbp = nilfs->ns_sbp[1];
		}

		spin_lock(&nilfs->ns_last_segment_lock);
		nilfs->ns_prot_seq = le64_to_cpu(sbp->s_last_seq);
		spin_unlock(&nilfs->ns_last_segment_lock);
	}
 out:
	return err;
}

> > >  		if (sbp) {
> > >  			spin_lock(&nilfs->ns_last_segment_lock);
> > > @@ -236,7 +254,8 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> > >  	return err;
> > >  }
> > >  
> > > -struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
> > > +struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi,
> > > +					       int flip)
> > >  {
> > >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> > >  	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> > > @@ -245,50 +264,62 @@ struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
> > >  	if (sbp[0]->s_magic != cpu_to_le16(NILFS_SUPER_MAGIC)) {
> > >  		if (sbp[1] &&
> > >  		    sbp[1]->s_magic == cpu_to_le16(NILFS_SUPER_MAGIC)) {
> > > -			nilfs_swap_super_block(nilfs);
> > > +			memcpy(sbp[0], sbp[1], nilfs->ns_sbsize);
> > >  		} else {
> > >  			printk(KERN_CRIT "NILFS: superblock broke on dev %s\n",
> > >  			       sbi->s_super->s_id);
> > >  			return NULL;
> > >  		}
> > > +	} else if (sbp[1] &&
> > > +		   sbp[1]->s_magic != cpu_to_le16(NILFS_SUPER_MAGIC)) {
> > > +			memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> > >  	}
> > > +
> > > +	if (flip && sbp[1])
> > > +		nilfs_swap_super_block(nilfs);
> > > +
> > >  	return sbp;
> > >  }
> > >  
> > > -int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
> > > +void nilfs_set_log_cursor(struct nilfs_super_block *sbp,
> > > +			  struct the_nilfs *nilfs)
> > >  {
> > > -	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > -	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> > >  	sector_t nfreeblocks;
> > > -	time_t t;
> > > -	int err;
> > >  
> > >  	/* nilfs->ns_sem must be locked by the caller. */
> > > -	err = nilfs_count_free_blocks(nilfs, &nfreeblocks);
> > > -	if (unlikely(err)) {
> > > -		printk(KERN_ERR "NILFS: failed to count free blocks\n");
> > > -		return err;
> > > -	}
> > > +	nilfs_count_free_blocks(nilfs, &nfreeblocks);
> > > +	sbp->s_free_blocks_count = cpu_to_le64(nfreeblocks);
> > > +
> > >  	spin_lock(&nilfs->ns_last_segment_lock);
> > > -	sbp[0]->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
> > > -	sbp[0]->s_last_pseg = cpu_to_le64(nilfs->ns_last_pseg);
> > > -	sbp[0]->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
> > > +	sbp->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
> > > +	sbp->s_last_pseg = cpu_to_le64(nilfs->ns_last_pseg);
> > > +	sbp->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
> > >  	spin_unlock(&nilfs->ns_last_segment_lock);
> > > +}
> > > +
> > > +int nilfs_commit_super(struct nilfs_sb_info *sbi, int flag)
> > > +{
> > > +	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > +	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> > > +	time_t t;
> > >  
> > > +	/* nilfs->ns_sem must be locked by the caller. */
> > >  	t = get_seconds();
> > > -	nilfs->ns_sbwtime[0] = t;
> > > -	sbp[0]->s_free_blocks_count = cpu_to_le64(nfreeblocks);
> > > +	nilfs->ns_sbwtime = t;
> > >  	sbp[0]->s_wtime = cpu_to_le64(t);
> > >  	sbp[0]->s_sum = 0;
> > >  	sbp[0]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
> > >  					     (unsigned char *)sbp[0],
> > >  					     nilfs->ns_sbsize));
> > > -	if (dupsb && sbp[1]) {
> > > -		memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> > > -		nilfs->ns_sbwtime[1] = t;
> > > +	if (flag == NILFS_SB_COMMIT_ALL && sbp[1]) {
> > > +		sbp[1]->s_wtime = sbp[0]->s_wtime;
> > > +		sbp[1]->s_sum = 0;
> > > +		sbp[1]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
> > > +					    (unsigned char *)sbp[1],
> > > +					    nilfs->ns_sbsize));
> > >  	}
> > >  	clear_nilfs_sb_dirty(nilfs);
> > > -	return nilfs_sync_super(sbi, dupsb);
> > > +	return nilfs_sync_super(sbi, flag);
> > >  }
> > >  
> > >  static void nilfs_put_super(struct super_block *sb)
> > > @@ -303,10 +334,12 @@ static void nilfs_put_super(struct super_block *sb)
> > >  
> > >  	if (!(sb->s_flags & MS_RDONLY)) {
> > >  		down_write(&nilfs->ns_sem);
> > > -		sbp = nilfs_prepare_super(sbi);
> > > +		sbp = nilfs_prepare_super(sbi, 0);
> > >  		if (likely(sbp)) {
> > > +			/* set state only for newer super block */
> > >  			sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
> > > -			nilfs_commit_super(sbi, 1);
> > > +			nilfs_set_log_cursor(sbp[0], nilfs);
> > > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > >  		}
> > >  		up_write(&nilfs->ns_sem);
> > >  	}
> > > @@ -328,6 +361,7 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
> > >  {
> > >  	struct nilfs_sb_info *sbi = NILFS_SB(sb);
> > >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > +	struct nilfs_super_block **sbp;
> > >  	int err = 0;
> > >  
> > >  	/* This function is called when super block should be written back */
> > > @@ -335,8 +369,13 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
> > >  		err = nilfs_construct_segment(sb);
> > >  
> > >  	down_write(&nilfs->ns_sem);
> > > -	if (nilfs_sb_dirty(nilfs) && nilfs_prepare_super(sbi))
> > > -		nilfs_commit_super(sbi, 1);
> > > +	if (nilfs_sb_dirty(nilfs)) {
> > > +		sbp = nilfs_prepare_super(sbi, nilfs_sb_need_swap(nilfs));
> > > +		if (likely(sbp)) {
> > > +			nilfs_set_log_cursor(sbp[0], nilfs);
> > > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > > +		}
> > > +	}
> > >  	up_write(&nilfs->ns_sem);
> > >  
> > >  	return err;
> > > @@ -635,14 +674,13 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
> > >  	int mnt_count;
> > >  
> > >  	/* nilfs->ns_sem must be locked by the caller. */
> > > -	sbp = nilfs_prepare_super(sbi);
> > > +	sbp = nilfs_prepare_super(sbi, 0);
> > >  	if (!sbp)
> > >  		return -EIO;
> > >  
> > >  	max_mnt_count = le16_to_cpu(sbp[0]->s_max_mnt_count);
> > >  	mnt_count = le16_to_cpu(sbp[0]->s_mnt_count);
> > >  
> > > -	/* nilfs->ns_sem must be locked by the caller. */
> > >  	if (nilfs->ns_mount_state & NILFS_ERROR_FS) {
> > >  		printk(KERN_WARNING
> > >  		       "NILFS warning: mounting fs with errors\n");
> > > @@ -659,7 +697,9 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
> > >  	sbp[0]->s_state =
> > >  		cpu_to_le16(le16_to_cpu(sbp[0]->s_state) & ~NILFS_VALID_FS);
> > >  	sbp[0]->s_mtime = cpu_to_le64(get_seconds());
> > > -	return nilfs_commit_super(sbi, 1);
> > > +	/* synchronize sbp[1] with sbp[0] */
> > > +	memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> > > +	return nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
> > >  }
> > >  
> > >  struct nilfs_super_block *nilfs_read_super_block(struct super_block *sb,
> > > @@ -906,14 +946,15 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
> > >  		 * the RDONLY flag and then mark the partition as valid again.
> > >  		 */
> > >  		down_write(&nilfs->ns_sem);
> > > -		sbp = nilfs_prepare_super(sbi);
> > > +		sbp = nilfs_prepare_super(sbi, 0);
> > >  		if (likely(sbp)) {
> > >  			if (!(sbp[0]->s_state & le16_to_cpu(NILFS_VALID_FS)) &&
> > >  			    (nilfs->ns_mount_state & NILFS_VALID_FS))
> > >  				sbp[0]->s_state =
> > >  					cpu_to_le16(nilfs->ns_mount_state);
> > >  			sbp[0]->s_mtime = cpu_to_le64(get_seconds());
> > > -			nilfs_commit_super(sbi, 1);
> > > +			nilfs_set_log_cursor(sbp[0], nilfs);
> > > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > >  		}
> > >  		up_write(&nilfs->ns_sem);
> > >  	} else {
> > > diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
> > > index 74b0480..7fdb780 100644
> > > --- a/fs/nilfs2/the_nilfs.c
> > > +++ b/fs/nilfs2/the_nilfs.c
> > > @@ -326,11 +326,13 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
> > >  
> > >  	down_write(&nilfs->ns_sem);
> > >  	err = -EIO;
> > > -	sbp = nilfs_prepare_super(sbi);
> > > +	sbp = nilfs_prepare_super(sbi, 0);
> > >  	if (likely(sbp)) {
> > >  		nilfs->ns_mount_state |= NILFS_VALID_FS;
> > > +		/* set the flag only for newer super block */
> > >  		sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
> > > -		err = nilfs_commit_super(sbi, 1);
> > > +		nilfs_set_log_cursor(sbp[0], nilfs);
> > > +		err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > >  	}
> > >  	up_write(&nilfs->ns_sem);
> > >  
> > > @@ -519,8 +521,8 @@ static int nilfs_load_super_block(struct the_nilfs *nilfs,
> > >  		nilfs_swap_super_block(nilfs);
> > >  	}
> > >  
> > > -	nilfs->ns_sbwtime[0] = le64_to_cpu(sbp[0]->s_wtime);
> > > -	nilfs->ns_sbwtime[1] = valid[!swp] ? le64_to_cpu(sbp[1]->s_wtime) : 0;
> > > +	nilfs->ns_sbwcount = 0;
> > > +	nilfs->ns_sbwtime = le64_to_cpu(sbp[0]->s_wtime);
> > >  	nilfs->ns_prot_seq = le64_to_cpu(sbp[valid[1] & !swp]->s_last_seq);
> > >  	*sbpp = sbp[0];
> > >  	return 0;
> > > diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
> > > index 85df47f..0a44688 100644
> > > --- a/fs/nilfs2/the_nilfs.h
> > > +++ b/fs/nilfs2/the_nilfs.h
> > > @@ -57,7 +57,8 @@ enum {
> > >   * @ns_current: back pointer to current mount
> > >   * @ns_sbh: buffer heads of on-disk super blocks
> > >   * @ns_sbp: pointers to super block data
> > > - * @ns_sbwtime: previous write time of super blocks
> > > + * @ns_sbwtime: previous write time of super block
> > > + * @ns_sbwcount: write count of super block
> > >   * @ns_sbsize: size of valid data in super block
> > >   * @ns_supers: list of nilfs super block structs
> > >   * @ns_seg_seq: segment sequence counter
> > > @@ -120,7 +121,8 @@ struct the_nilfs {
> > >  	 */
> > >  	struct buffer_head     *ns_sbh[2];
> > >  	struct nilfs_super_block *ns_sbp[2];
> > > -	time_t			ns_sbwtime[2];
> > > +	time_t			ns_sbwtime;
> > > +	unsigned		ns_sbwcount;
> > >  	unsigned		ns_sbsize;
> > >  	unsigned		ns_mount_state;
> > >  
> > > @@ -205,20 +207,17 @@ THE_NILFS_FNS(SB_DIRTY, sb_dirty)
> > >  
> > >  /* Minimum interval of periodical update of superblocks (in seconds) */
> > >  #define NILFS_SB_FREQ		10
> > > -#define NILFS_ALTSB_FREQ	60  /* spare superblock */
> > >  
> > >  static inline int nilfs_sb_need_update(struct the_nilfs *nilfs)
> > >  {
> > >  	u64 t = get_seconds();
> > > -	return t < nilfs->ns_sbwtime[0] ||
> > > -		 t > nilfs->ns_sbwtime[0] + NILFS_SB_FREQ;
> > > +	return t < nilfs->ns_sbwtime || t > nilfs->ns_sbwtime + NILFS_SB_FREQ;
> > >  }
> > >  
> > > -static inline int nilfs_altsb_need_update(struct the_nilfs *nilfs)
> > > +static inline int nilfs_sb_need_swap(struct the_nilfs *nilfs)
> > >  {
> > > -	u64 t = get_seconds();
> > > -	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> > > -	return sbp[1] && t > nilfs->ns_sbwtime[1] + NILFS_ALTSB_FREQ;
> > > +	int flip_bits = nilfs->ns_sbwcount & 0x0FL;
> > > +	return (flip_bits != 0x08 && flip_bits != 0x0F);
> > >  }
> > >  
> > >  void nilfs_set_last_segment(struct the_nilfs *, sector_t, u64, __u64);
> > > -- 
> > > 1.5.6.5
> > > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> > 
> 
> 
> -- 
> Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>


Regards,
Ryusuke Konishi
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: CentOS 5 support for nilfs

[Ryusuke Konishi]

Hi,
On Sat, 26 Jun 2010 02:10:17 -0500, "Martin, Nick" wrote:
> Hello,		
> 
> I have been watching the list and web site for some time, hoping for an update to the CentOS 5 download.
> It does not yet include the utilities released on 10 April.

I'm using the following packages for the CentOS 5 series.
Or, do you have any problems for them ?

  http://www.nilfs.org/pub/centos/5/RPMS/i386/nilfs-utils-2.0.18-3.i386.rpm
  http://www.nilfs.org/pub/centos/5/RPMS/x86_64/nilfs-utils-2.0.18-3.x86_64.rpm

These include the release on April 10.

Regards,
Ryusuke Konishi
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 2/2] nilfs2: sync super blocks in turns

[Jiro SEKIBA]

Hi,

At Sat, 26 Jun 2010 23:51:08 +0900 (JST),
Ryusuke Konishi wrote:
> 
> Hi,
> On Sat, 26 Jun 2010 14:05:31 +0900, Jiro SEKIBA wrote:
> > Hi, thank you for the comments.
> > 
> > At Fri, 25 Jun 2010 18:06:39 +0900 (JST),
> > Ryusuke Konishi wrote:
> > > 
> > > On Fri, 25 Jun 2010 00:27:40 +0900, Jiro SEKIBA wrote:
> > > > This will sync super blocks in turns instead of syncing duplicate
> > > > super blocks at the time.  This will help searching valid super root when
> > > > super block is written into disk before log is written, which is happen when
> > > > barrier-less block devices are unmounted uncleanly.
> > > > In the situation, old super block likely points to valid log.
> > > > 
> > > > This patch introduces ns_sbwcount member, which counts how many times super
> > > > blocks write back to the disk.  Newly introduced nilfs_sb_need_swap() function
> > > > decides whether flipping required or not based on the ns_sbwcount to sync
> > > > super blocks asymmetrically.
> > > > 
> > > > nilfs_prepare_super swaps super blocks according to the argument.
> > > > The argument is calculated by nilfs_sb_need_swap() function.
> > > > 
> > > > The patch also introduces new function nilfs_set_log_cursor to advance
> > > > log cursor for specified super block.  To update both of super block
> > > > information, caller of nilfs_commit_super must set the information on both
> > > > super blocks.
> > > > 
> > > > Signed-off-by: Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
> > > 
> > > May I change the function name of "nilfs_sb_need_swap" to
> > > "nilfs_sb_will_flip" or something?
> > > 
> > > "need swap" looks confusing at first glance.
> > 
> > OK, I'll change it nilfs_sb_will_flip.
> > if you have any further comments, please let me know
> 
> I found two issues.  Please see the following inline comment, and
> correct them together.
> 
> > thanks,
> > 
> > regards,
> > 
> > > Regards,
> > > Ryusuke Konishi
> > > 
> > > > ---
> > > >  fs/nilfs2/nilfs.h     |   13 +++++-
> > > >  fs/nilfs2/segment.c   |   10 +++-
> > > >  fs/nilfs2/super.c     |  131 ++++++++++++++++++++++++++++++++-----------------
> > > >  fs/nilfs2/the_nilfs.c |   10 ++--
> > > >  fs/nilfs2/the_nilfs.h |   17 +++---
> > > >  5 files changed, 119 insertions(+), 62 deletions(-)
> > > > 
> > > > diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
> > > > index 649e079..63c8fd4 100644
> > > > --- a/fs/nilfs2/nilfs.h
> > > > +++ b/fs/nilfs2/nilfs.h
> > > > @@ -107,6 +107,14 @@ enum {
> > > >  };
> > > >  
> > > >  /*
> > > > + * commit flags for nilfs_commit_super and nilfs_sync_super
> > > > + */
> > > > +enum {
> > > > +	NILFS_SB_COMMIT = 0,	/* Commit a super block alternately */
> > > > +	NILFS_SB_COMMIT_ALL	/* Commit both super blocks */
> > > > +};
> > > > +
> > > > +/*
> > > >   * Macros to check inode numbers
> > > >   */
> > > >  #define NILFS_MDT_INO_BITS   \
> > > > @@ -270,7 +278,10 @@ extern struct nilfs_super_block *
> > > >  nilfs_read_super_block(struct super_block *, u64, int, struct buffer_head **);
> > > >  extern int nilfs_store_magic_and_option(struct super_block *,
> > > >  					struct nilfs_super_block *, char *);
> > > > -extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *);
> > > > +extern void nilfs_set_log_cursor(struct nilfs_super_block *,
> > > > +				 struct the_nilfs *);
> > > > +extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *,
> > > > +						      int flip);
> > > >  extern int nilfs_commit_super(struct nilfs_sb_info *, int);
> > > >  extern int nilfs_attach_checkpoint(struct nilfs_sb_info *, __u64);
> > > >  extern void nilfs_detach_checkpoint(struct nilfs_sb_info *);
> > > > diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
> > > > index 075d7b0..844743e 100644
> > > > --- a/fs/nilfs2/segment.c
> > > > +++ b/fs/nilfs2/segment.c
> > > > @@ -2408,6 +2408,7 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
> > > >  {
> > > >  	struct nilfs_sb_info *sbi = sci->sc_sbi;
> > > >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > > +	struct nilfs_super_block **sbp;
> > > >  	int err = 0;
> > > >  
> > > >  	nilfs_segctor_accept(sci);
> > > > @@ -2424,9 +2425,12 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
> > > >  		    nilfs_discontinued(nilfs)) {
> > > >  			down_write(&nilfs->ns_sem);
> > > >  			err = -EIO;
> > > > -			if (likely(nilfs_prepare_super(sbi)))
> > > > -				err = nilfs_commit_super(
> > > > -					sbi, nilfs_altsb_need_update(nilfs));
> > > > +			sbp = nilfs_prepare_super(sbi,
> > > > +						  nilfs_sb_need_swap(nilfs));
> > > > +			if (likely(sbp)) {
> > > > +				nilfs_set_log_cursor(sbp[0], nilfs);
> > > > +				err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > > > +			}
> > > >  			up_write(&nilfs->ns_sem);
> > > >  		}
> > > >  	}
> > > > diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> > > > index 045b8d7..cd35f73 100644
> > > > --- a/fs/nilfs2/super.c
> > > > +++ b/fs/nilfs2/super.c
> > > > @@ -74,6 +74,25 @@ struct kmem_cache *nilfs_btree_path_cache;
> > > >  
> > > >  static int nilfs_remount(struct super_block *sb, int *flags, char *data);
> > > >  
> > > > +static void nilfs_set_error(struct nilfs_sb_info *sbi)
> > > > +{
> > > > +	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > > +	struct nilfs_super_block **sbp;
> > > > +
> > > > +	down_write(&nilfs->ns_sem);
> > > > +	if (!(nilfs->ns_mount_state & NILFS_ERROR_FS)) {
> > > > +		nilfs->ns_mount_state |= NILFS_ERROR_FS;
> > > > +		sbp = nilfs_prepare_super(sbi, 0);
> > > > +		if (likely(sbp)) {
> > > > +			sbp[0]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> > > > +			if (sbp[1])
> > > > +				sbp[1]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> > > > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
> > > > +		}
> > > > +	}
> > > > +	up_write(&nilfs->ns_sem);
> > > > +}
> > > > +
> > > >  /**
> > > >   * nilfs_error() - report failure condition on a filesystem
> > > >   *
> > > > @@ -90,7 +109,6 @@ void nilfs_error(struct super_block *sb, const char *function,
> > > >  		 const char *fmt, ...)
> > > >  {
> > > >  	struct nilfs_sb_info *sbi = NILFS_SB(sb);
> > > > -	struct nilfs_super_block **sbp;
> > > >  	va_list args;
> > > >  
> > > >  	va_start(args, fmt);
> > > > @@ -100,18 +118,7 @@ void nilfs_error(struct super_block *sb, const char *function,
> > > >  	va_end(args);
> > > >  
> > > >  	if (!(sb->s_flags & MS_RDONLY)) {
> > > > -		struct the_nilfs *nilfs = sbi->s_nilfs;
> > > > -
> > > > -		down_write(&nilfs->ns_sem);
> > > > -		if (!(nilfs->ns_mount_state & NILFS_ERROR_FS)) {
> > > > -			nilfs->ns_mount_state |= NILFS_ERROR_FS;
> > > > -			sbp = nilfs_prepare_super(sbi);
> > > > -			if (likely(sbp)) {
> > > > -				sbp[0]->s_state |= cpu_to_le16(NILFS_ERROR_FS);
> > > > -				nilfs_commit_super(sbi, 1);
> > > > -			}
> > > > -		}
> > > > -		up_write(&nilfs->ns_sem);
> > > > +		nilfs_set_error(sbi);
> > > >  
> > > >  		if (nilfs_test_opt(sbi, ERRORS_RO)) {
> > > >  			printk(KERN_CRIT "Remounting filesystem read-only\n");
> > > > @@ -179,7 +186,7 @@ static void nilfs_clear_inode(struct inode *inode)
> > > >  	nilfs_btnode_cache_clear(&ii->i_btnode_cache);
> > > >  }
> > > >  
> > > > -static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> > > > +static int nilfs_sync_super(struct nilfs_sb_info *sbi, int flag)
> > > >  {
> > > >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > >  	int err;
> > > > @@ -205,12 +212,20 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> > > >  		printk(KERN_ERR
> > > >  		       "NILFS: unable to write superblock (err=%d)\n", err);
> > > >  		if (err == -EIO && nilfs->ns_sbh[1]) {
> > > > +			/*
> > > > +			 * sbp[0] points to newer log than sbp[1],
> > > > +			 * so copy sbp[0] to sbp[1] to take over sbp[0].
> > > > +			 */
> > > > +			memcpy(nilfs->ns_sbp[1], nilfs->ns_sbp[0],
> > > > +			       nilfs->ns_sbsize);
> > > >  			nilfs_fall_back_super_block(nilfs);
> > > >  			goto retry;
> > > >  		}
> > > >  	} else {
> > > >  		struct nilfs_super_block *sbp = nilfs->ns_sbp[0];
> > > >  
> > > > +		nilfs->ns_sbwcount++;
> > > > +
> > > >  		/*
> > > >  		 * The latest segment becomes trailable from the position
> > > >  		 * written in superblock.
> > > > @@ -220,11 +235,14 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> > > >  		/* update GC protection for recent segments */
> > > >  		if (nilfs->ns_sbh[1]) {
> > > >  			sbp = NULL;
> > > > -			if (dupsb) {
> > > > +			if (flag == NILFS_SB_COMMIT_ALL) {
> > > >  				set_buffer_dirty(nilfs->ns_sbh[1]);
> > > >  				if (!sync_dirty_buffer(nilfs->ns_sbh[1]))
> > > >  					sbp = nilfs->ns_sbp[1];
> > > >  			}
> > > > +			if (sbp &&
> > > > +			    nilfs->ns_sbp[0]->s_last_cno < sbp->s_last_cno)
> > > > +				sbp = nilfs->ns_sbp[0];
> > > >  		}
> 
> Here, endian conversions are required for both s_last_cno references.
> 
> We can apply some bit operations without endian conversion (e.g. OR,
> AND, XOR, and so on), but it's not true for arithmetic operations like
> addition, subtraction and comparison.
> 
> And, the above code seems to never update ns_prot_seq when
> NILFS_SB_COMMIT is specified.

Grr, those are really careless bugs...
Thank you for the comments.  I'll revise it

thanks

regards

> How about simplifying this section as follows ?
> 
> 		struct nilfs_super_block *sbp = nilfs->ns_sbp[0];
> 		...
> 
> 		if (nilfs->ns_sbh[1]) {
> 			if (flag == NILFS_SB_COMMIT_ALL) {
> 				set_buffer_dirty(nilfs->ns_sbh[1]);
> 				if (sync_dirty_buffer(nilfs->ns_sbh[1]) < 0)
> 					goto out;
> 			}
> 			if (le64_to_cpu(nilfs->ns_sbp[1]->s_last_cno) <
> 			    le64_to_cpu(nilfs->ns_sbp[0]->s_last_cno))
> 				sbp = nilfs->ns_sbp[1];
> 		}
> 
> 		spin_lock(&nilfs->ns_last_segment_lock);
> 		nilfs->ns_prot_seq = le64_to_cpu(sbp->s_last_seq);
> 		spin_unlock(&nilfs->ns_last_segment_lock);
> 	}
>  out:
> 	return err;
> }
> 
> > > >  		if (sbp) {
> > > >  			spin_lock(&nilfs->ns_last_segment_lock);
> > > > @@ -236,7 +254,8 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
> > > >  	return err;
> > > >  }
> > > >  
> > > > -struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
> > > > +struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi,
> > > > +					       int flip)
> > > >  {
> > > >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > >  	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> > > > @@ -245,50 +264,62 @@ struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
> > > >  	if (sbp[0]->s_magic != cpu_to_le16(NILFS_SUPER_MAGIC)) {
> > > >  		if (sbp[1] &&
> > > >  		    sbp[1]->s_magic == cpu_to_le16(NILFS_SUPER_MAGIC)) {
> > > > -			nilfs_swap_super_block(nilfs);
> > > > +			memcpy(sbp[0], sbp[1], nilfs->ns_sbsize);
> > > >  		} else {
> > > >  			printk(KERN_CRIT "NILFS: superblock broke on dev %s\n",
> > > >  			       sbi->s_super->s_id);
> > > >  			return NULL;
> > > >  		}
> > > > +	} else if (sbp[1] &&
> > > > +		   sbp[1]->s_magic != cpu_to_le16(NILFS_SUPER_MAGIC)) {
> > > > +			memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> > > >  	}
> > > > +
> > > > +	if (flip && sbp[1])
> > > > +		nilfs_swap_super_block(nilfs);
> > > > +
> > > >  	return sbp;
> > > >  }
> > > >  
> > > > -int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
> > > > +void nilfs_set_log_cursor(struct nilfs_super_block *sbp,
> > > > +			  struct the_nilfs *nilfs)
> > > >  {
> > > > -	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > > -	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> > > >  	sector_t nfreeblocks;
> > > > -	time_t t;
> > > > -	int err;
> > > >  
> > > >  	/* nilfs->ns_sem must be locked by the caller. */
> > > > -	err = nilfs_count_free_blocks(nilfs, &nfreeblocks);
> > > > -	if (unlikely(err)) {
> > > > -		printk(KERN_ERR "NILFS: failed to count free blocks\n");
> > > > -		return err;
> > > > -	}
> > > > +	nilfs_count_free_blocks(nilfs, &nfreeblocks);
> > > > +	sbp->s_free_blocks_count = cpu_to_le64(nfreeblocks);
> > > > +
> > > >  	spin_lock(&nilfs->ns_last_segment_lock);
> > > > -	sbp[0]->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
> > > > -	sbp[0]->s_last_pseg = cpu_to_le64(nilfs->ns_last_pseg);
> > > > -	sbp[0]->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
> > > > +	sbp->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
> > > > +	sbp->s_last_pseg = cpu_to_le64(nilfs->ns_last_pseg);
> > > > +	sbp->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
> > > >  	spin_unlock(&nilfs->ns_last_segment_lock);
> > > > +}
> > > > +
> > > > +int nilfs_commit_super(struct nilfs_sb_info *sbi, int flag)
> > > > +{
> > > > +	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > > +	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> > > > +	time_t t;
> > > >  
> > > > +	/* nilfs->ns_sem must be locked by the caller. */
> > > >  	t = get_seconds();
> > > > -	nilfs->ns_sbwtime[0] = t;
> > > > -	sbp[0]->s_free_blocks_count = cpu_to_le64(nfreeblocks);
> > > > +	nilfs->ns_sbwtime = t;
> > > >  	sbp[0]->s_wtime = cpu_to_le64(t);
> > > >  	sbp[0]->s_sum = 0;
> > > >  	sbp[0]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
> > > >  					     (unsigned char *)sbp[0],
> > > >  					     nilfs->ns_sbsize));
> > > > -	if (dupsb && sbp[1]) {
> > > > -		memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> > > > -		nilfs->ns_sbwtime[1] = t;
> > > > +	if (flag == NILFS_SB_COMMIT_ALL && sbp[1]) {
> > > > +		sbp[1]->s_wtime = sbp[0]->s_wtime;
> > > > +		sbp[1]->s_sum = 0;
> > > > +		sbp[1]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
> > > > +					    (unsigned char *)sbp[1],
> > > > +					    nilfs->ns_sbsize));
> > > >  	}
> > > >  	clear_nilfs_sb_dirty(nilfs);
> > > > -	return nilfs_sync_super(sbi, dupsb);
> > > > +	return nilfs_sync_super(sbi, flag);
> > > >  }
> > > >  
> > > >  static void nilfs_put_super(struct super_block *sb)
> > > > @@ -303,10 +334,12 @@ static void nilfs_put_super(struct super_block *sb)
> > > >  
> > > >  	if (!(sb->s_flags & MS_RDONLY)) {
> > > >  		down_write(&nilfs->ns_sem);
> > > > -		sbp = nilfs_prepare_super(sbi);
> > > > +		sbp = nilfs_prepare_super(sbi, 0);
> > > >  		if (likely(sbp)) {
> > > > +			/* set state only for newer super block */
> > > >  			sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
> > > > -			nilfs_commit_super(sbi, 1);
> > > > +			nilfs_set_log_cursor(sbp[0], nilfs);
> > > > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > > >  		}
> > > >  		up_write(&nilfs->ns_sem);
> > > >  	}
> > > > @@ -328,6 +361,7 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
> > > >  {
> > > >  	struct nilfs_sb_info *sbi = NILFS_SB(sb);
> > > >  	struct the_nilfs *nilfs = sbi->s_nilfs;
> > > > +	struct nilfs_super_block **sbp;
> > > >  	int err = 0;
> > > >  
> > > >  	/* This function is called when super block should be written back */
> > > > @@ -335,8 +369,13 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
> > > >  		err = nilfs_construct_segment(sb);
> > > >  
> > > >  	down_write(&nilfs->ns_sem);
> > > > -	if (nilfs_sb_dirty(nilfs) && nilfs_prepare_super(sbi))
> > > > -		nilfs_commit_super(sbi, 1);
> > > > +	if (nilfs_sb_dirty(nilfs)) {
> > > > +		sbp = nilfs_prepare_super(sbi, nilfs_sb_need_swap(nilfs));
> > > > +		if (likely(sbp)) {
> > > > +			nilfs_set_log_cursor(sbp[0], nilfs);
> > > > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > > > +		}
> > > > +	}
> > > >  	up_write(&nilfs->ns_sem);
> > > >  
> > > >  	return err;
> > > > @@ -635,14 +674,13 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
> > > >  	int mnt_count;
> > > >  
> > > >  	/* nilfs->ns_sem must be locked by the caller. */
> > > > -	sbp = nilfs_prepare_super(sbi);
> > > > +	sbp = nilfs_prepare_super(sbi, 0);
> > > >  	if (!sbp)
> > > >  		return -EIO;
> > > >  
> > > >  	max_mnt_count = le16_to_cpu(sbp[0]->s_max_mnt_count);
> > > >  	mnt_count = le16_to_cpu(sbp[0]->s_mnt_count);
> > > >  
> > > > -	/* nilfs->ns_sem must be locked by the caller. */
> > > >  	if (nilfs->ns_mount_state & NILFS_ERROR_FS) {
> > > >  		printk(KERN_WARNING
> > > >  		       "NILFS warning: mounting fs with errors\n");
> > > > @@ -659,7 +697,9 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
> > > >  	sbp[0]->s_state =
> > > >  		cpu_to_le16(le16_to_cpu(sbp[0]->s_state) & ~NILFS_VALID_FS);
> > > >  	sbp[0]->s_mtime = cpu_to_le64(get_seconds());
> > > > -	return nilfs_commit_super(sbi, 1);
> > > > +	/* synchronize sbp[1] with sbp[0] */
> > > > +	memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> > > > +	return nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
> > > >  }
> > > >  
> > > >  struct nilfs_super_block *nilfs_read_super_block(struct super_block *sb,
> > > > @@ -906,14 +946,15 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
> > > >  		 * the RDONLY flag and then mark the partition as valid again.
> > > >  		 */
> > > >  		down_write(&nilfs->ns_sem);
> > > > -		sbp = nilfs_prepare_super(sbi);
> > > > +		sbp = nilfs_prepare_super(sbi, 0);
> > > >  		if (likely(sbp)) {
> > > >  			if (!(sbp[0]->s_state & le16_to_cpu(NILFS_VALID_FS)) &&
> > > >  			    (nilfs->ns_mount_state & NILFS_VALID_FS))
> > > >  				sbp[0]->s_state =
> > > >  					cpu_to_le16(nilfs->ns_mount_state);
> > > >  			sbp[0]->s_mtime = cpu_to_le64(get_seconds());
> > > > -			nilfs_commit_super(sbi, 1);
> > > > +			nilfs_set_log_cursor(sbp[0], nilfs);
> > > > +			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > > >  		}
> > > >  		up_write(&nilfs->ns_sem);
> > > >  	} else {
> > > > diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
> > > > index 74b0480..7fdb780 100644
> > > > --- a/fs/nilfs2/the_nilfs.c
> > > > +++ b/fs/nilfs2/the_nilfs.c
> > > > @@ -326,11 +326,13 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
> > > >  
> > > >  	down_write(&nilfs->ns_sem);
> > > >  	err = -EIO;
> > > > -	sbp = nilfs_prepare_super(sbi);
> > > > +	sbp = nilfs_prepare_super(sbi, 0);
> > > >  	if (likely(sbp)) {
> > > >  		nilfs->ns_mount_state |= NILFS_VALID_FS;
> > > > +		/* set the flag only for newer super block */
> > > >  		sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
> > > > -		err = nilfs_commit_super(sbi, 1);
> > > > +		nilfs_set_log_cursor(sbp[0], nilfs);
> > > > +		err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> > > >  	}
> > > >  	up_write(&nilfs->ns_sem);
> > > >  
> > > > @@ -519,8 +521,8 @@ static int nilfs_load_super_block(struct the_nilfs *nilfs,
> > > >  		nilfs_swap_super_block(nilfs);
> > > >  	}
> > > >  
> > > > -	nilfs->ns_sbwtime[0] = le64_to_cpu(sbp[0]->s_wtime);
> > > > -	nilfs->ns_sbwtime[1] = valid[!swp] ? le64_to_cpu(sbp[1]->s_wtime) : 0;
> > > > +	nilfs->ns_sbwcount = 0;
> > > > +	nilfs->ns_sbwtime = le64_to_cpu(sbp[0]->s_wtime);
> > > >  	nilfs->ns_prot_seq = le64_to_cpu(sbp[valid[1] & !swp]->s_last_seq);
> > > >  	*sbpp = sbp[0];
> > > >  	return 0;
> > > > diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
> > > > index 85df47f..0a44688 100644
> > > > --- a/fs/nilfs2/the_nilfs.h
> > > > +++ b/fs/nilfs2/the_nilfs.h
> > > > @@ -57,7 +57,8 @@ enum {
> > > >   * @ns_current: back pointer to current mount
> > > >   * @ns_sbh: buffer heads of on-disk super blocks
> > > >   * @ns_sbp: pointers to super block data
> > > > - * @ns_sbwtime: previous write time of super blocks
> > > > + * @ns_sbwtime: previous write time of super block
> > > > + * @ns_sbwcount: write count of super block
> > > >   * @ns_sbsize: size of valid data in super block
> > > >   * @ns_supers: list of nilfs super block structs
> > > >   * @ns_seg_seq: segment sequence counter
> > > > @@ -120,7 +121,8 @@ struct the_nilfs {
> > > >  	 */
> > > >  	struct buffer_head     *ns_sbh[2];
> > > >  	struct nilfs_super_block *ns_sbp[2];
> > > > -	time_t			ns_sbwtime[2];
> > > > +	time_t			ns_sbwtime;
> > > > +	unsigned		ns_sbwcount;
> > > >  	unsigned		ns_sbsize;
> > > >  	unsigned		ns_mount_state;
> > > >  
> > > > @@ -205,20 +207,17 @@ THE_NILFS_FNS(SB_DIRTY, sb_dirty)
> > > >  
> > > >  /* Minimum interval of periodical update of superblocks (in seconds) */
> > > >  #define NILFS_SB_FREQ		10
> > > > -#define NILFS_ALTSB_FREQ	60  /* spare superblock */
> > > >  
> > > >  static inline int nilfs_sb_need_update(struct the_nilfs *nilfs)
> > > >  {
> > > >  	u64 t = get_seconds();
> > > > -	return t < nilfs->ns_sbwtime[0] ||
> > > > -		 t > nilfs->ns_sbwtime[0] + NILFS_SB_FREQ;
> > > > +	return t < nilfs->ns_sbwtime || t > nilfs->ns_sbwtime + NILFS_SB_FREQ;
> > > >  }
> > > >  
> > > > -static inline int nilfs_altsb_need_update(struct the_nilfs *nilfs)
> > > > +static inline int nilfs_sb_need_swap(struct the_nilfs *nilfs)
> > > >  {
> > > > -	u64 t = get_seconds();
> > > > -	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> > > > -	return sbp[1] && t > nilfs->ns_sbwtime[1] + NILFS_ALTSB_FREQ;
> > > > +	int flip_bits = nilfs->ns_sbwcount & 0x0FL;
> > > > +	return (flip_bits != 0x08 && flip_bits != 0x0F);
> > > >  }
> > > >  
> > > >  void nilfs_set_last_segment(struct the_nilfs *, sector_t, u64, __u64);
> > > > -- 
> > > > 1.5.6.5
> > > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> > > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> > > 
> > > 
> > 
> > 
> > -- 
> > Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
> 
> 
> Regards,
> Ryusuke Konishi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

-- 
Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 2/2] nilfs2: sync super blocks in turns

[Ryusuke Konishi]

On Sun, 27 Jun 2010 12:22:53 +0900, Jiro SEKIBA wrote:
> Hi,
> 
> At Sat, 26 Jun 2010 23:51:08 +0900 (JST),
> Ryusuke Konishi wrote:
> > 
> > Hi,
> > On Sat, 26 Jun 2010 14:05:31 +0900, Jiro SEKIBA wrote:
> > > Hi, thank you for the comments.
> > > 
> > > At Fri, 25 Jun 2010 18:06:39 +0900 (JST),
> > > Ryusuke Konishi wrote:
> > > > 
> > > > > +			if (sbp &&
> > > > > +			    nilfs->ns_sbp[0]->s_last_cno < sbp->s_last_cno)
> > > > > +				sbp = nilfs->ns_sbp[0];
> > > > >  		}
> > 
> > Here, endian conversions are required for both s_last_cno references.
> > 
> > We can apply some bit operations without endian conversion (e.g. OR,
> > AND, XOR, and so on), but it's not true for arithmetic operations like
> > addition, subtraction and comparison.
> > 
> > And, the above code seems to never update ns_prot_seq when
> > NILFS_SB_COMMIT is specified.
> 
> Grr, those are really careless bugs...
> Thank you for the comments.  I'll revise it

I found another issue on the patch.

It occasionally makes the filesystem unclean after unmount or
read-only remount.  I found it's because the "clean" flag is left
unset for one of two super blocks even if both super blocks point to
the same checkpoint.

Here is a patch to fix it.  I will later squash this into your patch
and post the revised one.

Regards,
Ryusuke Konishi

---
From: Ryusuke Konishi <konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>

nilfs2: sync super blocks in turns fix

The patch "nilfs2: sync super blocks in turns" alternately writes back
super blocks to allow fallback when mount from the primary super block
failed.  But, it occasionally makes the filesystem unclean after
unmount or read-only remount.

This turned out because the "clean" flag of filesystem
(i.e. NILFS_VALID_FS) may not be restored to the primary super block
even if both super blocks point to the same checkpoint.

This may happen if the filesystem was unmounted without any change, or
if one of the two super blocks was destroyed on memory and the data
was copied back from the spare super block.

This fixes the issue by restoring the "clean" flag to both super
blocks if they point to the same checkpoint.  To share the same
cleanup work, a routine named "nilfs_cleanup_super" is added.

Cc: Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
Signed-off-by: Ryusuke Konishi <konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
---
 fs/nilfs2/nilfs.h     |    1 +
 fs/nilfs2/super.c     |   52 ++++++++++++++++++++++++++++++++-----------------
 fs/nilfs2/the_nilfs.c |   12 +---------
 3 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 63c8fd4..36998ea 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -283,6 +283,7 @@ extern void nilfs_set_log_cursor(struct nilfs_super_block *,
 extern struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *,
 						      int flip);
 extern int nilfs_commit_super(struct nilfs_sb_info *, int);
+extern int nilfs_cleanup_super(struct nilfs_sb_info *);
 extern int nilfs_attach_checkpoint(struct nilfs_sb_info *, __u64);
 extern void nilfs_detach_checkpoint(struct nilfs_sb_info *);
 
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 393bf89..8942baa 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -320,11 +320,42 @@ int nilfs_commit_super(struct nilfs_sb_info *sbi, int flag)
 	return nilfs_sync_super(sbi, flag);
 }
 
+/**
+ * nilfs_cleanup_super() - write filesystem state for cleanup
+ * @sbi: nilfs_sb_info to be unmounted or degraded to read-only
+ *
+ * This function restores state flags in the on-disk super block.
+ * This will set "clean" flag (i.e. NILFS_VALID_FS) unless the
+ * filesystem was not clean previously.
+ */
+int nilfs_cleanup_super(struct nilfs_sb_info *sbi)
+{
+	struct nilfs_super_block **sbp;
+	int flag = NILFS_SB_COMMIT;
+	int ret = -EIO;
+
+	sbp = nilfs_prepare_super(sbi, 0);
+	if (sbp) {
+		sbp[0]->s_state = cpu_to_le16(sbi->s_nilfs->ns_mount_state);
+		nilfs_set_log_cursor(sbp[0], sbi->s_nilfs);
+		if (sbp[1] && sbp[0]->s_last_cno == sbp[1]->s_last_cno) {
+			/*
+			 * make the "clean" flag also to the opposite
+			 * super block if both super blocks point to
+			 * the same checkpoint.
+			 */
+			sbp[1]->s_state = sbp[0]->s_state;
+			flag = NILFS_SB_COMMIT_ALL;
+		}
+		ret = nilfs_commit_super(sbi, flag);
+	}
+	return ret;
+}
+
 static void nilfs_put_super(struct super_block *sb)
 {
 	struct nilfs_sb_info *sbi = NILFS_SB(sb);
 	struct the_nilfs *nilfs = sbi->s_nilfs;
-	struct nilfs_super_block **sbp;
 
 	lock_kernel();
 
@@ -332,13 +363,7 @@ static void nilfs_put_super(struct super_block *sb)
 
 	if (!(sb->s_flags & MS_RDONLY)) {
 		down_write(&nilfs->ns_sem);
-		sbp = nilfs_prepare_super(sbi, 0);
-		if (likely(sbp)) {
-			/* set state only for newer super block */
-			sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
-			nilfs_set_log_cursor(sbp[0], nilfs);
-			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
-		}
+		nilfs_cleanup_super(sbi);
 		up_write(&nilfs->ns_sem);
 	}
 	down_write(&nilfs->ns_super_sem);
@@ -883,7 +908,6 @@ nilfs_fill_super(struct super_block *sb, void *data, int silent,
 static int nilfs_remount(struct super_block *sb, int *flags, char *data)
 {
 	struct nilfs_sb_info *sbi = NILFS_SB(sb);
-	struct nilfs_super_block **sbp;
 	struct the_nilfs *nilfs = sbi->s_nilfs;
 	unsigned long old_sb_flags;
 	struct nilfs_mount_options old_opts;
@@ -944,15 +968,7 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
 		 * the RDONLY flag and then mark the partition as valid again.
 		 */
 		down_write(&nilfs->ns_sem);
-		sbp = nilfs_prepare_super(sbi, 0);
-		if (likely(sbp)) {
-			if (!(sbp[0]->s_state & le16_to_cpu(NILFS_VALID_FS)) &&
-			    (nilfs->ns_mount_state & NILFS_VALID_FS))
-				sbp[0]->s_state =
-					cpu_to_le16(nilfs->ns_mount_state);
-			nilfs_set_log_cursor(sbp[0], nilfs);
-			nilfs_commit_super(sbi, NILFS_SB_COMMIT);
-		}
+		nilfs_cleanup_super(sbi);
 		up_write(&nilfs->ns_sem);
 	} else {
 		/*
diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
index 33c5347..530d277 100644
--- a/fs/nilfs2/the_nilfs.c
+++ b/fs/nilfs2/the_nilfs.c
@@ -261,7 +261,6 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
 	unsigned int s_flags = sbi->s_super->s_flags;
 	int really_read_only = bdev_read_only(nilfs->ns_bdev);
 	int valid_fs = nilfs_valid_fs(nilfs);
-	struct nilfs_super_block **sbp;
 	int err;
 
 	if (nilfs_loaded(nilfs)) {
@@ -325,15 +324,8 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
 		goto failed_unload;
 
 	down_write(&nilfs->ns_sem);
-	err = -EIO;
-	sbp = nilfs_prepare_super(sbi, 0);
-	if (likely(sbp)) {
-		nilfs->ns_mount_state |= NILFS_VALID_FS;
-		/* set the flag only for newer super block */
-		sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
-		nilfs_set_log_cursor(sbp[0], nilfs);
-		err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
-	}
+	nilfs->ns_mount_state |= NILFS_VALID_FS; /* set "clean" flag */
+	err = nilfs_cleanup_super(sbi);
 	up_write(&nilfs->ns_sem);
 
 	if (err) {
-- 
1.6.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: CentOS 5 support for nilfs

[Ryusuke Konishi]

Hi,
On Mon, 28 Jun 2010 12:19:13 -0500, "Martin, Nick" wrote:
> Thank you, I will try these.
> 
> When I looked in April after the update announcement, I did not find them.  
> Then I looked at the last updated column on this page: 
> 
> http://www.nilfs.org/en/download.html
> 
> This was the main source of my confusion.  
> I thought the "Update" column would change when something new appeared.
> It shows last "update" for CentOS in January 2010.  Some versions show even older.

Ah, I understood.  Actually, it looks confusing.

The CentOS pacakges are voluntarily maintained by Seiji Kihara, one of
our project members.  I'll share this issue with him.

> I was checking this page instead of the actual download site for new updates.
> 
> Thanks,
> Nick

Thanks,
Ryusuke Konishi
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html