All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] nilfs2: asynchronous super block updates
@ 2010-06-08 22:15 Jiro SEKIBA
       [not found] ` <1276035358-24284-1-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Jiro SEKIBA @ 2010-06-08 22:15 UTC (permalink / raw)
  To: linux-nilfs-u79uwXL29TY76Z2rM5mHXA
  Cc: konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg, Ryusuke Konishi, Jiro SEKIBA

Hi,

This is revised 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 nilfs_write_log_cursor to advance log cursor
for one of super blocks.  This patch also define a macro that generates
inline function to update information both super blocks for convenience.


 fs/nilfs2/nilfs.h     |    2 +
 fs/nilfs2/segment.c   |    8 +--
 fs/nilfs2/super.c     |  144 ++++++++++++++++++++++++++++++++++---------------
 fs/nilfs2/the_nilfs.c |   11 +++-
 fs/nilfs2/the_nilfs.h |   23 +++++---
 5 files changed, 128 insertions(+), 60 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] nilfs2: introduce nilfs_prepare_super
       [not found] ` <1276035358-24284-1-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
@ 2010-06-08 22:15   ` Jiro SEKIBA
  2010-06-08 22:15   ` [PATCH 2/2] nilfs2: sync super blocks in turns Jiro SEKIBA
  2010-06-09  1:36   ` [PATCH 0/2] nilfs2: asynchronous super block updates Ryusuke Konishi
  2 siblings, 0 replies; 8+ messages in thread
From: Jiro SEKIBA @ 2010-06-08 22:15 UTC (permalink / raw)
  To: linux-nilfs-u79uwXL29TY76Z2rM5mHXA
  Cc: konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg, Ryusuke Konishi, 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   |    7 +++-
 fs/nilfs2/super.c     |   84 ++++++++++++++++++++++++++++++++----------------
 fs/nilfs2/the_nilfs.c |   11 +++++--
 4 files changed, 70 insertions(+), 33 deletions(-)

diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 8723e5b..9805d35 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 6a7dbd8..d42e5ee 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2490,8 +2490,11 @@ 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));
+			if (likely(nilfs_prepare_super(sbi)))
+				err = nilfs_commit_super(
+					sbi, nilfs_altsb_need_update(nilfs));
+			else
+				err = -EIO;
 			up_write(&nilfs->ns_sem);
 		}
 	}
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 48145f5..a452e90 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -85,6 +85,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);
@@ -99,9 +100,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);
 
@@ -257,24 +260,32 @@ 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 != NILFS_SUPER_MAGIC) {
 		if (sbp[1] && sbp[1]->s_magic == NILFS_SUPER_MAGIC)
 			nilfs_swap_super_block(nilfs);
 		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");
@@ -306,6 +317,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();
 
@@ -313,8 +325,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);
@@ -342,7 +357,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) && likely(nilfs_prepare_super(sbi)))
 		nilfs_commit_super(sbi, 1);
 	up_write(&nilfs->ns_sem);
 
@@ -637,11 +652,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->sem must be locked by the caller. */
+	/* 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->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");
@@ -652,11 +675,13 @@ 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[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->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_mtime = cpu_to_le64(get_seconds());
 	return nilfs_commit_super(sbi, 1);
 }
 
@@ -845,7 +870,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;
@@ -896,12 +921,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 a756168..e8f2f1f 100644
--- a/fs/nilfs2/the_nilfs.c
+++ b/fs/nilfs2/the_nilfs.c
@@ -262,6 +262,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)) {
@@ -325,9 +326,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);
+	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);
+	} else
+		err = -EIO;
 	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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] nilfs2: sync super blocks in turns
       [not found] ` <1276035358-24284-1-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
  2010-06-08 22:15   ` [PATCH 1/2] nilfs2: introduce nilfs_prepare_super Jiro SEKIBA
@ 2010-06-08 22:15   ` Jiro SEKIBA
  2010-06-09  1:36   ` [PATCH 0/2] nilfs2: asynchronous super block updates Ryusuke Konishi
  2 siblings, 0 replies; 8+ messages in thread
From: Jiro SEKIBA @ 2010-06-08 22:15 UTC (permalink / raw)
  To: linux-nilfs-u79uwXL29TY76Z2rM5mHXA
  Cc: konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg, Ryusuke Konishi, 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 stiation, old super block likely points valid log.

This patch introduces nilfs_write_log_cursor to advance log cursor
one of super blocks.  To update super block information other than
log cursor, 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     |    1 +
 fs/nilfs2/segment.c   |   11 +-----
 fs/nilfs2/super.c     |   88 +++++++++++++++++++++++++++++++++----------------
 fs/nilfs2/the_nilfs.c |    2 +-
 fs/nilfs2/the_nilfs.h |   23 ++++++++----
 5 files changed, 78 insertions(+), 47 deletions(-)

diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 9805d35..581561b 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 int nilfs_write_log_cursor(struct nilfs_sb_info *);
 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);
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index d42e5ee..ca81f3a 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2488,15 +2488,8 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
 		if (mode != SC_FLUSH_DAT)
 			atomic_set(&nilfs->ns_ndirtyblks, 0);
 		if (test_bit(NILFS_SC_SUPER_ROOT, &sci->sc_flags) &&
-		    nilfs_discontinued(nilfs)) {
-			down_write(&nilfs->ns_sem);
-			if (likely(nilfs_prepare_super(sbi)))
-				err = nilfs_commit_super(
-					sbi, nilfs_altsb_need_update(nilfs));
-			else
-				err = -EIO;
-			up_write(&nilfs->ns_sem);
-		}
+		    nilfs_discontinued(nilfs))
+			err = nilfs_write_log_cursor(sbi);
 	}
 
 	nilfs_segctor_notify(sci, mode, err);
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index a452e90..14099ae 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -102,7 +102,8 @@ void nilfs_error(struct super_block *sb, const char *function,
 			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_super_set_state(sbp,
+				sbp[0]->s_state | cpu_to_le16(NILFS_ERROR_FS));
 				nilfs_commit_super(sbi, 1);
 			}
 		}
@@ -229,6 +230,8 @@ 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]) {
+			memcpy(nilfs->ns_sbp[1], nilfs->ns_sbp[0],
+			       nilfs->ns_sbsize);
 			nilfs_fall_back_super_block(nilfs);
 			goto retry;
 		}
@@ -248,6 +251,13 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
 				set_buffer_dirty(nilfs->ns_sbh[1]);
 				if (!sync_dirty_buffer(nilfs->ns_sbh[1]))
 					sbp = nilfs->ns_sbp[1];
+			} else {
+				int flip_bits = (nilfs->ns_cno & 0x0FL);
+				/* flip super block 9 to 7 ratio */
+				if (flip_bits == 0x08 || flip_bits == 0x0F) {
+					nilfs_swap_super_block(nilfs);
+					sbp = nilfs->ns_sbp[1];
+				}
 			}
 		}
 		if (sbp) {
@@ -281,38 +291,60 @@ 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");
-		return err;
-	}
-	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);
-	spin_unlock(&nilfs->ns_last_segment_lock);
 
 	t = get_seconds();
 	nilfs->ns_sbwtime[0] = t;
-	sbp[0]->s_free_blocks_count = cpu_to_le64(nfreeblocks);
 	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;
+		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);
 }
 
+int nilfs_write_log_cursor(struct nilfs_sb_info *sbi)
+{
+	struct the_nilfs *nilfs = sbi->s_nilfs;
+	struct nilfs_super_block **sbp;
+	sector_t nfreeblocks;
+	int err;
+
+	down_write(&nilfs->ns_sem);
+	sbp = nilfs_prepare_super(sbi);
+	if (unlikely(!sbp))  {
+		err = -EIO;
+		goto out;
+	}
+
+	err = nilfs_count_free_blocks(nilfs, &nfreeblocks);
+	if (unlikely(err)) {
+		printk(KERN_ERR "NILFS: failed to count free blocks\n");
+		goto out;
+	}
+	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);
+	spin_unlock(&nilfs->ns_last_segment_lock);
+
+	sbp[0]->s_free_blocks_count = cpu_to_le64(nfreeblocks);
+
+	err = nilfs_commit_super(sbi, 0);
+out:
+	up_write(&nilfs->ns_sem);
+	return err;
+}
+
 static void nilfs_put_super(struct super_block *sb)
 {
 	struct nilfs_sb_info *sbi = NILFS_SB(sb);
@@ -327,7 +359,8 @@ static void nilfs_put_super(struct super_block *sb)
 		down_write(&nilfs->ns_sem);
 		sbp = nilfs_prepare_super(sbi);
 		if (likely(sbp)) {
-			sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
+			nilfs_super_set_state(sbp,
+				cpu_to_le16(nilfs->ns_mount_state));
 			nilfs_commit_super(sbi, 1);
 		}
 		up_write(&nilfs->ns_sem);
@@ -356,11 +389,8 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
 	if (wait)
 		err = nilfs_construct_segment(sb);
 
-	down_write(&nilfs->ns_sem);
-	if (nilfs_sb_dirty(nilfs) && likely(nilfs_prepare_super(sbi)))
-		nilfs_commit_super(sbi, 1);
-	up_write(&nilfs->ns_sem);
-
+	if (nilfs_sb_dirty(nilfs))
+		nilfs_write_log_cursor(sbi);
 	return err;
 }
 
@@ -675,13 +705,13 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
 #endif
 	}
 	if (!max_mnt_count)
-		sbp[0]->s_max_mnt_count = cpu_to_le16(NILFS_DFL_MAX_MNT_COUNT);
-
-	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);
+		nilfs_super_set_max_mnt_count(sbp,
+				cpu_to_le16(NILFS_DFL_MAX_MNT_COUNT));
 
-	sbp[0]->s_mtime = cpu_to_le64(get_seconds());
+	nilfs_super_set_mnt_count(sbp, cpu_to_le16(mnt_count + 1));
+	nilfs_super_set_state(sbp,
+		cpu_to_le16(le16_to_cpu(sbp[0]->s_state) & ~NILFS_VALID_FS));
+	nilfs_super_set_mtime(sbp, cpu_to_le64(get_seconds()));
 	return nilfs_commit_super(sbi, 1);
 }
 
diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
index e8f2f1f..923faa7 100644
--- a/fs/nilfs2/the_nilfs.c
+++ b/fs/nilfs2/the_nilfs.c
@@ -329,7 +329,7 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
 	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);
+		nilfs_super_set_state(sbp, cpu_to_le16(nilfs->ns_mount_state));
 		err = nilfs_commit_super(sbi, 1);
 	} else
 		err = -EIO;
diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
index 1ab9745..1bc8994 100644
--- a/fs/nilfs2/the_nilfs.h
+++ b/fs/nilfs2/the_nilfs.h
@@ -201,9 +201,23 @@ THE_NILFS_FNS(DISCONTINUED, discontinued)
 THE_NILFS_FNS(GC_RUNNING, gc_running)
 THE_NILFS_FNS(SB_DIRTY, sb_dirty)
 
+#define NILFS_SUPER_SET_FN(name, type)				\
+static inline void nilfs_super_set_##name(			\
+			struct nilfs_super_block **sbp,		\
+			type val)				\
+{								\
+	sbp[0]->s_##name = val;					\
+	if (sbp[1] && sbp[1]->s_magic == NILFS_SUPER_MAGIC)	\
+		sbp[1]->s_##name = val;				\
+}
+
+NILFS_SUPER_SET_FN(state, __le16)
+NILFS_SUPER_SET_FN(mnt_count, __le16)
+NILFS_SUPER_SET_FN(max_mnt_count, __le16)
+NILFS_SUPER_SET_FN(mtime, __le64)
+
 /* 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)
 {
@@ -212,13 +226,6 @@ static inline int nilfs_sb_need_update(struct the_nilfs *nilfs)
 		 t > nilfs->ns_sbwtime[0] + NILFS_SB_FREQ;
 }
 
-static inline int nilfs_altsb_need_update(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;
-}
-
 void nilfs_set_last_segment(struct the_nilfs *, sector_t, u64, __u64);
 struct the_nilfs *find_or_create_nilfs(struct block_device *);
 void put_nilfs(struct the_nilfs *);
-- 
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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] nilfs2: asynchronous super block updates
       [not found] ` <1276035358-24284-1-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
  2010-06-08 22:15   ` [PATCH 1/2] nilfs2: introduce nilfs_prepare_super Jiro SEKIBA
  2010-06-08 22:15   ` [PATCH 2/2] nilfs2: sync super blocks in turns Jiro SEKIBA
@ 2010-06-09  1:36   ` Ryusuke Konishi
  2 siblings, 0 replies; 8+ messages in thread
From: Ryusuke Konishi @ 2010-06-09  1:36 UTC (permalink / raw)
  To: jir-hfpbi5WX9J54Eiagz67IpQ
  Cc: linux-nilfs-u79uwXL29TY76Z2rM5mHXA,
	konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg

On Wed,  9 Jun 2010 07:15:56 +0900, Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org> wrote:
> Hi,
> 
> This is revised 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 nilfs_write_log_cursor to advance log cursor
> for one of super blocks.  This patch also define a macro that generates
> inline function to update information both super blocks for convenience.

Ahh, this series doesn't apply as is.

Could you rebase it onto the for-next branch ?

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] nilfs2: sync super blocks in turns
       [not found]     ` <1276266639-15913-3-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
@ 2010-06-12 19:06       ` Ryusuke Konishi
  0 siblings, 0 replies; 8+ messages in thread
From: Ryusuke Konishi @ 2010-06-12 19:06 UTC (permalink / raw)
  To: jir-hfpbi5WX9J54Eiagz67IpQ
  Cc: linux-nilfs-u79uwXL29TY76Z2rM5mHXA,
	konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg

On Fri, 11 Jun 2010 23:30:39 +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 stiation, old super block likely points valid log.
> 
> This patch introduces nilfs_write_log_cursor to advance log cursor
> one of super blocks.  To update super block information other than
> log cursor, 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     |    5 ++
>  fs/nilfs2/segment.c   |   11 +----
>  fs/nilfs2/super.c     |  104 +++++++++++++++++++++++++++++++-----------------
>  fs/nilfs2/the_nilfs.c |    4 +-
>  fs/nilfs2/the_nilfs.h |   24 +++++++----
>  5 files changed, 92 insertions(+), 56 deletions(-)
> 
> diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
> index 9805d35..9867320 100644
> --- a/fs/nilfs2/nilfs.h
> +++ b/fs/nilfs2/nilfs.h
> @@ -259,6 +259,10 @@ extern void nilfs_dirty_inode(struct inode *);
>  extern struct dentry *nilfs_get_parent(struct dentry *);
>  
>  /* super.c */
> +enum {
> +	NILFS_SB_COMMIT = 0,
> +	NILFS_SB_COMMIT_ALL
> +};

This definition should be explained with some comments.

At least you should describe the target and meaning of each value
like:

  /*
   * Write mode of super blocks
   */
  enum {
  	NILFS_SB_COMMIT = 0,	/* Commit a super block alternately */
  	NILFS_SB_COMMIT_ALL	/* Commit both super blocks */
  };

And, this definition should be put earlier.

>  extern struct inode *nilfs_alloc_inode_common(struct the_nilfs *);
>  extern struct inode *nilfs_alloc_inode(struct super_block *);
>  extern void nilfs_destroy_inode(struct inode *);
> @@ -270,6 +274,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 int nilfs_write_log_cursor(struct nilfs_sb_info *);
>  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);
> diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
> index f67ffbb..e5a53fe 100644
> --- a/fs/nilfs2/segment.c
> +++ b/fs/nilfs2/segment.c
> @@ -2421,15 +2421,8 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
>  		if (mode != SC_FLUSH_DAT)
>  			atomic_set(&nilfs->ns_ndirtyblks, 0);
>  		if (test_bit(NILFS_SC_SUPER_ROOT, &sci->sc_flags) &&
> -		    nilfs_discontinued(nilfs)) {
> -			down_write(&nilfs->ns_sem);
> -			if (likely(nilfs_prepare_super(sbi)))
> -				err = nilfs_commit_super(
> -					sbi, nilfs_altsb_need_update(nilfs));
> -			else
> -				err = -EIO;
> -			up_write(&nilfs->ns_sem);
> -		}
> +		    nilfs_discontinued(nilfs))
> +			err = nilfs_write_log_cursor(sbi);
>  	}
>  
>  	nilfs_segctor_notify(sci, mode, err);
> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> index b978d6c..8872112 100644
> --- a/fs/nilfs2/super.c
> +++ b/fs/nilfs2/super.c
> @@ -107,8 +107,9 @@ void nilfs_error(struct super_block *sb, const char *function,
>  			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);
> +				nilfs_sb_set_state(sbp,
> +				sbp[0]->s_state | cpu_to_le16(NILFS_ERROR_FS));
> +				nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
>  			}
>  		}
>  		up_write(&nilfs->ns_sem);
> @@ -179,7 +180,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,6 +206,8 @@ 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]) {
> +			memcpy(nilfs->ns_sbp[1], nilfs->ns_sbp[0],
> +			       nilfs->ns_sbsize);
>  			nilfs_fall_back_super_block(nilfs);
>  			goto retry;
>  		}
> @@ -220,10 +223,17 @@ 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];
> +			} else {
> +				int flip_bits = (nilfs->ns_cno & 0x0FL);
> +				/* flip super block 9 to 7 ratio */
> +				if (flip_bits == 0x08 || flip_bits == 0x0F) {
> +					nilfs_swap_super_block(nilfs);
> +					sbp = nilfs->ns_sbp[1];
> +				}
>  			}
>  		}
>  		if (sbp) {
> @@ -254,19 +264,49 @@ struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
>  	return sbp;
>  }
>  
> -int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
> +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;
> -	sector_t nfreeblocks;
>  	time_t t;
> +
> +	t = get_seconds();
> +	nilfs->ns_sbwtime[0] = 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 (flag == NILFS_SB_COMMIT_ALL && sbp[1]) {
> +		nilfs->ns_sbwtime[1] = t;
> +		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, flag);
> +}
> +
> +int nilfs_write_log_cursor(struct nilfs_sb_info *sbi)
> +{
> +	struct the_nilfs *nilfs = sbi->s_nilfs;
> +	struct nilfs_super_block **sbp;
> +	sector_t nfreeblocks;
>  	int err;
>  
> -	/* nilfs->ns_sem must be locked by the caller. */
> +	down_write(&nilfs->ns_sem);
> +	sbp = nilfs_prepare_super(sbi);
> +	if (unlikely(!sbp))  {
> +		err = -EIO;
> +		goto out;
> +	}
> +
>  	err = nilfs_count_free_blocks(nilfs, &nfreeblocks);
>  	if (unlikely(err)) {
>  		printk(KERN_ERR "NILFS: failed to count free blocks\n");
> -		return err;
> +		goto out;
>  	}
>  	spin_lock(&nilfs->ns_last_segment_lock);
>  	sbp[0]->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
> @@ -274,20 +314,12 @@ int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
>  	sbp[0]->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
>  	spin_unlock(&nilfs->ns_last_segment_lock);
>  
> -	t = get_seconds();
> -	nilfs->ns_sbwtime[0] = t;
>  	sbp[0]->s_free_blocks_count = cpu_to_le64(nfreeblocks);
> -	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;
> -	}
> -	clear_nilfs_sb_dirty(nilfs);
> -	return nilfs_sync_super(sbi, dupsb);
> +
> +	err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
> +out:
> +	up_write(&nilfs->ns_sem);
> +	return err;
>  }
>  
>  static void nilfs_put_super(struct super_block *sb)
> @@ -304,8 +336,9 @@ static void nilfs_put_super(struct super_block *sb)
>  		down_write(&nilfs->ns_sem);
>  		sbp = nilfs_prepare_super(sbi);
>  		if (likely(sbp)) {
> -			sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
> -			nilfs_commit_super(sbi, 1);
> +			nilfs_sb_set_state(sbp,
> +				cpu_to_le16(nilfs->ns_mount_state));
> +			nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
>  		}
>  		up_write(&nilfs->ns_sem);
>  	}

This looks to have a problem.  Note that this will restore VALID_FS
flag for both super blocks.

If the master super block which indicates a newer log is destroyed,
nilfs skips roll-forward from an old log because the secondary super
block also has the VALID_FS flag. (See nilfs_search_super_root)


> @@ -333,11 +366,8 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
>  	if (wait)
>  		err = nilfs_construct_segment(sb);
>  
> -	down_write(&nilfs->ns_sem);
> -	if (nilfs_sb_dirty(nilfs) && likely(nilfs_prepare_super(sbi)))
> -		nilfs_commit_super(sbi, 1);
> -	up_write(&nilfs->ns_sem);
> -
> +	if (nilfs_sb_dirty(nilfs))
> +		nilfs_write_log_cursor(sbi);
>  	return err;
>  }
>  
> @@ -652,14 +682,14 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
>  #endif
>  	}
>  	if (!max_mnt_count)
> -		sbp[0]->s_max_mnt_count = cpu_to_le16(NILFS_DFL_MAX_MNT_COUNT);
> -
> -	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);
> +		nilfs_sb_set_max_mnt_count(sbp,
> +				cpu_to_le16(NILFS_DFL_MAX_MNT_COUNT));
> +
> +	nilfs_sb_set_mnt_count(sbp, cpu_to_le16(mnt_count + 1));
> +	nilfs_sb_set_state(sbp,
> +		cpu_to_le16(le16_to_cpu(sbp[0]->s_state) & ~NILFS_VALID_FS));
> +	nilfs_sb_set_mtime(sbp, cpu_to_le64(get_seconds()));
> +	return nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
>  }
>  
>  struct nilfs_super_block *nilfs_read_super_block(struct super_block *sb,
> @@ -913,7 +943,7 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
>  				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_commit_super(sbi, NILFS_SB_COMMIT_ALL);
>  		}
>  		up_write(&nilfs->ns_sem);
>  	} else {
> diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
> index e8f2f1f..95533d9 100644
> --- a/fs/nilfs2/the_nilfs.c
> +++ b/fs/nilfs2/the_nilfs.c
> @@ -329,8 +329,8 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
>  	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);
> +		nilfs_sb_set_state(sbp, cpu_to_le16(nilfs->ns_mount_state));
> +		err = nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
>  	} else
>  		err = -EIO;
>  	up_write(&nilfs->ns_sem);
> diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
> index 1ab9745..38ecb2d 100644
> --- a/fs/nilfs2/the_nilfs.h
> +++ b/fs/nilfs2/the_nilfs.h
> @@ -201,9 +201,24 @@ THE_NILFS_FNS(DISCONTINUED, discontinued)
>  THE_NILFS_FNS(GC_RUNNING, gc_running)
>  THE_NILFS_FNS(SB_DIRTY, sb_dirty)
>  
> +#define NILFS_SB_SET_FN(name, type)				\
> +static inline void nilfs_sb_set_##name(			\
> +			struct nilfs_super_block **sbp,		\
> +			type val)				\
> +{								\
> +	sbp[0]->s_##name = val;					\
> +	if (sbp[1] &&						\
> +	    sbp[1]->s_magic == cpu_to_le16(NILFS_SUPER_MAGIC))	\
> +		sbp[1]->s_##name = val;				\
> +}

This check looks overkill.

I think nilfs_prepare_super() should handle abnormal cases and try
recovery if one of two super blocks is invalid.

Without the change, this patch seems to break automatic repair feature
of super blocks.  Please think how a broken super block is repaired
with another one.

> +NILFS_SB_SET_FN(state, __le16)
> +NILFS_SB_SET_FN(mnt_count, __le16)
> +NILFS_SB_SET_FN(max_mnt_count, __le16)
> +NILFS_SB_SET_FN(mtime, __le64)
> +
>  /* 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)
>  {
> @@ -212,13 +227,6 @@ static inline int nilfs_sb_need_update(struct the_nilfs *nilfs)
>  		 t > nilfs->ns_sbwtime[0] + NILFS_SB_FREQ;
>  }
>  
> -static inline int nilfs_altsb_need_update(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;
> -}
> -
>  void nilfs_set_last_segment(struct the_nilfs *, sector_t, u64, __u64);
>  struct the_nilfs *find_or_create_nilfs(struct block_device *);
>  void put_nilfs(struct the_nilfs *);
> -- 
> 1.5.6.5

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] nilfs2: sync super blocks in turns
       [not found] ` <1276266639-15913-1-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
@ 2010-06-11 14:30   ` Jiro SEKIBA
       [not found]     ` <1276266639-15913-3-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Jiro SEKIBA @ 2010-06-11 14:30 UTC (permalink / raw)
  To: linux-nilfs-u79uwXL29TY76Z2rM5mHXA
  Cc: konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg, Ryusuke Konishi, 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 stiation, old super block likely points valid log.

This patch introduces nilfs_write_log_cursor to advance log cursor
one of super blocks.  To update super block information other than
log cursor, 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     |    5 ++
 fs/nilfs2/segment.c   |   11 +----
 fs/nilfs2/super.c     |  104 +++++++++++++++++++++++++++++++-----------------
 fs/nilfs2/the_nilfs.c |    4 +-
 fs/nilfs2/the_nilfs.h |   24 +++++++----
 5 files changed, 92 insertions(+), 56 deletions(-)

diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 9805d35..9867320 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -259,6 +259,10 @@ extern void nilfs_dirty_inode(struct inode *);
 extern struct dentry *nilfs_get_parent(struct dentry *);
 
 /* super.c */
+enum {
+	NILFS_SB_COMMIT = 0,
+	NILFS_SB_COMMIT_ALL
+};
 extern struct inode *nilfs_alloc_inode_common(struct the_nilfs *);
 extern struct inode *nilfs_alloc_inode(struct super_block *);
 extern void nilfs_destroy_inode(struct inode *);
@@ -270,6 +274,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 int nilfs_write_log_cursor(struct nilfs_sb_info *);
 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);
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index f67ffbb..e5a53fe 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2421,15 +2421,8 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
 		if (mode != SC_FLUSH_DAT)
 			atomic_set(&nilfs->ns_ndirtyblks, 0);
 		if (test_bit(NILFS_SC_SUPER_ROOT, &sci->sc_flags) &&
-		    nilfs_discontinued(nilfs)) {
-			down_write(&nilfs->ns_sem);
-			if (likely(nilfs_prepare_super(sbi)))
-				err = nilfs_commit_super(
-					sbi, nilfs_altsb_need_update(nilfs));
-			else
-				err = -EIO;
-			up_write(&nilfs->ns_sem);
-		}
+		    nilfs_discontinued(nilfs))
+			err = nilfs_write_log_cursor(sbi);
 	}
 
 	nilfs_segctor_notify(sci, mode, err);
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index b978d6c..8872112 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -107,8 +107,9 @@ void nilfs_error(struct super_block *sb, const char *function,
 			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);
+				nilfs_sb_set_state(sbp,
+				sbp[0]->s_state | cpu_to_le16(NILFS_ERROR_FS));
+				nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
 			}
 		}
 		up_write(&nilfs->ns_sem);
@@ -179,7 +180,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,6 +206,8 @@ 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]) {
+			memcpy(nilfs->ns_sbp[1], nilfs->ns_sbp[0],
+			       nilfs->ns_sbsize);
 			nilfs_fall_back_super_block(nilfs);
 			goto retry;
 		}
@@ -220,10 +223,17 @@ 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];
+			} else {
+				int flip_bits = (nilfs->ns_cno & 0x0FL);
+				/* flip super block 9 to 7 ratio */
+				if (flip_bits == 0x08 || flip_bits == 0x0F) {
+					nilfs_swap_super_block(nilfs);
+					sbp = nilfs->ns_sbp[1];
+				}
 			}
 		}
 		if (sbp) {
@@ -254,19 +264,49 @@ struct nilfs_super_block **nilfs_prepare_super(struct nilfs_sb_info *sbi)
 	return sbp;
 }
 
-int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
+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;
-	sector_t nfreeblocks;
 	time_t t;
+
+	t = get_seconds();
+	nilfs->ns_sbwtime[0] = 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 (flag == NILFS_SB_COMMIT_ALL && sbp[1]) {
+		nilfs->ns_sbwtime[1] = t;
+		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, flag);
+}
+
+int nilfs_write_log_cursor(struct nilfs_sb_info *sbi)
+{
+	struct the_nilfs *nilfs = sbi->s_nilfs;
+	struct nilfs_super_block **sbp;
+	sector_t nfreeblocks;
 	int err;
 
-	/* nilfs->ns_sem must be locked by the caller. */
+	down_write(&nilfs->ns_sem);
+	sbp = nilfs_prepare_super(sbi);
+	if (unlikely(!sbp))  {
+		err = -EIO;
+		goto out;
+	}
+
 	err = nilfs_count_free_blocks(nilfs, &nfreeblocks);
 	if (unlikely(err)) {
 		printk(KERN_ERR "NILFS: failed to count free blocks\n");
-		return err;
+		goto out;
 	}
 	spin_lock(&nilfs->ns_last_segment_lock);
 	sbp[0]->s_last_seq = cpu_to_le64(nilfs->ns_last_seq);
@@ -274,20 +314,12 @@ int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
 	sbp[0]->s_last_cno = cpu_to_le64(nilfs->ns_last_cno);
 	spin_unlock(&nilfs->ns_last_segment_lock);
 
-	t = get_seconds();
-	nilfs->ns_sbwtime[0] = t;
 	sbp[0]->s_free_blocks_count = cpu_to_le64(nfreeblocks);
-	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;
-	}
-	clear_nilfs_sb_dirty(nilfs);
-	return nilfs_sync_super(sbi, dupsb);
+
+	err = nilfs_commit_super(sbi, NILFS_SB_COMMIT);
+out:
+	up_write(&nilfs->ns_sem);
+	return err;
 }
 
 static void nilfs_put_super(struct super_block *sb)
@@ -304,8 +336,9 @@ static void nilfs_put_super(struct super_block *sb)
 		down_write(&nilfs->ns_sem);
 		sbp = nilfs_prepare_super(sbi);
 		if (likely(sbp)) {
-			sbp[0]->s_state = cpu_to_le16(nilfs->ns_mount_state);
-			nilfs_commit_super(sbi, 1);
+			nilfs_sb_set_state(sbp,
+				cpu_to_le16(nilfs->ns_mount_state));
+			nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
 		}
 		up_write(&nilfs->ns_sem);
 	}
@@ -333,11 +366,8 @@ static int nilfs_sync_fs(struct super_block *sb, int wait)
 	if (wait)
 		err = nilfs_construct_segment(sb);
 
-	down_write(&nilfs->ns_sem);
-	if (nilfs_sb_dirty(nilfs) && likely(nilfs_prepare_super(sbi)))
-		nilfs_commit_super(sbi, 1);
-	up_write(&nilfs->ns_sem);
-
+	if (nilfs_sb_dirty(nilfs))
+		nilfs_write_log_cursor(sbi);
 	return err;
 }
 
@@ -652,14 +682,14 @@ static int nilfs_setup_super(struct nilfs_sb_info *sbi)
 #endif
 	}
 	if (!max_mnt_count)
-		sbp[0]->s_max_mnt_count = cpu_to_le16(NILFS_DFL_MAX_MNT_COUNT);
-
-	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);
+		nilfs_sb_set_max_mnt_count(sbp,
+				cpu_to_le16(NILFS_DFL_MAX_MNT_COUNT));
+
+	nilfs_sb_set_mnt_count(sbp, cpu_to_le16(mnt_count + 1));
+	nilfs_sb_set_state(sbp,
+		cpu_to_le16(le16_to_cpu(sbp[0]->s_state) & ~NILFS_VALID_FS));
+	nilfs_sb_set_mtime(sbp, cpu_to_le64(get_seconds()));
+	return nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
 }
 
 struct nilfs_super_block *nilfs_read_super_block(struct super_block *sb,
@@ -913,7 +943,7 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
 				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_commit_super(sbi, NILFS_SB_COMMIT_ALL);
 		}
 		up_write(&nilfs->ns_sem);
 	} else {
diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
index e8f2f1f..95533d9 100644
--- a/fs/nilfs2/the_nilfs.c
+++ b/fs/nilfs2/the_nilfs.c
@@ -329,8 +329,8 @@ int load_nilfs(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi)
 	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);
+		nilfs_sb_set_state(sbp, cpu_to_le16(nilfs->ns_mount_state));
+		err = nilfs_commit_super(sbi, NILFS_SB_COMMIT_ALL);
 	} else
 		err = -EIO;
 	up_write(&nilfs->ns_sem);
diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
index 1ab9745..38ecb2d 100644
--- a/fs/nilfs2/the_nilfs.h
+++ b/fs/nilfs2/the_nilfs.h
@@ -201,9 +201,24 @@ THE_NILFS_FNS(DISCONTINUED, discontinued)
 THE_NILFS_FNS(GC_RUNNING, gc_running)
 THE_NILFS_FNS(SB_DIRTY, sb_dirty)
 
+#define NILFS_SB_SET_FN(name, type)				\
+static inline void nilfs_sb_set_##name(			\
+			struct nilfs_super_block **sbp,		\
+			type val)				\
+{								\
+	sbp[0]->s_##name = val;					\
+	if (sbp[1] &&						\
+	    sbp[1]->s_magic == cpu_to_le16(NILFS_SUPER_MAGIC))	\
+		sbp[1]->s_##name = val;				\
+}
+
+NILFS_SB_SET_FN(state, __le16)
+NILFS_SB_SET_FN(mnt_count, __le16)
+NILFS_SB_SET_FN(max_mnt_count, __le16)
+NILFS_SB_SET_FN(mtime, __le64)
+
 /* 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)
 {
@@ -212,13 +227,6 @@ static inline int nilfs_sb_need_update(struct the_nilfs *nilfs)
 		 t > nilfs->ns_sbwtime[0] + NILFS_SB_FREQ;
 }
 
-static inline int nilfs_altsb_need_update(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;
-}
-
 void nilfs_set_last_segment(struct the_nilfs *, sector_t, u64, __u64);
 struct the_nilfs *find_or_create_nilfs(struct block_device *);
 void put_nilfs(struct the_nilfs *);
-- 
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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] nilfs2: sync super blocks in turns
       [not found]     ` <1275776349-22820-3-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
@ 2010-06-07  6:15       ` Ryusuke Konishi
  0 siblings, 0 replies; 8+ messages in thread
From: Ryusuke Konishi @ 2010-06-07  6:15 UTC (permalink / raw)
  To: jir-hfpbi5WX9J54Eiagz67IpQ
  Cc: linux-nilfs-u79uwXL29TY76Z2rM5mHXA,
	konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg

On Sun,  6 Jun 2010 07:19:09 +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 stiation, old super block likely points valid log.
> 
> Signed-off-by: Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>

Here is my comment on the second patch.

> ---
>  fs/nilfs2/nilfs.h   |    1 +
>  fs/nilfs2/segment.c |    8 +++++---
>  fs/nilfs2/super.c   |   44 +++++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
> index c8edffd..b8fd4d1 100644
> --- a/fs/nilfs2/nilfs.h
> +++ b/fs/nilfs2/nilfs.h
> @@ -271,6 +271,7 @@ 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 int nilfs_prepare_super(struct nilfs_sb_info *);
> +extern int nilfs_update_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 72f779f..4801331 100644
> --- a/fs/nilfs2/segment.c
> +++ b/fs/nilfs2/segment.c
> @@ -2491,9 +2491,11 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
>  		    nilfs_discontinued(nilfs)) {
>  			down_write(&nilfs->ns_sem);
>  			err = nilfs_prepare_super(sbi);
> -			if (likely(!err))
> -				err = nilfs_commit_super(
> -					sbi, nilfs_altsb_need_update(nilfs));
> +			if (likely(!err)) {
> +				err = nilfs_update_super(sbi);
> +				if (likely(!err))
> +					err = nilfs_commit_super(sbi, 0);
> +			}
>  			up_write(&nilfs->ns_sem);
>  		}
>  	}

Please integrate these three functions into one.

You can write a superior function which calls nilfs_prepare_super(),
updates one of two cursors that point to the latest log, and then
calls nilfs_commit_super().

By doing so, this part can be simplifed as follows:

		if (test_bit(NILFS_SC_SUPER_ROOT, &sci->sc_flags) &&
		    nilfs_discontinued(nilfs))
			err = nilfs_write_log_cursor(nilfs);

> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> index 312b34a..9d0edfb 100644
> --- a/fs/nilfs2/super.c
> +++ b/fs/nilfs2/super.c
> @@ -227,6 +227,8 @@ 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]) {
> +			memcpy(nilfs->ns_sbp[1], nilfs->ns_sbp[0],
> +			       nilfs->ns_sbsize);
>  			nilfs_fall_back_super_block(nilfs);
>  			goto retry;
>  		}
> @@ -246,6 +248,13 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
>  				set_buffer_dirty(nilfs->ns_sbh[1]);
>  				if (!sync_dirty_buffer(nilfs->ns_sbh[1]))
>  					sbp = nilfs->ns_sbp[1];
> +			} else {
> +				int flip_bits = (nilfs->ns_cno & 0x0FL);
> +				/* flip super block 9 to 7 ratio */
> +				if (flip_bits == 0x08 || flip_bits == 0x0F) {
> +					nilfs_swap_super_block(nilfs);
> +					sbp = nilfs->ns_sbp[1];
> +				}
>  			}
>  		}
>  		if (sbp) {
> @@ -275,7 +284,7 @@ int nilfs_prepare_super(struct nilfs_sb_info *sbi)
>  	return 0;
>  }
>  
> -int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
> +int nilfs_update_super(struct nilfs_sb_info *sbi)
>  {
>  	struct the_nilfs *nilfs = sbi->s_nilfs;
>  	struct nilfs_super_block **sbp = nilfs->ns_sbp;
> @@ -303,9 +312,31 @@ int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
>  	sbp[0]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
>  					     (unsigned char *)sbp[0],
>  					     nilfs->ns_sbsize));

The second half of nilfs_update_super() (the following part started
with '+' marks) should be moved to the head of nilfs_commit_super
since these lines are closing the changes for super block.

+       t = get_seconds();
+       nilfs->ns_sbwtime[0] = t;
-       sbp[0]->s_free_blocks_count = cpu_to_le64(nfreeblocks);
+       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));

> +	return 0;
> +}
> +
> +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;
> +
> +	/* nilfs->sem must be locked by the caller. */
>  	if (dupsb && sbp[1]) {
> -		memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
> -		nilfs->ns_sbwtime[1] = t;
> +		nilfs->ns_sbwtime[1] = nilfs->ns_sbwtime[0];
> +		/* copy super block except s_last_cno, s_last_pseg,
> +		   s_last_seq, s_free_blocks_count.
> +		  Those are asymmetric members */
> +		const int ctimeoff =
> +			offsetof(struct nilfs_super_block, s_ctime);
> +		memcpy(sbp[1], sbp[0],
> +		       offsetof(struct nilfs_super_block, s_last_cno));
> +		memcpy((unsigned char *)sbp[1] + ctimeoff,
> +		       (unsigned char *)sbp[0] + ctimeoff,
> +		       nilfs->ns_sbsize - ctimeoff);

Umm, this looks bad.

So, we shouldn't hide update operations on the spare superblock...
I'll pull back my previous comment on nilfs_prepare_super.

We would be better off copying the updated field into its counterpart
within the callers of nilfs_commit_super().

	sbp[0]->s_state |= ...;
	if (sbp[1])
		sbp[1]->s_state = sbp[0]->s_state;

We can make a macro to do this in generic manner, but it seems to be
overkill for now.

> +		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);
> @@ -352,8 +383,11 @@ 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) && likely(!nilfs_prepare_super(sbi)))
> -		nilfs_commit_super(sbi, 1);
> +	if (nilfs_sb_dirty(nilfs) && likely(!nilfs_prepare_super(sbi))) {
> +		err = nilfs_update_super(sbi);
> +		if (likely(!err))
> +			nilfs_commit_super(sbi, 0);
> +	}
>  	up_write(&nilfs->ns_sem);
>  
>  	return err;
> -- 
> 1.5.6.5

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] nilfs2: sync super blocks in turns
       [not found] ` <1275776349-22820-1-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
@ 2010-06-05 22:19   ` Jiro SEKIBA
       [not found]     ` <1275776349-22820-3-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Jiro SEKIBA @ 2010-06-05 22:19 UTC (permalink / raw)
  To: linux-nilfs-u79uwXL29TY76Z2rM5mHXA
  Cc: konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg,
	konishi.ryusuke-Re5JQEeQqe8AvxtiuMwx3w, Ryusuke Konishi,
	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 stiation, old super block likely points valid log.

Signed-off-by: Jiro SEKIBA <jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
---
 fs/nilfs2/nilfs.h   |    1 +
 fs/nilfs2/segment.c |    8 +++++---
 fs/nilfs2/super.c   |   44 +++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index c8edffd..b8fd4d1 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -271,6 +271,7 @@ 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 int nilfs_prepare_super(struct nilfs_sb_info *);
+extern int nilfs_update_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 72f779f..4801331 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2491,9 +2491,11 @@ static int nilfs_segctor_construct(struct nilfs_sc_info *sci, int mode)
 		    nilfs_discontinued(nilfs)) {
 			down_write(&nilfs->ns_sem);
 			err = nilfs_prepare_super(sbi);
-			if (likely(!err))
-				err = nilfs_commit_super(
-					sbi, nilfs_altsb_need_update(nilfs));
+			if (likely(!err)) {
+				err = nilfs_update_super(sbi);
+				if (likely(!err))
+					err = nilfs_commit_super(sbi, 0);
+			}
 			up_write(&nilfs->ns_sem);
 		}
 	}
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 312b34a..9d0edfb 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -227,6 +227,8 @@ 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]) {
+			memcpy(nilfs->ns_sbp[1], nilfs->ns_sbp[0],
+			       nilfs->ns_sbsize);
 			nilfs_fall_back_super_block(nilfs);
 			goto retry;
 		}
@@ -246,6 +248,13 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi, int dupsb)
 				set_buffer_dirty(nilfs->ns_sbh[1]);
 				if (!sync_dirty_buffer(nilfs->ns_sbh[1]))
 					sbp = nilfs->ns_sbp[1];
+			} else {
+				int flip_bits = (nilfs->ns_cno & 0x0FL);
+				/* flip super block 9 to 7 ratio */
+				if (flip_bits == 0x08 || flip_bits == 0x0F) {
+					nilfs_swap_super_block(nilfs);
+					sbp = nilfs->ns_sbp[1];
+				}
 			}
 		}
 		if (sbp) {
@@ -275,7 +284,7 @@ int nilfs_prepare_super(struct nilfs_sb_info *sbi)
 	return 0;
 }
 
-int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
+int nilfs_update_super(struct nilfs_sb_info *sbi)
 {
 	struct the_nilfs *nilfs = sbi->s_nilfs;
 	struct nilfs_super_block **sbp = nilfs->ns_sbp;
@@ -303,9 +312,31 @@ int nilfs_commit_super(struct nilfs_sb_info *sbi, int dupsb)
 	sbp[0]->s_sum = cpu_to_le32(crc32_le(nilfs->ns_crc_seed,
 					     (unsigned char *)sbp[0],
 					     nilfs->ns_sbsize));
+	return 0;
+}
+
+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;
+
+	/* nilfs->sem must be locked by the caller. */
 	if (dupsb && sbp[1]) {
-		memcpy(sbp[1], sbp[0], nilfs->ns_sbsize);
-		nilfs->ns_sbwtime[1] = t;
+		nilfs->ns_sbwtime[1] = nilfs->ns_sbwtime[0];
+		/* copy super block except s_last_cno, s_last_pseg,
+		   s_last_seq, s_free_blocks_count.
+		  Those are asymmetric members */
+		const int ctimeoff =
+			offsetof(struct nilfs_super_block, s_ctime);
+		memcpy(sbp[1], sbp[0],
+		       offsetof(struct nilfs_super_block, s_last_cno));
+		memcpy((unsigned char *)sbp[1] + ctimeoff,
+		       (unsigned char *)sbp[0] + ctimeoff,
+		       nilfs->ns_sbsize - ctimeoff);
+		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);
@@ -352,8 +383,11 @@ 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) && likely(!nilfs_prepare_super(sbi)))
-		nilfs_commit_super(sbi, 1);
+	if (nilfs_sb_dirty(nilfs) && likely(!nilfs_prepare_super(sbi))) {
+		err = nilfs_update_super(sbi);
+		if (likely(!err))
+			nilfs_commit_super(sbi, 0);
+	}
 	up_write(&nilfs->ns_sem);
 
 	return 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-06-12 19:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-08 22:15 [PATCH 0/2] nilfs2: asynchronous super block updates Jiro SEKIBA
     [not found] ` <1276035358-24284-1-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
2010-06-08 22:15   ` [PATCH 1/2] nilfs2: introduce nilfs_prepare_super Jiro SEKIBA
2010-06-08 22:15   ` [PATCH 2/2] nilfs2: sync super blocks in turns Jiro SEKIBA
2010-06-09  1:36   ` [PATCH 0/2] nilfs2: asynchronous super block updates Ryusuke Konishi
  -- strict thread matches above, loose matches on Subject: below --
2010-06-11 14:30 [PATCH 0/2v2] " Jiro SEKIBA
     [not found] ` <1276266639-15913-1-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
2010-06-11 14:30   ` [PATCH 2/2] nilfs2: sync super blocks in turns Jiro SEKIBA
     [not found]     ` <1276266639-15913-3-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
2010-06-12 19:06       ` Ryusuke Konishi
2010-06-05 22:19 [PATCH 0/2] nilfs2: asynchronous super block updates Jiro SEKIBA
     [not found] ` <1275776349-22820-1-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
2010-06-05 22:19   ` [PATCH 2/2] nilfs2: sync super blocks in turns Jiro SEKIBA
     [not found]     ` <1275776349-22820-3-git-send-email-jir-hfpbi5WX9J54Eiagz67IpQ@public.gmane.org>
2010-06-07  6:15       ` Ryusuke Konishi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.