scoutfs: clean up super block use

The code that works with the super block had drifted a bit.  We still
had two from an old design and we weren't doing anything with its crc.

Move to only using one super block at a fixed blkno and store and verify
its crc field by sharing code with the btree block checksumming.

Signed-off-by: Zach Brown <zab@versity.com>

kmod/src/Makefile

@@ -5,7 +5,7 @@ CFLAGS_super.o = -DSCOUTFS_GIT_DESCRIBE=\"$(SCOUTFS_GIT_DESCRIBE)\" \
 
 CFLAGS_scoutfs_trace.o = -I$(src) # define_trace.h double include
  
-scoutfs-y += bio.o btree.o client.o compact.o counters.o data.o dir.o \
+scoutfs-y += bio.o block.o btree.o client.o compact.o counters.o data.o dir.o \
 	     export.o extents.o file.o inode.o ioctl.o item.o lock.o \
 	     manifest.o msg.o options.o per_task.o seg.o server.o \
 	     scoutfs_trace.o sock.o sort_priv.o super.o sysfs.o trans.o \

kmod/src/block.c

@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2018 Versity Software, Inc.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+#include <linux/kernel.h>
+#include <linux/fs.h>
+#include <linux/crc32c.h>
+
+#include "format.h"
+#include "super.h"
+#include "block.h"
+
+__le32 scoutfs_block_calc_crc(struct scoutfs_block_header *hdr)
+{
+	int off = offsetof(struct scoutfs_block_header, crc) +
+		  FIELD_SIZEOF(struct scoutfs_block_header, crc);
+	u32 calc = crc32c(~0, (char *)hdr + off, SCOUTFS_BLOCK_SIZE - off);
+
+	return cpu_to_le32(calc);
+}
+
+bool scoutfs_block_valid_crc(struct scoutfs_block_header *hdr)
+{
+	return hdr->crc == scoutfs_block_calc_crc(hdr);
+}
+
+bool scoutfs_block_valid_ref(struct super_block *sb,
+			     struct scoutfs_block_header *hdr,
+			     __le64 seq, __le64 blkno)
+{
+	struct scoutfs_super_block *super = &SCOUTFS_SB(sb)->super;
+
+	return hdr->fsid == super->hdr.fsid && hdr->seq == seq &&
+	       hdr->blkno == blkno;
+}

kmod/src/block.h

@@ -0,0 +1,10 @@
+#ifndef _SCOUTFS_BLOCK_H_
+#define _SCOUTFS_BLOCK_H_
+
+__le32 scoutfs_block_calc_crc(struct scoutfs_block_header *hdr);
+bool scoutfs_block_valid_crc(struct scoutfs_block_header *hdr);
+bool scoutfs_block_valid_ref(struct super_block *sb,
+			     struct scoutfs_block_header *hdr,
+			     __le64 seq, __le64 blkno);
+
+#endif

kmod/src/btree.c

@@ -28,6 +28,7 @@
 #include "triggers.h"
 #include "options.h"
 #include "msg.h"
+#include "block.h"
 
 #include "scoutfs_trace.h"
 
@@ -603,24 +604,16 @@ BUFFER_FNS(ScoutfsValidCrc, scoutfs_valid_crc)	/* crc matched */
  * Make sure that we've found a valid block and that it's the block that
  * we're looking for.
  */
-static bool valid_referenced_block(struct scoutfs_super_block *super,
+static bool valid_referenced_block(struct super_block *sb,
 				   struct scoutfs_btree_ref *ref,
 				   struct scoutfs_btree_block *bt,
 				   struct buffer_head *bh)
 {
-	__le32 existing;
-	u32 calc;
-
 	smp_rmb(); /* load checked before crc */
 	if (!buffer_scoutfs_checked(bh)) {
 		lock_buffer(bh);
 		if (!buffer_scoutfs_checked(bh)) {
-			existing = bt->crc;
-			bt->crc = 0;
-			calc = crc32c(~0, bt, SCOUTFS_BLOCK_SIZE);
-			bt->crc = existing;
-
-			if (calc == le32_to_cpu(existing))
+			if (scoutfs_block_valid_crc(&bt->hdr))
 				set_buffer_scoutfs_valid_crc(bh);
 			else
 				clear_buffer_scoutfs_valid_crc(bh);
@@ -631,8 +624,8 @@ static bool valid_referenced_block(struct scoutfs_super_block *super,
 		unlock_buffer(bh);
 	}
 
-	return buffer_scoutfs_valid_crc(bh) && super->hdr.fsid == bt->fsid &&
-	       ref->blkno == bt->blkno && ref->seq == bt->seq;
+	return buffer_scoutfs_valid_crc(bh) &&
+	       scoutfs_block_valid_ref(sb, &bt->hdr, ref->seq, ref->blkno);
 }
 
 /*
@@ -681,7 +674,7 @@ retry:
 		}
 		bt = (void *)bh->b_data;
 
-		if (!valid_referenced_block(super, ref, bt, bh) ||
+		if (!valid_referenced_block(sb, ref, bt, bh) ||
 		    scoutfs_trigger(sb, BTREE_STALE_READ)) {
 
 			scoutfs_inc_counter(sb, btree_stale_read);
@@ -706,7 +699,7 @@ retry:
 
 		/* done if not dirtying or already dirty */
 		if (!(flags & BTW_DIRTY) ||
-		    (le64_to_cpu(bt->seq) >= bti->first_dirty_seq)) {
+		    (le64_to_cpu(bt->hdr.seq) >= bti->first_dirty_seq)) {
 			ret = 0;
 			goto out;
 		}
@@ -784,15 +777,15 @@ retry:
 		bt = new;
 		new = NULL;
 		memset(bt, 0, SCOUTFS_BLOCK_SIZE);
-		bt->fsid = super->hdr.fsid;
+		bt->hdr.fsid = super->hdr.fsid;
 		bt->free_end = cpu_to_le16(SCOUTFS_BLOCK_SIZE);
 	}
 
-	bt->blkno = cpu_to_le64(blkno);
-	bt->seq = cpu_to_le64(seq);
+	bt->hdr.blkno = cpu_to_le64(blkno);
+	bt->hdr.seq = cpu_to_le64(seq);
 	if (ref) {
-		ref->blkno = bt->blkno;
-		ref->seq = bt->seq;
+		ref->blkno = bt->hdr.blkno;
+		ref->seq = bt->hdr.seq;
 	}
 	ret = 0;
 
@@ -816,8 +809,8 @@ static void create_parent_item(struct scoutfs_btree_ring *bring,
 			       void *key, unsigned key_len)
 {
 	struct scoutfs_btree_ref ref = {
-		.blkno = child->blkno,
-		.seq = child->seq,
+		.blkno = child->hdr.blkno,
+		.seq = child->hdr.seq,
 	};
 
 	create_item(parent, pos, key, key_len, &ref, sizeof(ref));
@@ -888,8 +881,8 @@ static int try_split(struct super_block *sb, struct scoutfs_btree_root *root,
 
 		parent->level = root->height;
 		root->height++;
-		root->ref.blkno = parent->blkno;
-		root->ref.seq = parent->seq;
+		root->ref.blkno = parent->hdr.blkno;
+		root->ref.seq = parent->hdr.seq;
 
 		pos = 0;
 		create_parent_item(bring, parent, pos, right,
@@ -975,8 +968,8 @@ static int try_merge(struct super_block *sb, struct scoutfs_btree_root *root,
 	/* and finally shrink the tree if our parent is the root with 1 */
 	if (le16_to_cpu(parent->nr_items) == 1) {
 		root->height--;
-		root->ref.blkno = bt->blkno;
-		root->ref.seq = bt->seq;
+		root->ref.blkno = bt->hdr.blkno;
+		root->ref.seq = bt->hdr.seq;
 	}
 
 	put_btree_block(sib);
@@ -1041,7 +1034,7 @@ static int verify_btree_block(struct scoutfs_btree_block *bt, int level)
 out:
 	if (bad) {
 		printk("bt %p blkno %llu level %d end %u reclaim %u nr %u (after %u bytes %u)\n",
-			bt, le64_to_cpu(bt->blkno), level,
+			bt, le64_to_cpu(bt->hdr.blkno), level,
 			le16_to_cpu(bt->free_end),
 			le16_to_cpu(bt->free_reclaim), le16_to_cpu(bt->nr_items),
 			after_off, bytes);
@@ -1160,8 +1153,8 @@ restart:
 					   root->height,
 					   le64_to_cpu(root->ref.blkno),
 					   le64_to_cpu(root->ref.seq),
-					   le64_to_cpu(bt->blkno),
-					   le64_to_cpu(bt->seq), bt->level,
+					   le64_to_cpu(bt->hdr.blkno),
+					   le64_to_cpu(bt->hdr.seq), bt->level,
 					   level);
 			ret = -EIO;
 			break;
@@ -1201,8 +1194,8 @@ restart:
 					   root->height,
 					   le64_to_cpu(root->ref.blkno),
 					   le64_to_cpu(root->ref.seq),
-					   le64_to_cpu(bt->blkno),
-					   le64_to_cpu(bt->seq), bt->level,
+					   le64_to_cpu(bt->hdr.blkno),
+					   le64_to_cpu(bt->hdr.seq), bt->level,
 					   nr, pos, cmp);
 			ret = -EIO;
 			break;
@@ -1653,8 +1646,8 @@ int scoutfs_btree_write_dirty(struct super_block *sb)
 	/* checksum everything to reduce time between io submission merging */
 	for_each_dirty_bh(bti, bh, tmp) {
 		bt = (void *)bh->b_data;
-		bt->crc = 0;
-		bt->crc = cpu_to_le32(crc32c(~0, bt, SCOUTFS_BLOCK_SIZE));
+		bt->hdr._pad = 0;
+		bt->hdr.crc = scoutfs_block_calc_crc(&bt->hdr);
 	}
 
         blk_start_plug(&plug);

kmod/src/client.c

@@ -271,7 +271,7 @@ static int client_connect(struct client_info *client)
 			break;
 		}
 
-		ret = scoutfs_read_supers(sb, &super);
+		ret = scoutfs_read_super(sb, &super);
 		if (ret)
 			continue;
 

kmod/src/format.h

@@ -33,17 +33,15 @@
 #define SCOUTFS_BLOCK_PAGE_ORDER (SCOUTFS_BLOCK_SHIFT - PAGE_SHIFT)
 
 /*
- * The super blocks leave some room at the start of the first block for
- * platform structures like boot loaders.
+ * The super block leaves some room before the first block for platform
+ * structures like boot loaders.
  */
-#define SCOUTFS_SUPER_BLKNO ((64 * 1024) >> SCOUTFS_BLOCK_SHIFT)
-#define SCOUTFS_SUPER_NR 2
+#define SCOUTFS_SUPER_BLKNO ((64ULL * 1024) >> SCOUTFS_BLOCK_SHIFT)
 
 /*
- * This header is found at the start of every block so that we can
- * verify that it's what we were looking for.  The crc and padding
- * starts the block so that its calculation operations on a nice 64bit
- * aligned region.
+ * This header is stored at the start of btree blocks and the super
+ * block for verification.  The crc is calculated by zeroing the crc and
+ * padding so the buffer is large and aligned.
  */
 struct scoutfs_block_header {
 	__le32 crc;
@@ -183,11 +181,7 @@ struct scoutfs_btree_item {
 } __packed;
 
 struct scoutfs_btree_block {
-	__le64 fsid;
-	__le64 blkno;
-	__le64 seq;
-	__le32 crc;
-	__le32 _pad;
+	struct scoutfs_block_header hdr;
 	__le16 free_end;
 	__le16 free_reclaim;
 	__le16 nr_items;

kmod/src/server.c

@@ -1340,7 +1340,7 @@ static void scoutfs_server_func(struct work_struct *work)
 		goto out;
 
 	/* publish the address for clients to connect to */
-	ret = scoutfs_read_supers(sb, super);
+	ret = scoutfs_read_super(sb, super);
 	if (ret)
 		goto out;
 

kmod/src/super.c

@@ -22,6 +22,7 @@
 #include <linux/debugfs.h>
 
 #include "super.h"
+#include "block.h"
 #include "export.h"
 #include "format.h"
 #include "inode.h"
@@ -155,7 +156,7 @@ static const struct super_operations scoutfs_super_ops = {
 };
 
 /*
- * The caller advances the block number and sequence number in the super
+ * The caller advances the sequence number in the super block header
  * every time it wants to dirty it and eventually write it to reference
  * dirty data that's been written.
  */
@@ -164,13 +165,7 @@ void scoutfs_advance_dirty_super(struct super_block *sb)
 	struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb);
 	struct scoutfs_super_block *super = &sbi->super;
 
-	le64_add_cpu(&super->hdr.blkno, 1);
-	if (le64_to_cpu(super->hdr.blkno) == (SCOUTFS_SUPER_BLKNO +
-					      SCOUTFS_SUPER_NR))
-		super->hdr.blkno = cpu_to_le64(SCOUTFS_SUPER_BLKNO);
-
 	le64_add_cpu(&super->hdr.seq, 1);
-
 	trace_scoutfs_advance_dirty_super(sb, le64_to_cpu(super->hdr.seq));
 }
 
@@ -193,6 +188,8 @@ int scoutfs_write_dirty_super(struct super_block *sb)
 
 	super = page_address(page);
 	memcpy(super, &sbi->super, sizeof(*super));
+	super->hdr._pad = 0;
+	super->hdr.crc = scoutfs_block_calc_crc(&super->hdr);
 
 	ret = scoutfs_bio_write(sb, &page, le64_to_cpu(super->hdr.blkno), 1);
 	WARN_ON_ONCE(ret);
@@ -203,67 +200,64 @@ int scoutfs_write_dirty_super(struct super_block *sb)
 }
 
 /*
- * Read the pair of super blocks and store the most recent one in the sb
- * info.  Clients reference but don't modify the super.  The server has
- * to re-read the super every time it comes up so that it can work from
- * the most recent persistent state.
+ * Read the super block.  If it's valid store it in the caller's super
+ * struct.
  */
-int scoutfs_read_supers(struct super_block *sb,
-			struct scoutfs_super_block *local)
+int scoutfs_read_super(struct super_block *sb,
+		       struct scoutfs_super_block *super_res)
 {
-	struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb);
 	struct scoutfs_super_block *super;
 	struct page *page;
-	int found = -1;
+	__le32 calc;
 	int ret;
-	int i;
-	u64 seq = 0;
 
 	page = alloc_page(GFP_KERNEL);
 	if (!page)
 		return -ENOMEM;
 
-	for (i = 0; i < SCOUTFS_SUPER_NR; i++) {
-
-		ret = scoutfs_bio_read(sb, &page, SCOUTFS_SUPER_BLKNO + i, 1);
-		if (ret) {
-			scoutfs_warn(sb, "couldn't read super block %u", i);
-			continue;
-		}
-
-		super = scoutfs_page_block_address(&page, 0);
-
-		if (super->id != cpu_to_le64(SCOUTFS_SUPER_ID)) {
-			scoutfs_warn(sb, "super block %u has invalid id %llx",
-				     i, le64_to_cpu(super->id));
-			continue;
-		}
-
-		if (super->format_hash != cpu_to_le64(SCOUTFS_FORMAT_HASH)) {
-			scoutfs_warn(sb, "super block %u has invalid format hash 0x%llx, expected 0x%llx",
-				     i, le64_to_cpu(super->format_hash),
-				     SCOUTFS_FORMAT_HASH);
-			continue;
-		}
-
-		if (found < 0 || (le64_to_cpu(super->hdr.seq) > seq)) {
-			*local = *super;
-			seq = le64_to_cpu((*local).hdr.seq);
-			found = i;
-		}
+	ret = scoutfs_bio_read(sb, &page, SCOUTFS_SUPER_BLKNO, 1);
+	if (ret) {
+		scoutfs_err(sb, "error reading super block: %d", ret);
+		goto out;
 	}
 
+	super = scoutfs_page_block_address(&page, 0);
+
+	if (super->id != cpu_to_le64(SCOUTFS_SUPER_ID)) {
+		scoutfs_err(sb, "super block has invalid id %llx",
+			    le64_to_cpu(super->id));
+		ret = -EINVAL;
+		goto out;
+	}
+
+	calc = scoutfs_block_calc_crc(&super->hdr);
+	if (calc != super->hdr.crc) {
+		scoutfs_err(sb, "super block has invalid crc 0x%08x, calculated 0x%08x",
+			    le32_to_cpu(super->hdr.crc), le32_to_cpu(calc));
+		ret = -EINVAL;
+	}
+
+	if (le64_to_cpu(super->hdr.blkno) != SCOUTFS_SUPER_BLKNO) {
+		scoutfs_err(sb, "super block has invalid block number %llu, data read from %llu",
+		le64_to_cpu(super->hdr.blkno), SCOUTFS_SUPER_BLKNO);
+		ret = -EINVAL;
+		goto out;
+	}
+
+
+	if (super->format_hash != cpu_to_le64(SCOUTFS_FORMAT_HASH)) {
+		scoutfs_err(sb, "super block has invalid format hash 0x%llx, expected 0x%llx",
+			    le64_to_cpu(super->format_hash),
+			    SCOUTFS_FORMAT_HASH);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	*super_res = *super;
+	ret = 0;
+out:
 	__free_page(page);
-
-	if (found < 0) {
-		scoutfs_err(sb, "unable to read valid super block");
-		return -EINVAL;
-	}
-
-	scoutfs_info(sb, "using super %u with seq %llu",
-		     found, le64_to_cpu(sbi->super.hdr.seq));
-
-	return 0;
+	return ret;
 }
 
 static int scoutfs_debugfs_setup(struct super_block *sb)
@@ -328,7 +322,7 @@ static int scoutfs_fill_super(struct super_block *sb, void *data, int silent)
 
 	ret = scoutfs_setup_sysfs(sb) ?:
 	      scoutfs_setup_counters(sb) ?:
-	      scoutfs_read_supers(sb, &SCOUTFS_SB(sb)->super) ?:
+	      scoutfs_read_super(sb, &SCOUTFS_SB(sb)->super) ?:
 	      scoutfs_debugfs_setup(sb) ?:
 	      scoutfs_options_setup(sb) ?:
 	      scoutfs_setup_triggers(sb) ?:

kmod/src/super.h

@@ -79,8 +79,8 @@ static inline struct scoutfs_sb_info *SCOUTFS_SB(struct super_block *sb)
 	return sb->s_fs_info;
 }
 
-int scoutfs_read_supers(struct super_block *sb,
-			struct scoutfs_super_block *local);
+int scoutfs_read_super(struct super_block *sb,
+		       struct scoutfs_super_block *super_res);
 void scoutfs_advance_dirty_super(struct super_block *sb);
 int scoutfs_write_dirty_super(struct super_block *sb);