From e19716a0f22386286a97c1618be5d0ec58558f76 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Thu, 28 Jun 2018 15:09:28 -0700 Subject: [PATCH] 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 --- kmod/src/Makefile | 2 +- kmod/src/block.c | 43 +++++++++++++++++++ kmod/src/block.h | 10 +++++ kmod/src/btree.c | 57 +++++++++++-------------- kmod/src/client.c | 2 +- kmod/src/format.h | 20 +++------ kmod/src/server.c | 2 +- kmod/src/super.c | 106 ++++++++++++++++++++++------------------------ kmod/src/super.h | 4 +- 9 files changed, 140 insertions(+), 106 deletions(-) create mode 100644 kmod/src/block.c create mode 100644 kmod/src/block.h diff --git a/kmod/src/Makefile b/kmod/src/Makefile index 73589b6c..c9375a0b 100644 --- a/kmod/src/Makefile +++ b/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 \ diff --git a/kmod/src/block.c b/kmod/src/block.c new file mode 100644 index 00000000..bfe07e16 --- /dev/null +++ b/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 +#include +#include + +#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; +} diff --git a/kmod/src/block.h b/kmod/src/block.h new file mode 100644 index 00000000..5bf42331 --- /dev/null +++ b/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 diff --git a/kmod/src/btree.c b/kmod/src/btree.c index 028c81c3..eee9c547 100644 --- a/kmod/src/btree.c +++ b/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); diff --git a/kmod/src/client.c b/kmod/src/client.c index fe837420..69b12248 100644 --- a/kmod/src/client.c +++ b/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; diff --git a/kmod/src/format.h b/kmod/src/format.h index 65c10b69..9addabf5 100644 --- a/kmod/src/format.h +++ b/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; diff --git a/kmod/src/server.c b/kmod/src/server.c index 6eed54ca..a88f8721 100644 --- a/kmod/src/server.c +++ b/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; diff --git a/kmod/src/super.c b/kmod/src/super.c index aaea4cba..8e7e12fd 100644 --- a/kmod/src/super.c +++ b/kmod/src/super.c @@ -22,6 +22,7 @@ #include #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) ?: diff --git a/kmod/src/super.h b/kmod/src/super.h index fdc15efc..8e9cece1 100644 --- a/kmod/src/super.h +++ b/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);