REVIEW: Change mkfs.xfs to set primary superblock inodes in ALL secondaries

REVIEW: Change mkfs.xfs to set primary superblock inodes in ALL secondaries

[Barry Naujok]

One peculiarity of mkfs.xfs that no-one has yet been able to explain
to me is that all the secondary superblocks do not contain the
primary superblock's root inode, realtime inodes and quota inodes.

The root inode is stored in the middle and last AG as well to
make things more unexpected.

The following makes all the secondaries the same as the primary
(other than the global counters).

---
  xfsprogs/mkfs/xfs_mkfs.c |   45  
+++++++++++++++------------------------------
  xfstests/030.out.linux   |   12 ------------
  xfstests/178.out         |   12 ------------
  3 files changed, 15 insertions(+), 54 deletions(-)

Index: ci/xfsprogs/mkfs/xfs_mkfs.c
===================================================================
--- ci.orig/xfsprogs/mkfs/xfs_mkfs.c
+++ ci/xfsprogs/mkfs/xfs_mkfs.c
@@ -2397,33 +2397,20 @@ an AG size that is one stripe unit small
  	}

  	/*
-	 * Write out multiple secondary superblocks with rootinode field set
+	 * Write out secondary superblocks with inode fields set
  	 */
-	if (mp->m_sb.sb_agcount > 1) {
-		/*
-		 * the last superblock
-		 */
-		buf = libxfs_readbuf(mp->m_dev,
-				XFS_AGB_TO_DADDR(mp, mp->m_sb.sb_agcount-1,
-					XFS_SB_DADDR),
-				XFS_FSS_TO_BB(mp, 1),
-				LIBXFS_EXIT_ON_FAILURE);
-		INT_SET((XFS_BUF_TO_SBP(buf))->sb_rootino,
-				ARCH_CONVERT, mp->m_sb.sb_rootino);
-		libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
-		/*
-		 * and one in the middle for luck
-		 */
-		if (mp->m_sb.sb_agcount > 2) {
-			buf = libxfs_readbuf(mp->m_dev,
-				XFS_AGB_TO_DADDR(mp, (mp->m_sb.sb_agcount-1)/2,
-					XFS_SB_DADDR),
-				XFS_FSS_TO_BB(mp, 1),
-				LIBXFS_EXIT_ON_FAILURE);
-			INT_SET((XFS_BUF_TO_SBP(buf))->sb_rootino,
-				ARCH_CONVERT, mp->m_sb.sb_rootino);
-			libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
-		}
+	buf = libxfs_getsb(mp, LIBXFS_EXIT_ON_FAILURE);
+	XFS_BUF_TO_SBP(buf)->sb_inprogress = 0;
+
+	for (agno = 1; agno < mp->m_sb.sb_agcount; agno++) {
+		xfs_buf_t	*sbuf;
+
+		sbuf = libxfs_getbuf(mp->m_dev,
+				XFS_AGB_TO_DADDR(mp, agno, XFS_SB_DADDR),
+				XFS_FSS_TO_BB(mp, 1));
+		memcpy(XFS_BUF_PTR(sbuf), XFS_BUF_PTR(buf),
+				XFS_BUF_SIZE(sbuf));
+		libxfs_writebuf(sbuf, LIBXFS_EXIT_ON_FAILURE);
  	}

  	/*
@@ -2432,13 +2419,11 @@ an AG size that is one stripe unit small
  	 */
  	libxfs_rtmount_destroy(mp);
  	libxfs_icache_purge();
-	libxfs_bcache_purge();
+	libxfs_bcache_flush();

  	/*
-	 * Mark the filesystem ok.
+	 * Finalize the filesystem (sb_inprogress = 0 from above).
  	 */
-	buf = libxfs_getsb(mp, LIBXFS_EXIT_ON_FAILURE);
-	(XFS_BUF_TO_SBP(buf))->sb_inprogress = 0;
  	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);

  	libxfs_umount(mp);
Index: ci/xfstests/030.out.linux
===================================================================
--- ci.orig/xfstests/030.out.linux
+++ ci/xfstests/030.out.linux
@@ -14,12 +14,6 @@ attempting to find secondary superblock.
  found candidate secondary superblock...
  verified secondary superblock...
  writing modified primary superblock
-sb root inode value INO inconsistent with calculated value INO
-resetting superblock root inode pointer to INO
-sb realtime bitmap inode INO inconsistent with calculated value INO
-resetting superblock realtime bitmap ino pointer to INO
-sb realtime summary inode INO inconsistent with calculated value INO
-resetting superblock realtime summary ino pointer to INO
  Phase 2 - using <TYPEOF> log
          - zero log...
          - scan filesystem freespace and inode maps...
@@ -132,12 +126,6 @@ attempting to find secondary superblock.
  found candidate secondary superblock...
  verified secondary superblock...
  writing modified primary superblock
-sb root inode value INO inconsistent with calculated value INO
-resetting superblock root inode pointer to INO
-sb realtime bitmap inode INO inconsistent with calculated value INO
-resetting superblock realtime bitmap ino pointer to INO
-sb realtime summary inode INO inconsistent with calculated value INO
-resetting superblock realtime summary ino pointer to INO
  Phase 2 - using <TYPEOF> log
          - zero log...
          - scan filesystem freespace and inode maps...
Index: ci/xfstests/178.out
===================================================================
--- ci.orig/xfstests/178.out
+++ ci/xfstests/178.out
@@ -12,12 +12,6 @@ attempting to find secondary superblock.
  found candidate secondary superblock...
  verified secondary superblock...
  writing modified primary superblock
-sb root inode value INO inconsistent with calculated value INO
-resetting superblock root inode pointer to INO
-sb realtime bitmap inode INO inconsistent with calculated value INO
-resetting superblock realtime bitmap ino pointer to INO
-sb realtime summary inode INO inconsistent with calculated value INO
-resetting superblock realtime summary ino pointer to INO
  Phase 2 - using <TYPEOF> log
          - zero log...
          - scan filesystem freespace and inode maps...
@@ -48,12 +42,6 @@ attempting to find secondary superblock.
  found candidate secondary superblock...
  verified secondary superblock...
  writing modified primary superblock
-sb root inode value INO inconsistent with calculated value INO
-resetting superblock root inode pointer to INO
-sb realtime bitmap inode INO inconsistent with calculated value INO
-resetting superblock realtime bitmap ino pointer to INO
-sb realtime summary inode INO inconsistent with calculated value INO
-resetting superblock realtime summary ino pointer to INO
  Phase 2 - using <TYPEOF> log
          - zero log...
          - scan filesystem freespace and inode maps...

Re: REVIEW: Change mkfs.xfs to set primary superblock inodes in ALL secondaries

[Christoph Hellwig]

On Wed, Sep 03, 2008 at 04:57:30PM +1000, Barry Naujok wrote:
> One peculiarity of mkfs.xfs that no-one has yet been able to explain
> to me is that all the secondary superblocks do not contain the
> primary superblock's root inode, realtime inodes and quota inodes.
>
> The root inode is stored in the middle and last AG as well to
> make things more unexpected.
>
> The following makes all the secondaries the same as the primary
> (other than the global counters).

Looks good to me, but I'd really love to know the original reason behind
it, too..

Re: REVIEW: Change mkfs.xfs to set primary superblock inodes in ALL secondaries

[Eric Sandeen]

Barry Naujok wrote:
> One peculiarity of mkfs.xfs that no-one has yet been able to explain
> to me is that all the secondary superblocks do not contain the
> primary superblock's root inode, realtime inodes and quota inodes.
> 
> The root inode is stored in the middle and last AG as well to
> make things more unexpected.
> 
> The following makes all the secondaries the same as the primary
> (other than the global counters).

Also looks good to me.

I've always wondered, too, if writing a single backup superblock in the
last sector(s) of the device at mkfs/growfs time might be sane?

When the primary is corrupt you could quickly get the size of the
device, seek to the end, read on the last 1k boundary, see if it's a
superblock, and use that as the first easily-findable backup.

Just a thought :)

-Eric