All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] f2fs: support superblock checksum
@ 2018-09-19 13:53 Junling Zheng
  2018-09-19 13:53 ` [PATCH 2/5] f2fs-tools: rename CHECKSUM_OFFSET to CP_CHKSUM_OFFSET Junling Zheng
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Junling Zheng @ 2018-09-19 13:53 UTC (permalink / raw)
  To: jaegeuk, yuchao0; +Cc: miaoxie, linux-f2fs-devel

Now we support crc32 checksum for superblock.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
---
 fs/f2fs/f2fs.h          |  2 ++
 fs/f2fs/super.c         | 29 +++++++++++++++++++++++++++++
 fs/f2fs/sysfs.c         |  7 +++++++
 include/linux/f2fs_fs.h |  3 ++-
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4525f4f82af0..d50d6efda96b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -147,6 +147,7 @@ struct f2fs_mount_info {
 #define F2FS_FEATURE_INODE_CRTIME	0x0100
 #define F2FS_FEATURE_LOST_FOUND		0x0200
 #define F2FS_FEATURE_VERITY		0x0400	/* reserved */
+#define F2FS_FEATURE_SB_CHKSUM		0x0800
 
 #define F2FS_HAS_FEATURE(sb, mask)					\
 	((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
@@ -3376,6 +3377,7 @@ F2FS_FEATURE_FUNCS(flexible_inline_xattr, FLEXIBLE_INLINE_XATTR);
 F2FS_FEATURE_FUNCS(quota_ino, QUOTA_INO);
 F2FS_FEATURE_FUNCS(inode_crtime, INODE_CRTIME);
 F2FS_FEATURE_FUNCS(lost_found, LOST_FOUND);
+F2FS_FEATURE_FUNCS(sb_chksum, SB_CHKSUM);
 
 #ifdef CONFIG_BLK_DEV_ZONED
 static inline int get_blkz_type(struct f2fs_sb_info *sbi,
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bd57be470e23..3ffc336caea8 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2149,6 +2149,26 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
 					(bh->b_data + F2FS_SUPER_OFFSET);
 	struct super_block *sb = sbi->sb;
 	unsigned int blocksize;
+	size_t crc_offset = 0;
+	__u32 crc = 0;
+
+	/* Check checksum_offset and crc in superblock */
+	if (le32_to_cpu(raw_super->feature) & F2FS_FEATURE_SB_CHKSUM) {
+		crc_offset = le32_to_cpu(raw_super->checksum_offset);
+		if (crc_offset !=
+			offsetof(struct f2fs_super_block, crc)) {
+			f2fs_msg(sb, KERN_INFO,
+				"Invalid SB checksum offset: %zu",
+				crc_offset);
+			return 1;
+		}
+		crc = le32_to_cpu(raw_super->crc);
+		if (!f2fs_crc_valid(sbi, crc, raw_super, crc_offset)) {
+			f2fs_msg(sb, KERN_INFO,
+				"Invalid SB checksum value: %u", crc);
+			return 1;
+		}
+	}
 
 	if (F2FS_SUPER_MAGIC != le32_to_cpu(raw_super->magic)) {
 		f2fs_msg(sb, KERN_INFO,
@@ -2568,6 +2588,7 @@ static int read_raw_super_block(struct f2fs_sb_info *sbi,
 int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
 {
 	struct buffer_head *bh;
+	__u32 crc = 0;
 	int err;
 
 	if ((recover && f2fs_readonly(sbi->sb)) ||
@@ -2576,6 +2597,13 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
 		return -EROFS;
 	}
 
+	/* we should update superblock crc here */
+	if (!recover && f2fs_sb_has_sb_chksum(sbi->sb)) {
+		crc = f2fs_crc32(sbi, F2FS_RAW_SUPER(sbi),
+				offsetof(struct f2fs_super_block, crc));
+		F2FS_RAW_SUPER(sbi)->crc = cpu_to_le32(crc);
+	}
+
 	/* write back-up superblock first */
 	bh = sb_bread(sbi->sb, sbi->valid_super_block ? 0 : 1);
 	if (!bh)
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index cd2e030e47b8..c86d91be6c48 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -120,6 +120,9 @@ static ssize_t features_show(struct f2fs_attr *a,
 	if (f2fs_sb_has_lost_found(sb))
 		len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
 				len ? ", " : "", "lost_found");
+	if (f2fs_sb_has_sb_chksum(sb))
+		len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
+				len ? ", " : "", "sb_checksum");
 	len += snprintf(buf + len, PAGE_SIZE - len, "\n");
 	return len;
 }
@@ -337,6 +340,7 @@ enum feat_id {
 	FEAT_QUOTA_INO,
 	FEAT_INODE_CRTIME,
 	FEAT_LOST_FOUND,
+	FEAT_SB_CHECKSUM,
 };
 
 static ssize_t f2fs_feature_show(struct f2fs_attr *a,
@@ -353,6 +357,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
 	case FEAT_QUOTA_INO:
 	case FEAT_INODE_CRTIME:
 	case FEAT_LOST_FOUND:
+	case FEAT_SB_CHECKSUM:
 		return snprintf(buf, PAGE_SIZE, "supported\n");
 	}
 	return 0;
@@ -433,6 +438,7 @@ F2FS_FEATURE_RO_ATTR(flexible_inline_xattr, FEAT_FLEXIBLE_INLINE_XATTR);
 F2FS_FEATURE_RO_ATTR(quota_ino, FEAT_QUOTA_INO);
 F2FS_FEATURE_RO_ATTR(inode_crtime, FEAT_INODE_CRTIME);
 F2FS_FEATURE_RO_ATTR(lost_found, FEAT_LOST_FOUND);
+F2FS_FEATURE_RO_ATTR(sb_checksum, FEAT_SB_CHECKSUM);
 
 #define ATTR_LIST(name) (&f2fs_attr_##name.attr)
 static struct attribute *f2fs_attrs[] = {
@@ -489,6 +495,7 @@ static struct attribute *f2fs_feat_attrs[] = {
 	ATTR_LIST(quota_ino),
 	ATTR_LIST(inode_crtime),
 	ATTR_LIST(lost_found),
+	ATTR_LIST(sb_checksum),
 	NULL,
 };
 
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index f70f8ac9c4f4..aa4b586569c1 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -112,7 +112,8 @@ struct f2fs_super_block {
 	struct f2fs_device devs[MAX_DEVICES];	/* device list */
 	__le32 qf_ino[F2FS_MAX_QUOTAS];	/* quota inode numbers */
 	__u8 hot_ext_count;		/* # of hot file extension */
-	__u8 reserved[314];		/* valid reserved region */
+	__u8 reserved[310];		/* valid reserved region */
+	__le32 crc;			/* checksum of superblock */
 } __packed;
 
 /*
-- 
2.18.0

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

* [PATCH 2/5] f2fs-tools: rename CHECKSUM_OFFSET to CP_CHKSUM_OFFSET
  2018-09-19 13:53 [PATCH 1/5] f2fs: support superblock checksum Junling Zheng
@ 2018-09-19 13:53 ` Junling Zheng
  2018-09-19 13:53 ` [PATCH 3/5] fsck.f2fs: unify the updating of superblocks Junling Zheng
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Junling Zheng @ 2018-09-19 13:53 UTC (permalink / raw)
  To: jaegeuk, yuchao0; +Cc: miaoxie, linux-f2fs-devel

This patch renamed CHECKSUM_OFFSET to CP_CHKSUM_OFFSET.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
---
 fsck/fsck.c        |  4 ++--
 fsck/mount.c       |  4 ++--
 fsck/resize.c      |  8 ++++----
 include/f2fs_fs.h  |  6 +++---
 mkfs/f2fs_format.c | 14 +++++++-------
 5 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index d550403..f080d3c 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -2010,8 +2010,8 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi)
 	set_cp(valid_node_count, fsck->chk.valid_node_cnt);
 	set_cp(valid_inode_count, fsck->chk.valid_inode_cnt);
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) = cpu_to_le32(crc);
+	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
+	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) = cpu_to_le32(crc);
 
 	cp_blk_no = get_sb(cp_blkaddr);
 	if (sbi->cur_cp == 2)
diff --git a/fsck/mount.c b/fsck/mount.c
index 2c2473d..1daef75 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -2252,8 +2252,8 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
 	flags = update_nat_bits_flags(sb, cp, flags);
 	set_cp(ckpt_flags, flags);
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) = cpu_to_le32(crc);
+	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
+	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) = cpu_to_le32(crc);
 
 	cp_blk_no = get_sb(cp_blkaddr);
 	if (sbi->cur_cp == 2)
diff --git a/fsck/resize.c b/fsck/resize.c
index fe8a61a..e9612b3 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -90,10 +90,10 @@ static int get_new_sb(struct f2fs_super_block *sb)
 		 * It requires more pages for cp.
 		 */
 		if (max_sit_bitmap_size > MAX_SIT_BITMAP_SIZE_IN_CKPT) {
-			max_nat_bitmap_size = CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1;
+			max_nat_bitmap_size = CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1;
 			set_sb(cp_payload, F2FS_BLK_ALIGN(max_sit_bitmap_size));
 		} else {
-			max_nat_bitmap_size = CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1
+			max_nat_bitmap_size = CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1
 				- max_sit_bitmap_size;
 			set_sb(cp_payload, 0);
 		}
@@ -520,8 +520,8 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
 						(unsigned char *)cp);
 	new_cp->checkpoint_ver = cpu_to_le64(cp_ver + 1);
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, new_cp, CHECKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)new_cp + CHECKSUM_OFFSET)) =
+	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, new_cp, CP_CHKSUM_OFFSET);
+	*((__le32 *)((unsigned char *)new_cp + CP_CHKSUM_OFFSET)) =
 							cpu_to_le32(crc);
 
 	/* Write a new checkpoint in the other set */
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 2c086a9..38a0da4 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -278,7 +278,7 @@ static inline uint64_t bswap_64(uint64_t val)
 #define PAGE_CACHE_SIZE		4096
 #define BITS_PER_BYTE		8
 #define F2FS_SUPER_MAGIC	0xF2F52010	/* F2FS Magic Number */
-#define CHECKSUM_OFFSET		4092
+#define CP_CHKSUM_OFFSET	4092
 #define MAX_PATH_LEN		64
 #define MAX_DEVICES		8
 
@@ -682,9 +682,9 @@ struct f2fs_checkpoint {
 } __attribute__((packed));
 
 #define MAX_SIT_BITMAP_SIZE_IN_CKPT    \
-	(CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1 - 64)
+	(CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1 - 64)
 #define MAX_BITMAP_SIZE_IN_CKPT	\
-	(CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1)
+	(CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1)
 
 /*
  * For orphan inode management
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 4b88d93..621126c 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -342,12 +342,12 @@ static int f2fs_prepare_super_block(void)
 		 * It requires more pages for cp.
 		 */
 		if (max_sit_bitmap_size > MAX_SIT_BITMAP_SIZE_IN_CKPT) {
-			max_nat_bitmap_size = CHECKSUM_OFFSET -
+			max_nat_bitmap_size = CP_CHKSUM_OFFSET -
 					sizeof(struct f2fs_checkpoint) + 1;
 			set_sb(cp_payload, F2FS_BLK_ALIGN(max_sit_bitmap_size));
 	        } else {
 			max_nat_bitmap_size =
-				CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1
+				CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1
 				- max_sit_bitmap_size;
 			set_sb(cp_payload, 0);
 		}
@@ -684,10 +684,10 @@ static int f2fs_write_check_point_pack(void)
 	set_cp(nat_ver_bitmap_bytesize, ((get_sb(segment_count_nat) / 2) <<
 			 get_sb(log_blocks_per_seg)) / 8);
 
-	set_cp(checksum_offset, CHECKSUM_OFFSET);
+	set_cp(checksum_offset, CP_CHKSUM_OFFSET);
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) =
+	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
+	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) =
 							cpu_to_le32(crc);
 
 	blk_size_bytes = 1 << get_sb(log_blocksize);
@@ -932,8 +932,8 @@ static int f2fs_write_check_point_pack(void)
 	 */
 	cp->checkpoint_ver = 0;
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) =
+	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
+	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) =
 							cpu_to_le32(crc);
 	cp_seg_blk = get_sb(segment0_blkaddr) + c.blks_per_seg;
 	DBG(1, "\tWriting cp page 1 of checkpoint pack 2, at offset 0x%08"PRIx64"\n",
-- 
2.19.0

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

* [PATCH 3/5] fsck.f2fs: unify the updating of superblocks
  2018-09-19 13:53 [PATCH 1/5] f2fs: support superblock checksum Junling Zheng
  2018-09-19 13:53 ` [PATCH 2/5] f2fs-tools: rename CHECKSUM_OFFSET to CP_CHKSUM_OFFSET Junling Zheng
@ 2018-09-19 13:53 ` Junling Zheng
  2018-09-19 23:33   ` Jaegeuk Kim
  2018-09-19 13:53 ` [PATCH 4/5] f2fs-tools: introduce sb checksum Junling Zheng
  2018-09-19 13:53 ` [PATCH 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack Junling Zheng
  3 siblings, 1 reply; 18+ messages in thread
From: Junling Zheng @ 2018-09-19 13:53 UTC (permalink / raw)
  To: jaegeuk, yuchao0; +Cc: miaoxie, linux-f2fs-devel

Rename write_superblock() to update_superblock() and make it support updating
specified one superblock or both two superblocks, then unify all places where
sb needs to be updated.

Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
---
 fsck/fsck.h   | 12 ++++++-
 fsck/mount.c  | 94 +++++++++++++++++++--------------------------------
 fsck/resize.c | 20 ++---------
 3 files changed, 47 insertions(+), 79 deletions(-)

diff --git a/fsck/fsck.h b/fsck/fsck.h
index 6042e68..bdd7f8d 100644
--- a/fsck/fsck.h
+++ b/fsck/fsck.h
@@ -32,6 +32,16 @@ enum {
 	EUNKNOWN_ARG,
 };
 
+enum SB_ADDR {
+	SB_ADDR_0,
+	SB_ADDR_1,
+	SB_ADDR_MAX,
+};
+
+#define SB_FIRST	(1 << SB_ADDR_0)
+#define SB_SECOND	(1 << SB_ADDR_1)
+#define SB_ALL		(SB_FIRST | SB_SECOND)
+
 /* fsck.c */
 struct orphan_info {
 	u32 nr_inodes;
@@ -178,7 +188,7 @@ extern void move_curseg_info(struct f2fs_sb_info *, u64, int);
 extern void write_curseg_info(struct f2fs_sb_info *);
 extern int find_next_free_block(struct f2fs_sb_info *, u64 *, int, int);
 extern void write_checkpoint(struct f2fs_sb_info *);
-extern void write_superblock(struct f2fs_super_block *);
+extern void update_superblock(struct f2fs_super_block *, int);
 extern void update_data_blkaddr(struct f2fs_sb_info *, nid_t, u16, block_t);
 extern void update_nat_blkaddr(struct f2fs_sb_info *, nid_t, nid_t, block_t);
 
diff --git a/fsck/mount.c b/fsck/mount.c
index 1daef75..74ff7c6 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -475,8 +475,28 @@ void print_sb_state(struct f2fs_super_block *sb)
 	MSG(0, "\n");
 }
 
+void update_superblock(struct f2fs_super_block *sb, int sb_mask)
+{
+	int index, ret;
+	u_int8_t *buf;
+
+	buf = calloc(BLOCK_SZ, 1);
+	ASSERT(buf);
+
+	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
+	for (index = 0; index < SB_ADDR_MAX; index++) {
+		if ((1 << index) & sb_mask) {
+			ret = dev_write_block(buf, index);
+			ASSERT(ret >= 0);
+		}
+	}
+
+	free(buf);
+	DBG(0, "Info: Done to update superblock\n");
+}
+
 static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
-							u64 offset)
+							enum SB_ADDR sb_addr)
 {
 	u32 segment0_blkaddr = get_sb(segment0_blkaddr);
 	u32 cp_blkaddr = get_sb(cp_blkaddr);
@@ -542,14 +562,11 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
 			segment_count_main << log_blocks_per_seg);
 		return -1;
 	} else if (main_end_blkaddr < seg_end_blkaddr) {
-		int err;
-
 		set_sb(segment_count, (main_end_blkaddr -
 				segment0_blkaddr) >> log_blocks_per_seg);
 
-		err = dev_write(sb, offset, sizeof(struct f2fs_super_block));
-		MSG(0, "Info: Fix alignment: %s, start(%u) end(%u) block(%u)\n",
-			err ? "failed": "done",
+		update_superblock(sb, 1 << sb_addr);
+		MSG(0, "Info: Fix alignment: start(%u) end(%u) block(%u)\n",
 			main_blkaddr,
 			segment0_blkaddr +
 				(segment_count << log_blocks_per_seg),
@@ -558,7 +575,7 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
 	return 0;
 }
 
-int sanity_check_raw_super(struct f2fs_super_block *sb, u64 offset)
+int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
 {
 	unsigned int blocksize;
 
@@ -600,30 +617,24 @@ int sanity_check_raw_super(struct f2fs_super_block *sb, u64 offset)
 	if (get_sb(segment_count) > F2FS_MAX_SEGMENT)
 		return -1;
 
-	if (sanity_check_area_boundary(sb, offset))
+	if (sanity_check_area_boundary(sb, sb_addr))
 		return -1;
 	return 0;
 }
 
-int validate_super_block(struct f2fs_sb_info *sbi, int block)
+int validate_super_block(struct f2fs_sb_info *sbi, enum SB_ADDR sb_addr)
 {
-	u64 offset;
 	char buf[F2FS_BLKSIZE];
 
 	sbi->raw_super = malloc(sizeof(struct f2fs_super_block));
 
-	if (block == 0)
-		offset = F2FS_SUPER_OFFSET;
-	else
-		offset = F2FS_BLKSIZE + F2FS_SUPER_OFFSET;
-
-	if (dev_read_block(buf, block))
+	if (dev_read_block(buf, sb_addr))
 		return -1;
 
 	memcpy(sbi->raw_super, buf + F2FS_SUPER_OFFSET,
 					sizeof(struct f2fs_super_block));
 
-	if (!sanity_check_raw_super(sbi->raw_super, offset)) {
+	if (!sanity_check_raw_super(sbi->raw_super, sb_addr)) {
 		/* get kernel version */
 		if (c.kd >= 0) {
 			dev_read_version(c.version, 0, VERSION_LEN);
@@ -642,13 +653,9 @@ int validate_super_block(struct f2fs_sb_info *sbi, int block)
 		MSG(0, "Info: FSCK version\n  from \"%s\"\n    to \"%s\"\n",
 					c.sb_version, c.version);
 		if (memcmp(c.sb_version, c.version, VERSION_LEN)) {
-			int ret;
-
 			memcpy(sbi->raw_super->version,
 						c.version, VERSION_LEN);
-			ret = dev_write(sbi->raw_super, offset,
-					sizeof(struct f2fs_super_block));
-			ASSERT(ret >= 0);
+			update_superblock(sbi->raw_super, 1 << sb_addr);
 
 			c.auto_fix = 0;
 			c.fix_on = 1;
@@ -659,7 +666,7 @@ int validate_super_block(struct f2fs_sb_info *sbi, int block)
 
 	free(sbi->raw_super);
 	sbi->raw_super = NULL;
-	MSG(0, "\tCan't find a valid F2FS superblock at 0x%x\n", block);
+	MSG(0, "\tCan't find a valid F2FS superblock at 0x%x\n", sb_addr);
 
 	return -EINVAL;
 }
@@ -2295,23 +2302,6 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
 	ASSERT(ret >= 0);
 }
 
-void write_superblock(struct f2fs_super_block *new_sb)
-{
-	int index, ret;
-	u_int8_t *buf;
-
-	buf = calloc(BLOCK_SZ, 1);
-	ASSERT(buf);
-
-	memcpy(buf + F2FS_SUPER_OFFSET, new_sb, sizeof(*new_sb));
-	for (index = 0; index < 2; index++) {
-		ret = dev_write_block(buf, index);
-		ASSERT(ret >= 0);
-	}
-	free(buf);
-	DBG(0, "Info: Done to rebuild superblock\n");
-}
-
 void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
 {
 	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
@@ -2450,9 +2440,7 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
 
 static int check_sector_size(struct f2fs_super_block *sb)
 {
-	int index;
 	u_int32_t log_sectorsize, log_sectors_per_block;
-	u_int8_t *zero_buff;
 
 	log_sectorsize = log_base_2(c.sector_size);
 	log_sectors_per_block = log_base_2(c.sectors_per_blk);
@@ -2461,24 +2449,10 @@ static int check_sector_size(struct f2fs_super_block *sb)
 			log_sectors_per_block == get_sb(log_sectors_per_block))
 		return 0;
 
-	zero_buff = calloc(F2FS_BLKSIZE, 1);
-	ASSERT(zero_buff);
-
 	set_sb(log_sectorsize, log_sectorsize);
 	set_sb(log_sectors_per_block, log_sectors_per_block);
 
-	memcpy(zero_buff + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
-	DBG(1, "\tWriting super block, at offset 0x%08x\n", 0);
-	for (index = 0; index < 2; index++) {
-		if (dev_write(zero_buff, index * F2FS_BLKSIZE, F2FS_BLKSIZE)) {
-			MSG(1, "\tError: Failed while writing supe_blk "
-				"on disk!!! index : %d\n", index);
-			free(zero_buff);
-			return -1;
-		}
-	}
-
-	free(zero_buff);
+	update_superblock(sb, SB_ALL);
 	return 0;
 }
 
@@ -2499,7 +2473,7 @@ static void tune_sb_features(struct f2fs_sb_info *sbi)
 	if (!sb_changed)
 		return;
 
-	write_superblock(sb);
+	update_superblock(sb, SB_ALL);
 }
 
 int f2fs_do_mount(struct f2fs_sb_info *sbi)
@@ -2509,9 +2483,9 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
 	int ret;
 
 	sbi->active_logs = NR_CURSEG_TYPE;
-	ret = validate_super_block(sbi, 0);
+	ret = validate_super_block(sbi, SB_ADDR_0);
 	if (ret) {
-		ret = validate_super_block(sbi, 1);
+		ret = validate_super_block(sbi, SB_ADDR_1);
 		if (ret)
 			return -1;
 	}
diff --git a/fsck/resize.c b/fsck/resize.c
index e9612b3..5161a1f 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -577,22 +577,6 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
 	DBG(0, "Info: Done to rebuild checkpoint blocks\n");
 }
 
-static void rebuild_superblock(struct f2fs_super_block *new_sb)
-{
-	int index, ret;
-	u_int8_t *buf;
-
-	buf = calloc(BLOCK_SZ, 1);
-
-	memcpy(buf + F2FS_SUPER_OFFSET, new_sb, sizeof(*new_sb));
-	for (index = 0; index < 2; index++) {
-		ret = dev_write_block(buf, index);
-		ASSERT(ret >= 0);
-	}
-	free(buf);
-	DBG(0, "Info: Done to rebuild superblock\n");
-}
-
 static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
 {
 	struct f2fs_super_block *sb = F2FS_RAW_SUPER(sbi);
@@ -644,7 +628,7 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
 	migrate_nat(sbi, new_sb);
 	migrate_sit(sbi, new_sb, offset_seg);
 	rebuild_checkpoint(sbi, new_sb, offset_seg);
-	write_superblock(new_sb);
+	update_superblock(new_sb, SB_ALL);
 	return 0;
 }
 
@@ -695,7 +679,7 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
 		return -ENOSPC;
 	}
 
-	rebuild_superblock(new_sb);
+	update_superblock(new_sb, SB_ALL);
 	rebuild_checkpoint(sbi, new_sb, 0);
 	/*if (!c.safe_resize) {
 		migrate_sit(sbi, new_sb, offset_seg);
-- 
2.19.0

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

* [PATCH 4/5] f2fs-tools: introduce sb checksum
  2018-09-19 13:53 [PATCH 1/5] f2fs: support superblock checksum Junling Zheng
  2018-09-19 13:53 ` [PATCH 2/5] f2fs-tools: rename CHECKSUM_OFFSET to CP_CHKSUM_OFFSET Junling Zheng
  2018-09-19 13:53 ` [PATCH 3/5] fsck.f2fs: unify the updating of superblocks Junling Zheng
@ 2018-09-19 13:53 ` Junling Zheng
  2018-09-19 23:35   ` Jaegeuk Kim
  2018-09-19 13:53 ` [PATCH 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack Junling Zheng
  3 siblings, 1 reply; 18+ messages in thread
From: Junling Zheng @ 2018-09-19 13:53 UTC (permalink / raw)
  To: jaegeuk, yuchao0; +Cc: miaoxie, linux-f2fs-devel

This patch introduced crc for superblock.

Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
---
 fsck/mount.c       | 33 +++++++++++++++++++++++++++++++++
 fsck/resize.c      | 12 ++++++------
 include/f2fs_fs.h  |  6 +++++-
 mkfs/f2fs_format.c |  8 ++++++++
 4 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/fsck/mount.c b/fsck/mount.c
index 74ff7c6..9019921 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
 	DISP_u32(sb, node_ino);
 	DISP_u32(sb, meta_ino);
 	DISP_u32(sb, cp_payload);
+	DISP_u32(sb, crc);
 	DISP("%-.256s", sb, version);
 	printf("\n");
 }
@@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
 	if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
 		MSG(0, "%s", " lost_found");
 	}
+	if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
+		MSG(0, "%s", " sb_checksum");
+	}
 	MSG(0, "\n");
 	MSG(0, "Info: superblock encrypt level = %d, salt = ",
 					sb->encryption_level);
@@ -479,10 +483,20 @@ void update_superblock(struct f2fs_super_block *sb, int sb_mask)
 {
 	int index, ret;
 	u_int8_t *buf;
+	u32 old_crc, new_crc;
 
 	buf = calloc(BLOCK_SZ, 1);
 	ASSERT(buf);
 
+	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
+		old_crc = get_sb(crc);
+		new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
+						SB_CHKSUM_OFFSET);
+		set_sb(crc, new_crc);
+		MSG(1, "Info: SB CRC is updated (0x%x -> 0x%x)\n",
+							old_crc, new_crc);
+	}
+
 	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
 	for (index = 0; index < SB_ADDR_MAX; index++) {
 		if ((1 << index) & sb_mask) {
@@ -575,10 +589,29 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
 	return 0;
 }
 
+static int verify_sb_chksum(struct f2fs_super_block *sb)
+{
+	if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
+		MSG(0, "\tInvalid SB CRC offset: %u\n",
+					get_sb(checksum_offset));
+		return -1;
+	}
+	if (f2fs_crc_valid(get_sb(crc), sb,
+			get_sb(checksum_offset))) {
+		MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
+		return -1;
+	}
+	return 0;
+}
+
 int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
 {
 	unsigned int blocksize;
 
+	if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
+					verify_sb_chksum(sb))
+		return -1;
+
 	if (F2FS_SUPER_MAGIC != get_sb(magic))
 		return -1;
 
diff --git a/fsck/resize.c b/fsck/resize.c
index 5161a1f..3462165 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
 		}
 	}
 
-	print_raw_sb_info(sb);
-	print_raw_sb_info(new_sb);
-
 	old_main_blkaddr = get_sb(main_blkaddr);
 	new_main_blkaddr = get_newsb(main_blkaddr);
 	offset = new_main_blkaddr - old_main_blkaddr;
@@ -629,6 +626,9 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
 	migrate_sit(sbi, new_sb, offset_seg);
 	rebuild_checkpoint(sbi, new_sb, offset_seg);
 	update_superblock(new_sb, SB_ALL);
+	print_raw_sb_info(sb);
+	print_raw_sb_info(new_sb);
+
 	return 0;
 }
 
@@ -658,9 +658,6 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
 		}
 	}
 
-	print_raw_sb_info(sb);
-	print_raw_sb_info(new_sb);
-
 	old_main_blkaddr = get_sb(main_blkaddr);
 	new_main_blkaddr = get_newsb(main_blkaddr);
 	offset = old_main_blkaddr - new_main_blkaddr;
@@ -690,6 +687,9 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
 	/* move whole data region */
 	//if (err)
 	//	migrate_main(sbi, offset);
+	print_raw_sb_info(sb);
+	print_raw_sb_info(new_sb);
+
 	return 0;
 }
 
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 38a0da4..f80632a 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -279,6 +279,7 @@ static inline uint64_t bswap_64(uint64_t val)
 #define BITS_PER_BYTE		8
 #define F2FS_SUPER_MAGIC	0xF2F52010	/* F2FS Magic Number */
 #define CP_CHKSUM_OFFSET	4092
+#define SB_CHKSUM_OFFSET	3068
 #define MAX_PATH_LEN		64
 #define MAX_DEVICES		8
 
@@ -579,6 +580,7 @@ enum {
 #define F2FS_FEATURE_INODE_CRTIME	0x0100
 #define F2FS_FEATURE_LOST_FOUND		0x0200
 #define F2FS_FEATURE_VERITY		0x0400	/* reserved */
+#define F2FS_FEATURE_SB_CHKSUM		0x0800
 
 #define MAX_VOLUME_NAME		512
 
@@ -632,7 +634,8 @@ struct f2fs_super_block {
 	struct f2fs_device devs[MAX_DEVICES];	/* device list */
 	__le32 qf_ino[F2FS_MAX_QUOTAS];	/* quota inode numbers */
 	__u8 hot_ext_count;		/* # of hot file extension */
-	__u8 reserved[314];		/* valid reserved region */
+	__u8 reserved[310];		/* valid reserved region */
+	__le32 crc;			/* checksum of superblock */
 } __attribute__((packed));
 
 /*
@@ -1338,6 +1341,7 @@ struct feature feature_table[] = {					\
 	{ "inode_crtime",		F2FS_FEATURE_INODE_CRTIME },	\
 	{ "lost_found",			F2FS_FEATURE_LOST_FOUND },	\
 	{ "verity",			F2FS_FEATURE_VERITY },	/* reserved */ \
+	{ "sb_checksum",		F2FS_FEATURE_SB_CHKSUM },	\
 	{ NULL,				0x0},				\
 };
 
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 621126c..9b0a0d1 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -504,6 +504,14 @@ static int f2fs_prepare_super_block(void)
 
 	sb->feature = c.feature;
 
+	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
+		set_sb(checksum_offset, SB_CHKSUM_OFFSET);
+		set_sb(crc, f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
+						SB_CHKSUM_OFFSET));
+		MSG(1, "Info: SB CRC is set: offset (%d), crc (0x%x)\n",
+					get_sb(checksum_offset), get_sb(crc));
+	}
+
 	return 0;
 }
 
-- 
2.19.0

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

* [PATCH 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack
  2018-09-19 13:53 [PATCH 1/5] f2fs: support superblock checksum Junling Zheng
                   ` (2 preceding siblings ...)
  2018-09-19 13:53 ` [PATCH 4/5] f2fs-tools: introduce sb checksum Junling Zheng
@ 2018-09-19 13:53 ` Junling Zheng
  2018-09-19 23:38   ` Jaegeuk Kim
  3 siblings, 1 reply; 18+ messages in thread
From: Junling Zheng @ 2018-09-19 13:53 UTC (permalink / raw)
  To: jaegeuk, yuchao0; +Cc: miaoxie, linux-f2fs-devel

From: Chao Yu <yuchao0@huawei.com>

If sb checksum is not enabled, and cp pack is valid due to no
crc inconsistence, let's try to recover cp_payload based on
cp_pack_start_sum in cp pack.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fsck/f2fs.h  |  5 +++++
 fsck/mount.c | 10 +++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/fsck/f2fs.h b/fsck/f2fs.h
index d216444..0d0d5e2 100644
--- a/fsck/f2fs.h
+++ b/fsck/f2fs.h
@@ -259,6 +259,11 @@ static inline unsigned long __bitmap_size(struct f2fs_sb_info *sbi, int flag)
 	return 0;
 }
 
+static inline block_t __cp_payload(struct f2fs_sb_info *sbi)
+{
+	return le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload);
+}
+
 static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
 {
 	struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
diff --git a/fsck/mount.c b/fsck/mount.c
index 9019921..0e8fa41 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -975,12 +975,16 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
 	}
 
 	cp_pack_start_sum = __start_sum_addr(sbi);
-	cp_payload = get_sb(cp_payload);
+	cp_payload = __cp_payload(sbi);
 	if (cp_pack_start_sum < cp_payload + 1 ||
 		cp_pack_start_sum > blocks_per_seg - 1 -
 			NR_CURSEG_TYPE) {
-		MSG(0, "\tWrong cp_pack_start_sum(%u)\n", cp_pack_start_sum);
-		return 1;
+		MSG(0, "\tWrong cp_pack_start_sum(%u) or cp_payload(%u)\n",
+			cp_pack_start_sum, cp_payload);
+		if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM))
+			return 1;
+		set_sb(cp_payload, cp_pack_start_sum - 1);
+		update_superblock(sb, SB_ALL);
 	}
 
 	return 0;
-- 
2.19.0

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

* Re: [PATCH 3/5] fsck.f2fs: unify the updating of superblocks
  2018-09-19 13:53 ` [PATCH 3/5] fsck.f2fs: unify the updating of superblocks Junling Zheng
@ 2018-09-19 23:33   ` Jaegeuk Kim
  0 siblings, 0 replies; 18+ messages in thread
From: Jaegeuk Kim @ 2018-09-19 23:33 UTC (permalink / raw)
  To: Junling Zheng; +Cc: miaoxie, linux-f2fs-devel

On 09/19, Junling Zheng wrote:
> Rename write_superblock() to update_superblock() and make it support updating
> specified one superblock or both two superblocks, then unify all places where
> sb needs to be updated.
> 
> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
> ---
>  fsck/fsck.h   | 12 ++++++-
>  fsck/mount.c  | 94 +++++++++++++++++++--------------------------------
>  fsck/resize.c | 20 ++---------
>  3 files changed, 47 insertions(+), 79 deletions(-)
> 
> diff --git a/fsck/fsck.h b/fsck/fsck.h
> index 6042e68..bdd7f8d 100644
> --- a/fsck/fsck.h
> +++ b/fsck/fsck.h
> @@ -32,6 +32,16 @@ enum {
>  	EUNKNOWN_ARG,
>  };
>  
> +enum SB_ADDR {
> +	SB_ADDR_0,
> +	SB_ADDR_1,
> +	SB_ADDR_MAX,

SB0_ADDR = 0,
SB1_ADDR,
SB_MAX_ADDR,

> +};
> +

#define SB_MASK(i)	(1 << i)
#define SB_MASK_ALL	(SB_MASK(SB0_ADDR) | SB_MASK(SB1_ADDR))

> +#define SB_FIRST	(1 << SB_ADDR_0)
> +#define SB_SECOND	(1 << SB_ADDR_1)
> +#define SB_ALL		(SB_FIRST | SB_SECOND)
> +
>  /* fsck.c */
>  struct orphan_info {
>  	u32 nr_inodes;
> @@ -178,7 +188,7 @@ extern void move_curseg_info(struct f2fs_sb_info *, u64, int);
>  extern void write_curseg_info(struct f2fs_sb_info *);
>  extern int find_next_free_block(struct f2fs_sb_info *, u64 *, int, int);
>  extern void write_checkpoint(struct f2fs_sb_info *);
> -extern void write_superblock(struct f2fs_super_block *);
> +extern void update_superblock(struct f2fs_super_block *, int);
>  extern void update_data_blkaddr(struct f2fs_sb_info *, nid_t, u16, block_t);
>  extern void update_nat_blkaddr(struct f2fs_sb_info *, nid_t, nid_t, block_t);
>  
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 1daef75..74ff7c6 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -475,8 +475,28 @@ void print_sb_state(struct f2fs_super_block *sb)
>  	MSG(0, "\n");
>  }
>  
> +void update_superblock(struct f2fs_super_block *sb, int sb_mask)
> +{
> +	int index, ret;
> +	u_int8_t *buf;
> +
> +	buf = calloc(BLOCK_SZ, 1);
> +	ASSERT(buf);
> +
> +	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
> +	for (index = 0; index < SB_ADDR_MAX; index++) {

	for (addr = SB0_ADDR; addr < SB_MAX_ADDR; addr++)

> +		if ((1 << index) & sb_mask) {

		if (SB_MASK(addr) & sb_mask) {

> +			ret = dev_write_block(buf, index);
> +			ASSERT(ret >= 0);
> +		}
> +	}
> +
> +	free(buf);
> +	DBG(0, "Info: Done to update superblock\n");
> +}
> +
>  static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
> -							u64 offset)
> +							enum SB_ADDR sb_addr)
>  {
>  	u32 segment0_blkaddr = get_sb(segment0_blkaddr);
>  	u32 cp_blkaddr = get_sb(cp_blkaddr);
> @@ -542,14 +562,11 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
>  			segment_count_main << log_blocks_per_seg);
>  		return -1;
>  	} else if (main_end_blkaddr < seg_end_blkaddr) {
> -		int err;
> -
>  		set_sb(segment_count, (main_end_blkaddr -
>  				segment0_blkaddr) >> log_blocks_per_seg);
>  
> -		err = dev_write(sb, offset, sizeof(struct f2fs_super_block));
> -		MSG(0, "Info: Fix alignment: %s, start(%u) end(%u) block(%u)\n",
> -			err ? "failed": "done",
> +		update_superblock(sb, 1 << sb_addr);

		update_superblock(sb, SB_MASK(sb_addr));

> +		MSG(0, "Info: Fix alignment: start(%u) end(%u) block(%u)\n",
>  			main_blkaddr,
>  			segment0_blkaddr +
>  				(segment_count << log_blocks_per_seg),
> @@ -558,7 +575,7 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
>  	return 0;
>  }
>  
> -int sanity_check_raw_super(struct f2fs_super_block *sb, u64 offset)
> +int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
>  {
>  	unsigned int blocksize;
>  
> @@ -600,30 +617,24 @@ int sanity_check_raw_super(struct f2fs_super_block *sb, u64 offset)
>  	if (get_sb(segment_count) > F2FS_MAX_SEGMENT)
>  		return -1;
>  
> -	if (sanity_check_area_boundary(sb, offset))
> +	if (sanity_check_area_boundary(sb, sb_addr))
>  		return -1;
>  	return 0;
>  }
>  
> -int validate_super_block(struct f2fs_sb_info *sbi, int block)
> +int validate_super_block(struct f2fs_sb_info *sbi, enum SB_ADDR sb_addr)
>  {
> -	u64 offset;
>  	char buf[F2FS_BLKSIZE];
>  
>  	sbi->raw_super = malloc(sizeof(struct f2fs_super_block));
>  
> -	if (block == 0)
> -		offset = F2FS_SUPER_OFFSET;
> -	else
> -		offset = F2FS_BLKSIZE + F2FS_SUPER_OFFSET;
> -
> -	if (dev_read_block(buf, block))
> +	if (dev_read_block(buf, sb_addr))
>  		return -1;
>  
>  	memcpy(sbi->raw_super, buf + F2FS_SUPER_OFFSET,
>  					sizeof(struct f2fs_super_block));
>  
> -	if (!sanity_check_raw_super(sbi->raw_super, offset)) {
> +	if (!sanity_check_raw_super(sbi->raw_super, sb_addr)) {
>  		/* get kernel version */
>  		if (c.kd >= 0) {
>  			dev_read_version(c.version, 0, VERSION_LEN);
> @@ -642,13 +653,9 @@ int validate_super_block(struct f2fs_sb_info *sbi, int block)
>  		MSG(0, "Info: FSCK version\n  from \"%s\"\n    to \"%s\"\n",
>  					c.sb_version, c.version);
>  		if (memcmp(c.sb_version, c.version, VERSION_LEN)) {
> -			int ret;
> -
>  			memcpy(sbi->raw_super->version,
>  						c.version, VERSION_LEN);
> -			ret = dev_write(sbi->raw_super, offset,
> -					sizeof(struct f2fs_super_block));
> -			ASSERT(ret >= 0);
> +			update_superblock(sbi->raw_super, 1 << sb_addr);

							SB_MASK(sb_addr)

>  
>  			c.auto_fix = 0;
>  			c.fix_on = 1;
> @@ -659,7 +666,7 @@ int validate_super_block(struct f2fs_sb_info *sbi, int block)
>  
>  	free(sbi->raw_super);
>  	sbi->raw_super = NULL;
> -	MSG(0, "\tCan't find a valid F2FS superblock at 0x%x\n", block);
> +	MSG(0, "\tCan't find a valid F2FS superblock at 0x%x\n", sb_addr);
>  
>  	return -EINVAL;
>  }
> @@ -2295,23 +2302,6 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
>  	ASSERT(ret >= 0);
>  }
>  
> -void write_superblock(struct f2fs_super_block *new_sb)
> -{
> -	int index, ret;
> -	u_int8_t *buf;
> -
> -	buf = calloc(BLOCK_SZ, 1);
> -	ASSERT(buf);
> -
> -	memcpy(buf + F2FS_SUPER_OFFSET, new_sb, sizeof(*new_sb));
> -	for (index = 0; index < 2; index++) {
> -		ret = dev_write_block(buf, index);
> -		ASSERT(ret >= 0);
> -	}
> -	free(buf);
> -	DBG(0, "Info: Done to rebuild superblock\n");
> -}
> -
>  void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>  {
>  	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
> @@ -2450,9 +2440,7 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>  
>  static int check_sector_size(struct f2fs_super_block *sb)
>  {
> -	int index;
>  	u_int32_t log_sectorsize, log_sectors_per_block;
> -	u_int8_t *zero_buff;
>  
>  	log_sectorsize = log_base_2(c.sector_size);
>  	log_sectors_per_block = log_base_2(c.sectors_per_blk);
> @@ -2461,24 +2449,10 @@ static int check_sector_size(struct f2fs_super_block *sb)
>  			log_sectors_per_block == get_sb(log_sectors_per_block))
>  		return 0;
>  
> -	zero_buff = calloc(F2FS_BLKSIZE, 1);
> -	ASSERT(zero_buff);
> -
>  	set_sb(log_sectorsize, log_sectorsize);
>  	set_sb(log_sectors_per_block, log_sectors_per_block);
>  
> -	memcpy(zero_buff + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
> -	DBG(1, "\tWriting super block, at offset 0x%08x\n", 0);
> -	for (index = 0; index < 2; index++) {
> -		if (dev_write(zero_buff, index * F2FS_BLKSIZE, F2FS_BLKSIZE)) {
> -			MSG(1, "\tError: Failed while writing supe_blk "
> -				"on disk!!! index : %d\n", index);
> -			free(zero_buff);
> -			return -1;
> -		}
> -	}
> -
> -	free(zero_buff);
> +	update_superblock(sb, SB_ALL);

				SB_MASK_ALL

>  	return 0;
>  }
>  
> @@ -2499,7 +2473,7 @@ static void tune_sb_features(struct f2fs_sb_info *sbi)
>  	if (!sb_changed)
>  		return;
>  
> -	write_superblock(sb);
> +	update_superblock(sb, SB_ALL);
>  }
>  
>  int f2fs_do_mount(struct f2fs_sb_info *sbi)
> @@ -2509,9 +2483,9 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
>  	int ret;
>  
>  	sbi->active_logs = NR_CURSEG_TYPE;
> -	ret = validate_super_block(sbi, 0);
> +	ret = validate_super_block(sbi, SB_ADDR_0);

					SB0_ADDR

and so on...

>  	if (ret) {
> -		ret = validate_super_block(sbi, 1);
> +		ret = validate_super_block(sbi, SB_ADDR_1);
>  		if (ret)
>  			return -1;
>  	}
> diff --git a/fsck/resize.c b/fsck/resize.c
> index e9612b3..5161a1f 100644
> --- a/fsck/resize.c
> +++ b/fsck/resize.c
> @@ -577,22 +577,6 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
>  	DBG(0, "Info: Done to rebuild checkpoint blocks\n");
>  }
>  
> -static void rebuild_superblock(struct f2fs_super_block *new_sb)
> -{
> -	int index, ret;
> -	u_int8_t *buf;
> -
> -	buf = calloc(BLOCK_SZ, 1);
> -
> -	memcpy(buf + F2FS_SUPER_OFFSET, new_sb, sizeof(*new_sb));
> -	for (index = 0; index < 2; index++) {
> -		ret = dev_write_block(buf, index);
> -		ASSERT(ret >= 0);
> -	}
> -	free(buf);
> -	DBG(0, "Info: Done to rebuild superblock\n");
> -}
> -
>  static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>  {
>  	struct f2fs_super_block *sb = F2FS_RAW_SUPER(sbi);
> @@ -644,7 +628,7 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>  	migrate_nat(sbi, new_sb);
>  	migrate_sit(sbi, new_sb, offset_seg);
>  	rebuild_checkpoint(sbi, new_sb, offset_seg);
> -	write_superblock(new_sb);
> +	update_superblock(new_sb, SB_ALL);
>  	return 0;
>  }
>  
> @@ -695,7 +679,7 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>  		return -ENOSPC;
>  	}
>  
> -	rebuild_superblock(new_sb);
> +	update_superblock(new_sb, SB_ALL);
>  	rebuild_checkpoint(sbi, new_sb, 0);
>  	/*if (!c.safe_resize) {
>  		migrate_sit(sbi, new_sb, offset_seg);
> -- 
> 2.19.0

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

* Re: [PATCH 4/5] f2fs-tools: introduce sb checksum
  2018-09-19 13:53 ` [PATCH 4/5] f2fs-tools: introduce sb checksum Junling Zheng
@ 2018-09-19 23:35   ` Jaegeuk Kim
  2018-09-20  2:17     ` Junling Zheng
  0 siblings, 1 reply; 18+ messages in thread
From: Jaegeuk Kim @ 2018-09-19 23:35 UTC (permalink / raw)
  To: Junling Zheng; +Cc: miaoxie, linux-f2fs-devel

On 09/19, Junling Zheng wrote:
> This patch introduced crc for superblock.
> 
> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
> ---
>  fsck/mount.c       | 33 +++++++++++++++++++++++++++++++++
>  fsck/resize.c      | 12 ++++++------
>  include/f2fs_fs.h  |  6 +++++-
>  mkfs/f2fs_format.c |  8 ++++++++
>  4 files changed, 52 insertions(+), 7 deletions(-)
> 
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 74ff7c6..9019921 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
>  	DISP_u32(sb, node_ino);
>  	DISP_u32(sb, meta_ino);
>  	DISP_u32(sb, cp_payload);
> +	DISP_u32(sb, crc);
>  	DISP("%-.256s", sb, version);
>  	printf("\n");
>  }
> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
>  	if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
>  		MSG(0, "%s", " lost_found");
>  	}
> +	if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
> +		MSG(0, "%s", " sb_checksum");
> +	}
>  	MSG(0, "\n");
>  	MSG(0, "Info: superblock encrypt level = %d, salt = ",
>  					sb->encryption_level);
> @@ -479,10 +483,20 @@ void update_superblock(struct f2fs_super_block *sb, int sb_mask)
>  {
>  	int index, ret;
>  	u_int8_t *buf;
> +	u32 old_crc, new_crc;
>  
>  	buf = calloc(BLOCK_SZ, 1);
>  	ASSERT(buf);
>  
> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
> +		old_crc = get_sb(crc);
> +		new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
> +						SB_CHKSUM_OFFSET);
> +		set_sb(crc, new_crc);
> +		MSG(1, "Info: SB CRC is updated (0x%x -> 0x%x)\n",
> +							old_crc, new_crc);
> +	}
> +
>  	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
>  	for (index = 0; index < SB_ADDR_MAX; index++) {
>  		if ((1 << index) & sb_mask) {
> @@ -575,10 +589,29 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
>  	return 0;
>  }
>  
> +static int verify_sb_chksum(struct f2fs_super_block *sb)
> +{
> +	if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
> +		MSG(0, "\tInvalid SB CRC offset: %u\n",
> +					get_sb(checksum_offset));
> +		return -1;
> +	}
> +	if (f2fs_crc_valid(get_sb(crc), sb,
> +			get_sb(checksum_offset))) {
> +		MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
> +		return -1;
> +	}
> +	return 0;
> +}
> +
>  int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
>  {
>  	unsigned int blocksize;
>  
> +	if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
> +					verify_sb_chksum(sb))
> +		return -1;
> +
>  	if (F2FS_SUPER_MAGIC != get_sb(magic))
>  		return -1;
>  
> diff --git a/fsck/resize.c b/fsck/resize.c
> index 5161a1f..3462165 100644
> --- a/fsck/resize.c
> +++ b/fsck/resize.c
> @@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>  		}
>  	}
>  
> -	print_raw_sb_info(sb);
> -	print_raw_sb_info(new_sb);

It'd be worth to keep this to show the previous states.

> -
>  	old_main_blkaddr = get_sb(main_blkaddr);
>  	new_main_blkaddr = get_newsb(main_blkaddr);
>  	offset = new_main_blkaddr - old_main_blkaddr;
> @@ -629,6 +626,9 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>  	migrate_sit(sbi, new_sb, offset_seg);
>  	rebuild_checkpoint(sbi, new_sb, offset_seg);
>  	update_superblock(new_sb, SB_ALL);
> +	print_raw_sb_info(sb);
> +	print_raw_sb_info(new_sb);
> +
>  	return 0;
>  }
>  
> @@ -658,9 +658,6 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>  		}
>  	}
>  
> -	print_raw_sb_info(sb);
> -	print_raw_sb_info(new_sb);

Ditto.

> -
>  	old_main_blkaddr = get_sb(main_blkaddr);
>  	new_main_blkaddr = get_newsb(main_blkaddr);
>  	offset = old_main_blkaddr - new_main_blkaddr;
> @@ -690,6 +687,9 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>  	/* move whole data region */
>  	//if (err)
>  	//	migrate_main(sbi, offset);
> +	print_raw_sb_info(sb);
> +	print_raw_sb_info(new_sb);
> +
>  	return 0;
>  }
>  
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index 38a0da4..f80632a 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -279,6 +279,7 @@ static inline uint64_t bswap_64(uint64_t val)
>  #define BITS_PER_BYTE		8
>  #define F2FS_SUPER_MAGIC	0xF2F52010	/* F2FS Magic Number */
>  #define CP_CHKSUM_OFFSET	4092
> +#define SB_CHKSUM_OFFSET	3068
>  #define MAX_PATH_LEN		64
>  #define MAX_DEVICES		8
>  
> @@ -579,6 +580,7 @@ enum {
>  #define F2FS_FEATURE_INODE_CRTIME	0x0100
>  #define F2FS_FEATURE_LOST_FOUND		0x0200
>  #define F2FS_FEATURE_VERITY		0x0400	/* reserved */
> +#define F2FS_FEATURE_SB_CHKSUM		0x0800
>  
>  #define MAX_VOLUME_NAME		512
>  
> @@ -632,7 +634,8 @@ struct f2fs_super_block {
>  	struct f2fs_device devs[MAX_DEVICES];	/* device list */
>  	__le32 qf_ino[F2FS_MAX_QUOTAS];	/* quota inode numbers */
>  	__u8 hot_ext_count;		/* # of hot file extension */
> -	__u8 reserved[314];		/* valid reserved region */
> +	__u8 reserved[310];		/* valid reserved region */
> +	__le32 crc;			/* checksum of superblock */
>  } __attribute__((packed));
>  
>  /*
> @@ -1338,6 +1341,7 @@ struct feature feature_table[] = {					\
>  	{ "inode_crtime",		F2FS_FEATURE_INODE_CRTIME },	\
>  	{ "lost_found",			F2FS_FEATURE_LOST_FOUND },	\
>  	{ "verity",			F2FS_FEATURE_VERITY },	/* reserved */ \
> +	{ "sb_checksum",		F2FS_FEATURE_SB_CHKSUM },	\
>  	{ NULL,				0x0},				\
>  };
>  
> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
> index 621126c..9b0a0d1 100644
> --- a/mkfs/f2fs_format.c
> +++ b/mkfs/f2fs_format.c
> @@ -504,6 +504,14 @@ static int f2fs_prepare_super_block(void)
>  
>  	sb->feature = c.feature;
>  
> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
> +		set_sb(checksum_offset, SB_CHKSUM_OFFSET);
> +		set_sb(crc, f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
> +						SB_CHKSUM_OFFSET));
> +		MSG(1, "Info: SB CRC is set: offset (%d), crc (0x%x)\n",
> +					get_sb(checksum_offset), get_sb(crc));
> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.19.0

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

* Re: [PATCH 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack
  2018-09-19 13:53 ` [PATCH 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack Junling Zheng
@ 2018-09-19 23:38   ` Jaegeuk Kim
  2018-09-20  2:38     ` Junling Zheng
  0 siblings, 1 reply; 18+ messages in thread
From: Jaegeuk Kim @ 2018-09-19 23:38 UTC (permalink / raw)
  To: Junling Zheng; +Cc: miaoxie, linux-f2fs-devel

On 09/19, Junling Zheng wrote:
> From: Chao Yu <yuchao0@huawei.com>
> 
> If sb checksum is not enabled, and cp pack is valid due to no
> crc inconsistence, let's try to recover cp_payload based on
> cp_pack_start_sum in cp pack.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fsck/f2fs.h  |  5 +++++
>  fsck/mount.c | 10 +++++++---
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/fsck/f2fs.h b/fsck/f2fs.h
> index d216444..0d0d5e2 100644
> --- a/fsck/f2fs.h
> +++ b/fsck/f2fs.h
> @@ -259,6 +259,11 @@ static inline unsigned long __bitmap_size(struct f2fs_sb_info *sbi, int flag)
>  	return 0;
>  }
>  
> +static inline block_t __cp_payload(struct f2fs_sb_info *sbi)
> +{
> +	return le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload);
> +}
> +
>  static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
>  {
>  	struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 9019921..0e8fa41 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -975,12 +975,16 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
>  	}
>  
>  	cp_pack_start_sum = __start_sum_addr(sbi);
> -	cp_payload = get_sb(cp_payload);
> +	cp_payload = __cp_payload(sbi);
>  	if (cp_pack_start_sum < cp_payload + 1 ||
>  		cp_pack_start_sum > blocks_per_seg - 1 -
>  			NR_CURSEG_TYPE) {
> -		MSG(0, "\tWrong cp_pack_start_sum(%u)\n", cp_pack_start_sum);
> -		return 1;
> +		MSG(0, "\tWrong cp_pack_start_sum(%u) or cp_payload(%u)\n",
> +			cp_pack_start_sum, cp_payload);
> +		if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM))
> +			return 1;

Is this trying to fix superblock? Why is it related to SB_CHKSUM?

> +		set_sb(cp_payload, cp_pack_start_sum - 1);
> +		update_superblock(sb, SB_ALL);
>  	}
>  
>  	return 0;
> -- 
> 2.19.0

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

* Re: [PATCH 4/5] f2fs-tools: introduce sb checksum
  2018-09-19 23:35   ` Jaegeuk Kim
@ 2018-09-20  2:17     ` Junling Zheng
  2018-09-20 21:38       ` Jaegeuk Kim
  0 siblings, 1 reply; 18+ messages in thread
From: Junling Zheng @ 2018-09-20  2:17 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: miaoxie, linux-f2fs-devel

Hi, Jaegeuk

On 2018/9/20 7:35, Jaegeuk Kim wrote:
> On 09/19, Junling Zheng wrote:
>> This patch introduced crc for superblock.
>>
>> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
>> ---
>>  fsck/mount.c       | 33 +++++++++++++++++++++++++++++++++
>>  fsck/resize.c      | 12 ++++++------
>>  include/f2fs_fs.h  |  6 +++++-
>>  mkfs/f2fs_format.c |  8 ++++++++
>>  4 files changed, 52 insertions(+), 7 deletions(-)
>>
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index 74ff7c6..9019921 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
>>  	DISP_u32(sb, node_ino);
>>  	DISP_u32(sb, meta_ino);
>>  	DISP_u32(sb, cp_payload);
>> +	DISP_u32(sb, crc);
>>  	DISP("%-.256s", sb, version);
>>  	printf("\n");
>>  }
>> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
>>  	if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
>>  		MSG(0, "%s", " lost_found");
>>  	}
>> +	if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
>> +		MSG(0, "%s", " sb_checksum");
>> +	}
>>  	MSG(0, "\n");
>>  	MSG(0, "Info: superblock encrypt level = %d, salt = ",
>>  					sb->encryption_level);
>> @@ -479,10 +483,20 @@ void update_superblock(struct f2fs_super_block *sb, int sb_mask)
>>  {
>>  	int index, ret;
>>  	u_int8_t *buf;
>> +	u32 old_crc, new_crc;
>>  
>>  	buf = calloc(BLOCK_SZ, 1);
>>  	ASSERT(buf);
>>  
>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
>> +		old_crc = get_sb(crc);
>> +		new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
>> +						SB_CHKSUM_OFFSET);
>> +		set_sb(crc, new_crc);
>> +		MSG(1, "Info: SB CRC is updated (0x%x -> 0x%x)\n",
>> +							old_crc, new_crc);
>> +	}
>> +
>>  	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
>>  	for (index = 0; index < SB_ADDR_MAX; index++) {
>>  		if ((1 << index) & sb_mask) {
>> @@ -575,10 +589,29 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
>>  	return 0;
>>  }
>>  
>> +static int verify_sb_chksum(struct f2fs_super_block *sb)
>> +{
>> +	if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
>> +		MSG(0, "\tInvalid SB CRC offset: %u\n",
>> +					get_sb(checksum_offset));
>> +		return -1;
>> +	}
>> +	if (f2fs_crc_valid(get_sb(crc), sb,
>> +			get_sb(checksum_offset))) {
>> +		MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
>> +		return -1;
>> +	}
>> +	return 0;
>> +}
>> +
>>  int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
>>  {
>>  	unsigned int blocksize;
>>  
>> +	if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
>> +					verify_sb_chksum(sb))
>> +		return -1;
>> +
>>  	if (F2FS_SUPER_MAGIC != get_sb(magic))
>>  		return -1;
>>  
>> diff --git a/fsck/resize.c b/fsck/resize.c
>> index 5161a1f..3462165 100644
>> --- a/fsck/resize.c
>> +++ b/fsck/resize.c
>> @@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>>  		}
>>  	}
>>  
>> -	print_raw_sb_info(sb);
>> -	print_raw_sb_info(new_sb);
> 
> It'd be worth to keep this to show the previous states.
> 

Here, I just want to move the printing of sb and new_sb to the place
behind update_superblock(), where the crc in sb will be updated.

>> -
>>  	old_main_blkaddr = get_sb(main_blkaddr);
>>  	new_main_blkaddr = get_newsb(main_blkaddr);
>>  	offset = new_main_blkaddr - old_main_blkaddr;
>> @@ -629,6 +626,9 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>>  	migrate_sit(sbi, new_sb, offset_seg);
>>  	rebuild_checkpoint(sbi, new_sb, offset_seg);
>>  	update_superblock(new_sb, SB_ALL);
>> +	print_raw_sb_info(sb);
>> +	print_raw_sb_info(new_sb);
>> +
>>  	return 0;
>>  }
>>  
>> @@ -658,9 +658,6 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>>  		}
>>  	}
>>  
>> -	print_raw_sb_info(sb);
>> -	print_raw_sb_info(new_sb);
> 
> Ditto.
> 
>> -
>>  	old_main_blkaddr = get_sb(main_blkaddr);
>>  	new_main_blkaddr = get_newsb(main_blkaddr);
>>  	offset = old_main_blkaddr - new_main_blkaddr;
>> @@ -690,6 +687,9 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>>  	/* move whole data region */
>>  	//if (err)
>>  	//	migrate_main(sbi, offset);
>> +	print_raw_sb_info(sb);
>> +	print_raw_sb_info(new_sb);
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>> index 38a0da4..f80632a 100644
>> --- a/include/f2fs_fs.h
>> +++ b/include/f2fs_fs.h
>> @@ -279,6 +279,7 @@ static inline uint64_t bswap_64(uint64_t val)
>>  #define BITS_PER_BYTE		8
>>  #define F2FS_SUPER_MAGIC	0xF2F52010	/* F2FS Magic Number */
>>  #define CP_CHKSUM_OFFSET	4092
>> +#define SB_CHKSUM_OFFSET	3068
>>  #define MAX_PATH_LEN		64
>>  #define MAX_DEVICES		8
>>  
>> @@ -579,6 +580,7 @@ enum {
>>  #define F2FS_FEATURE_INODE_CRTIME	0x0100
>>  #define F2FS_FEATURE_LOST_FOUND		0x0200
>>  #define F2FS_FEATURE_VERITY		0x0400	/* reserved */
>> +#define F2FS_FEATURE_SB_CHKSUM		0x0800
>>  
>>  #define MAX_VOLUME_NAME		512
>>  
>> @@ -632,7 +634,8 @@ struct f2fs_super_block {
>>  	struct f2fs_device devs[MAX_DEVICES];	/* device list */
>>  	__le32 qf_ino[F2FS_MAX_QUOTAS];	/* quota inode numbers */
>>  	__u8 hot_ext_count;		/* # of hot file extension */
>> -	__u8 reserved[314];		/* valid reserved region */
>> +	__u8 reserved[310];		/* valid reserved region */
>> +	__le32 crc;			/* checksum of superblock */
>>  } __attribute__((packed));
>>  
>>  /*
>> @@ -1338,6 +1341,7 @@ struct feature feature_table[] = {					\
>>  	{ "inode_crtime",		F2FS_FEATURE_INODE_CRTIME },	\
>>  	{ "lost_found",			F2FS_FEATURE_LOST_FOUND },	\
>>  	{ "verity",			F2FS_FEATURE_VERITY },	/* reserved */ \
>> +	{ "sb_checksum",		F2FS_FEATURE_SB_CHKSUM },	\
>>  	{ NULL,				0x0},				\
>>  };
>>  
>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
>> index 621126c..9b0a0d1 100644
>> --- a/mkfs/f2fs_format.c
>> +++ b/mkfs/f2fs_format.c
>> @@ -504,6 +504,14 @@ static int f2fs_prepare_super_block(void)
>>  
>>  	sb->feature = c.feature;
>>  
>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
>> +		set_sb(checksum_offset, SB_CHKSUM_OFFSET);
>> +		set_sb(crc, f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
>> +						SB_CHKSUM_OFFSET));
>> +		MSG(1, "Info: SB CRC is set: offset (%d), crc (0x%x)\n",
>> +					get_sb(checksum_offset), get_sb(crc));
>> +	}
>> +
>>  	return 0;
>>  }
>>  
>> -- 
>> 2.19.0
> 
> .
> 

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

* Re: [PATCH 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack
  2018-09-19 23:38   ` Jaegeuk Kim
@ 2018-09-20  2:38     ` Junling Zheng
  2018-09-20  6:45       ` Chao Yu
  0 siblings, 1 reply; 18+ messages in thread
From: Junling Zheng @ 2018-09-20  2:38 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: miaoxie, linux-f2fs-devel

On 2018/9/20 7:38, Jaegeuk Kim wrote:
> On 09/19, Junling Zheng wrote:
>> From: Chao Yu <yuchao0@huawei.com>
>>
>> If sb checksum is not enabled, and cp pack is valid due to no
>> crc inconsistence, let's try to recover cp_payload based on
>> cp_pack_start_sum in cp pack.
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fsck/f2fs.h  |  5 +++++
>>  fsck/mount.c | 10 +++++++---
>>  2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/fsck/f2fs.h b/fsck/f2fs.h
>> index d216444..0d0d5e2 100644
>> --- a/fsck/f2fs.h
>> +++ b/fsck/f2fs.h
>> @@ -259,6 +259,11 @@ static inline unsigned long __bitmap_size(struct f2fs_sb_info *sbi, int flag)
>>  	return 0;
>>  }
>>  
>> +static inline block_t __cp_payload(struct f2fs_sb_info *sbi)
>> +{
>> +	return le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload);
>> +}
>> +
>>  static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
>>  {
>>  	struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index 9019921..0e8fa41 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -975,12 +975,16 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>  	}
>>  
>>  	cp_pack_start_sum = __start_sum_addr(sbi);
>> -	cp_payload = get_sb(cp_payload);
>> +	cp_payload = __cp_payload(sbi);
>>  	if (cp_pack_start_sum < cp_payload + 1 ||
>>  		cp_pack_start_sum > blocks_per_seg - 1 -
>>  			NR_CURSEG_TYPE) {
>> -		MSG(0, "\tWrong cp_pack_start_sum(%u)\n", cp_pack_start_sum);
>> -		return 1;
>> +		MSG(0, "\tWrong cp_pack_start_sum(%u) or cp_payload(%u)\n",
>> +			cp_pack_start_sum, cp_payload);
>> +		if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM))
>> +			return 1;
> 
> Is this trying to fix superblock? Why is it related to SB_CHKSUM?
> 

If sb_checksum is enabled, we should not try to fix superblock.
In this situation, both sb and cp are verified with crc, so we
don't know which one, cp_payload or cp_pack_start_sum, is the
exact one.

By contraries, if sb_checksum is disabled, we could try to fix
superblock as below :)

>> +		set_sb(cp_payload, cp_pack_start_sum - 1);
>> +		update_superblock(sb, SB_ALL);
>>  	}
>>  >>  	return 0;
>> -- 
>> 2.19.0
> 
> .
> 

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

* Re: [PATCH 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack
  2018-09-20  2:38     ` Junling Zheng
@ 2018-09-20  6:45       ` Chao Yu
  0 siblings, 0 replies; 18+ messages in thread
From: Chao Yu @ 2018-09-20  6:45 UTC (permalink / raw)
  To: Junling Zheng, Jaegeuk Kim; +Cc: miaoxie, linux-f2fs-devel

On 2018/9/20 10:38, Junling Zheng wrote:
> On 2018/9/20 7:38, Jaegeuk Kim wrote:
>> On 09/19, Junling Zheng wrote:
>>> From: Chao Yu <yuchao0@huawei.com>
>>>
>>> If sb checksum is not enabled, and cp pack is valid due to no
>>> crc inconsistence, let's try to recover cp_payload based on
>>> cp_pack_start_sum in cp pack.
>>>
>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>> ---
>>>  fsck/f2fs.h  |  5 +++++
>>>  fsck/mount.c | 10 +++++++---
>>>  2 files changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fsck/f2fs.h b/fsck/f2fs.h
>>> index d216444..0d0d5e2 100644
>>> --- a/fsck/f2fs.h
>>> +++ b/fsck/f2fs.h
>>> @@ -259,6 +259,11 @@ static inline unsigned long __bitmap_size(struct f2fs_sb_info *sbi, int flag)
>>>  	return 0;
>>>  }
>>>  
>>> +static inline block_t __cp_payload(struct f2fs_sb_info *sbi)
>>> +{
>>> +	return le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload);
>>> +}
>>> +
>>>  static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
>>>  {
>>>  	struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
>>> diff --git a/fsck/mount.c b/fsck/mount.c
>>> index 9019921..0e8fa41 100644
>>> --- a/fsck/mount.c
>>> +++ b/fsck/mount.c
>>> @@ -975,12 +975,16 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>>  	}
>>>  
>>>  	cp_pack_start_sum = __start_sum_addr(sbi);
>>> -	cp_payload = get_sb(cp_payload);
>>> +	cp_payload = __cp_payload(sbi);
>>>  	if (cp_pack_start_sum < cp_payload + 1 ||
>>>  		cp_pack_start_sum > blocks_per_seg - 1 -
>>>  			NR_CURSEG_TYPE) {
>>> -		MSG(0, "\tWrong cp_pack_start_sum(%u)\n", cp_pack_start_sum);
>>> -		return 1;
>>> +		MSG(0, "\tWrong cp_pack_start_sum(%u) or cp_payload(%u)\n",
>>> +			cp_pack_start_sum, cp_payload);
>>> +		if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM))
>>> +			return 1;
>>
>> Is this trying to fix superblock? Why is it related to SB_CHKSUM?
>>
> 
> If sb_checksum is enabled, we should not try to fix superblock.
> In this situation, both sb and cp are verified with crc, so we
> don't know which one, cp_payload or cp_pack_start_sum, is the
> exact one.
> 
> By contraries, if sb_checksum is disabled, we could try to fix
> superblock as below :)

Thanks for helping explanation, Junling.

That's true, w/o sb checksum feature, sb data is run out-of-protection,
instead, checkpoint pack data is always protected by checksum, so I expect
cp pack data is more trusty, that's why I just do recovery based on cp
pack's data.

If sb checksum feature is on, and checksum in SB and CP are all correct, we
still do not know which data is more trusty, so I just leave such case for now.

Thanks,

> 
>>> +		set_sb(cp_payload, cp_pack_start_sum - 1);
>>> +		update_superblock(sb, SB_ALL);
>>>  	}
>>>  >>  	return 0;
>>> -- 
>>> 2.19.0
>>
>> .
>>
> 
> 
> 
> .
> 

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

* Re: [PATCH 4/5] f2fs-tools: introduce sb checksum
  2018-09-20  2:17     ` Junling Zheng
@ 2018-09-20 21:38       ` Jaegeuk Kim
  2018-09-21  3:09         ` Junling Zheng
  0 siblings, 1 reply; 18+ messages in thread
From: Jaegeuk Kim @ 2018-09-20 21:38 UTC (permalink / raw)
  To: Junling Zheng; +Cc: miaoxie, linux-f2fs-devel

On 09/20, Junling Zheng wrote:
> Hi, Jaegeuk
> 
> On 2018/9/20 7:35, Jaegeuk Kim wrote:
> > On 09/19, Junling Zheng wrote:
> >> This patch introduced crc for superblock.
> >>
> >> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
> >> ---
> >>  fsck/mount.c       | 33 +++++++++++++++++++++++++++++++++
> >>  fsck/resize.c      | 12 ++++++------
> >>  include/f2fs_fs.h  |  6 +++++-
> >>  mkfs/f2fs_format.c |  8 ++++++++
> >>  4 files changed, 52 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/fsck/mount.c b/fsck/mount.c
> >> index 74ff7c6..9019921 100644
> >> --- a/fsck/mount.c
> >> +++ b/fsck/mount.c
> >> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
> >>  	DISP_u32(sb, node_ino);
> >>  	DISP_u32(sb, meta_ino);
> >>  	DISP_u32(sb, cp_payload);
> >> +	DISP_u32(sb, crc);
> >>  	DISP("%-.256s", sb, version);
> >>  	printf("\n");
> >>  }
> >> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
> >>  	if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
> >>  		MSG(0, "%s", " lost_found");
> >>  	}
> >> +	if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
> >> +		MSG(0, "%s", " sb_checksum");
> >> +	}
> >>  	MSG(0, "\n");
> >>  	MSG(0, "Info: superblock encrypt level = %d, salt = ",
> >>  					sb->encryption_level);
> >> @@ -479,10 +483,20 @@ void update_superblock(struct f2fs_super_block *sb, int sb_mask)
> >>  {
> >>  	int index, ret;
> >>  	u_int8_t *buf;
> >> +	u32 old_crc, new_crc;
> >>  
> >>  	buf = calloc(BLOCK_SZ, 1);
> >>  	ASSERT(buf);
> >>  
> >> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
> >> +		old_crc = get_sb(crc);
> >> +		new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
> >> +						SB_CHKSUM_OFFSET);
> >> +		set_sb(crc, new_crc);
> >> +		MSG(1, "Info: SB CRC is updated (0x%x -> 0x%x)\n",
> >> +							old_crc, new_crc);
> >> +	}
> >> +
> >>  	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
> >>  	for (index = 0; index < SB_ADDR_MAX; index++) {
> >>  		if ((1 << index) & sb_mask) {
> >> @@ -575,10 +589,29 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
> >>  	return 0;
> >>  }
> >>  
> >> +static int verify_sb_chksum(struct f2fs_super_block *sb)
> >> +{
> >> +	if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
> >> +		MSG(0, "\tInvalid SB CRC offset: %u\n",
> >> +					get_sb(checksum_offset));
> >> +		return -1;
> >> +	}
> >> +	if (f2fs_crc_valid(get_sb(crc), sb,
> >> +			get_sb(checksum_offset))) {
> >> +		MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
> >> +		return -1;
> >> +	}
> >> +	return 0;
> >> +}
> >> +
> >>  int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
> >>  {
> >>  	unsigned int blocksize;
> >>  
> >> +	if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
> >> +					verify_sb_chksum(sb))
> >> +		return -1;
> >> +
> >>  	if (F2FS_SUPER_MAGIC != get_sb(magic))
> >>  		return -1;
> >>  
> >> diff --git a/fsck/resize.c b/fsck/resize.c
> >> index 5161a1f..3462165 100644
> >> --- a/fsck/resize.c
> >> +++ b/fsck/resize.c
> >> @@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
> >>  		}
> >>  	}
> >>  
> >> -	print_raw_sb_info(sb);
> >> -	print_raw_sb_info(new_sb);
> > 
> > It'd be worth to keep this to show the previous states.
> > 
> 
> Here, I just want to move the printing of sb and new_sb to the place
> behind update_superblock(), where the crc in sb will be updated.

We'd better to see the changes, no?

> 
> >> -
> >>  	old_main_blkaddr = get_sb(main_blkaddr);
> >>  	new_main_blkaddr = get_newsb(main_blkaddr);
> >>  	offset = new_main_blkaddr - old_main_blkaddr;
> >> @@ -629,6 +626,9 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
> >>  	migrate_sit(sbi, new_sb, offset_seg);
> >>  	rebuild_checkpoint(sbi, new_sb, offset_seg);
> >>  	update_superblock(new_sb, SB_ALL);
> >> +	print_raw_sb_info(sb);
> >> +	print_raw_sb_info(new_sb);
> >> +
> >>  	return 0;
> >>  }
> >>  
> >> @@ -658,9 +658,6 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
> >>  		}
> >>  	}
> >>  
> >> -	print_raw_sb_info(sb);
> >> -	print_raw_sb_info(new_sb);
> > 
> > Ditto.
> > 
> >> -
> >>  	old_main_blkaddr = get_sb(main_blkaddr);
> >>  	new_main_blkaddr = get_newsb(main_blkaddr);
> >>  	offset = old_main_blkaddr - new_main_blkaddr;
> >> @@ -690,6 +687,9 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
> >>  	/* move whole data region */
> >>  	//if (err)
> >>  	//	migrate_main(sbi, offset);
> >> +	print_raw_sb_info(sb);
> >> +	print_raw_sb_info(new_sb);
> >> +
> >>  	return 0;
> >>  }
> >>  
> >> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> >> index 38a0da4..f80632a 100644
> >> --- a/include/f2fs_fs.h
> >> +++ b/include/f2fs_fs.h
> >> @@ -279,6 +279,7 @@ static inline uint64_t bswap_64(uint64_t val)
> >>  #define BITS_PER_BYTE		8
> >>  #define F2FS_SUPER_MAGIC	0xF2F52010	/* F2FS Magic Number */
> >>  #define CP_CHKSUM_OFFSET	4092
> >> +#define SB_CHKSUM_OFFSET	3068
> >>  #define MAX_PATH_LEN		64
> >>  #define MAX_DEVICES		8
> >>  
> >> @@ -579,6 +580,7 @@ enum {
> >>  #define F2FS_FEATURE_INODE_CRTIME	0x0100
> >>  #define F2FS_FEATURE_LOST_FOUND		0x0200
> >>  #define F2FS_FEATURE_VERITY		0x0400	/* reserved */
> >> +#define F2FS_FEATURE_SB_CHKSUM		0x0800
> >>  
> >>  #define MAX_VOLUME_NAME		512
> >>  
> >> @@ -632,7 +634,8 @@ struct f2fs_super_block {
> >>  	struct f2fs_device devs[MAX_DEVICES];	/* device list */
> >>  	__le32 qf_ino[F2FS_MAX_QUOTAS];	/* quota inode numbers */
> >>  	__u8 hot_ext_count;		/* # of hot file extension */
> >> -	__u8 reserved[314];		/* valid reserved region */
> >> +	__u8 reserved[310];		/* valid reserved region */
> >> +	__le32 crc;			/* checksum of superblock */
> >>  } __attribute__((packed));
> >>  
> >>  /*
> >> @@ -1338,6 +1341,7 @@ struct feature feature_table[] = {					\
> >>  	{ "inode_crtime",		F2FS_FEATURE_INODE_CRTIME },	\
> >>  	{ "lost_found",			F2FS_FEATURE_LOST_FOUND },	\
> >>  	{ "verity",			F2FS_FEATURE_VERITY },	/* reserved */ \
> >> +	{ "sb_checksum",		F2FS_FEATURE_SB_CHKSUM },	\
> >>  	{ NULL,				0x0},				\
> >>  };
> >>  
> >> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
> >> index 621126c..9b0a0d1 100644
> >> --- a/mkfs/f2fs_format.c
> >> +++ b/mkfs/f2fs_format.c
> >> @@ -504,6 +504,14 @@ static int f2fs_prepare_super_block(void)
> >>  
> >>  	sb->feature = c.feature;
> >>  
> >> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
> >> +		set_sb(checksum_offset, SB_CHKSUM_OFFSET);
> >> +		set_sb(crc, f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
> >> +						SB_CHKSUM_OFFSET));
> >> +		MSG(1, "Info: SB CRC is set: offset (%d), crc (0x%x)\n",
> >> +					get_sb(checksum_offset), get_sb(crc));
> >> +	}
> >> +
> >>  	return 0;
> >>  }
> >>  
> >> -- 
> >> 2.19.0
> > 
> > .
> > 
> 

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

* Re: [PATCH 4/5] f2fs-tools: introduce sb checksum
  2018-09-20 21:38       ` Jaegeuk Kim
@ 2018-09-21  3:09         ` Junling Zheng
  2018-09-26  1:57           ` Jaegeuk Kim
  0 siblings, 1 reply; 18+ messages in thread
From: Junling Zheng @ 2018-09-21  3:09 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: miaoxie, linux-f2fs-devel

On 2018/9/21 5:38, Jaegeuk Kim wrote:
> On 09/20, Junling Zheng wrote:
>> Hi, Jaegeuk
>>
>> On 2018/9/20 7:35, Jaegeuk Kim wrote:
>>> On 09/19, Junling Zheng wrote:
>>>> This patch introduced crc for superblock.
>>>>
>>>> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
>>>> ---
>>>>  fsck/mount.c       | 33 +++++++++++++++++++++++++++++++++
>>>>  fsck/resize.c      | 12 ++++++------
>>>>  include/f2fs_fs.h  |  6 +++++-
>>>>  mkfs/f2fs_format.c |  8 ++++++++
>>>>  4 files changed, 52 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/fsck/mount.c b/fsck/mount.c
>>>> index 74ff7c6..9019921 100644
>>>> --- a/fsck/mount.c
>>>> +++ b/fsck/mount.c
>>>> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
>>>>  	DISP_u32(sb, node_ino);
>>>>  	DISP_u32(sb, meta_ino);
>>>>  	DISP_u32(sb, cp_payload);
>>>> +	DISP_u32(sb, crc);
>>>>  	DISP("%-.256s", sb, version);
>>>>  	printf("\n");
>>>>  }
>>>> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
>>>>  	if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
>>>>  		MSG(0, "%s", " lost_found");
>>>>  	}
>>>> +	if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
>>>> +		MSG(0, "%s", " sb_checksum");
>>>> +	}
>>>>  	MSG(0, "\n");
>>>>  	MSG(0, "Info: superblock encrypt level = %d, salt = ",
>>>>  					sb->encryption_level);
>>>> @@ -479,10 +483,20 @@ void update_superblock(struct f2fs_super_block *sb, int sb_mask)
>>>>  {
>>>>  	int index, ret;
>>>>  	u_int8_t *buf;
>>>> +	u32 old_crc, new_crc;
>>>>  
>>>>  	buf = calloc(BLOCK_SZ, 1);
>>>>  	ASSERT(buf);
>>>>  
>>>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
>>>> +		old_crc = get_sb(crc);
>>>> +		new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
>>>> +						SB_CHKSUM_OFFSET);
>>>> +		set_sb(crc, new_crc);
>>>> +		MSG(1, "Info: SB CRC is updated (0x%x -> 0x%x)\n",
>>>> +							old_crc, new_crc);
>>>> +	}
>>>> +
>>>>  	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
>>>>  	for (index = 0; index < SB_ADDR_MAX; index++) {
>>>>  		if ((1 << index) & sb_mask) {
>>>> @@ -575,10 +589,29 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
>>>>  	return 0;
>>>>  }
>>>>  
>>>> +static int verify_sb_chksum(struct f2fs_super_block *sb)
>>>> +{
>>>> +	if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
>>>> +		MSG(0, "\tInvalid SB CRC offset: %u\n",
>>>> +					get_sb(checksum_offset));
>>>> +		return -1;
>>>> +	}
>>>> +	if (f2fs_crc_valid(get_sb(crc), sb,
>>>> +			get_sb(checksum_offset))) {
>>>> +		MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
>>>> +		return -1;
>>>> +	}
>>>> +	return 0;
>>>> +}
>>>> +
>>>>  int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
>>>>  {
>>>>  	unsigned int blocksize;
>>>>  
>>>> +	if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
>>>> +					verify_sb_chksum(sb))
>>>> +		return -1;
>>>> +
>>>>  	if (F2FS_SUPER_MAGIC != get_sb(magic))
>>>>  		return -1;
>>>>  
>>>> diff --git a/fsck/resize.c b/fsck/resize.c
>>>> index 5161a1f..3462165 100644
>>>> --- a/fsck/resize.c
>>>> +++ b/fsck/resize.c
>>>> @@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>>>>  		}
>>>>  	}
>>>>  
>>>> -	print_raw_sb_info(sb);
>>>> -	print_raw_sb_info(new_sb);
>>>
>>> It'd be worth to keep this to show the previous states.
>>>
>>
>> Here, I just want to move the printing of sb and new_sb to the place
>> behind update_superblock(), where the crc in sb will be updated.
> 
> We'd better to see the changes, no?
> 

Yeah, we print sb and new_sb here just for seeing the changes of superblock.
However, the crc in new_sb will not be updated until calling update_superblock(),
so if we keep the current printing location, we can't get the changes.

Thanks,

>>
>>>> -
>>>>  	old_main_blkaddr = get_sb(main_blkaddr);
>>>>  	new_main_blkaddr = get_newsb(main_blkaddr);
>>>>  	offset = new_main_blkaddr - old_main_blkaddr;
>>>> @@ -629,6 +626,9 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>>>>  	migrate_sit(sbi, new_sb, offset_seg);
>>>>  	rebuild_checkpoint(sbi, new_sb, offset_seg);
>>>>  	update_superblock(new_sb, SB_ALL);
>>>> +	print_raw_sb_info(sb);
>>>> +	print_raw_sb_info(new_sb);
>>>> +
>>>>  	return 0;
>>>>  }
>>>>  
>>>> @@ -658,9 +658,6 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>>>>  		}
>>>>  	}
>>>>  
>>>> -	print_raw_sb_info(sb);
>>>> -	print_raw_sb_info(new_sb);
>>>
>>> Ditto.
>>>
>>>> -
>>>>  	old_main_blkaddr = get_sb(main_blkaddr);
>>>>  	new_main_blkaddr = get_newsb(main_blkaddr);
>>>>  	offset = old_main_blkaddr - new_main_blkaddr;
>>>> @@ -690,6 +687,9 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>>>>  	/* move whole data region */
>>>>  	//if (err)
>>>>  	//	migrate_main(sbi, offset);
>>>> +	print_raw_sb_info(sb);
>>>> +	print_raw_sb_info(new_sb);
>>>> +
>>>>  	return 0;
>>>>  }
>>>>  
>>>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>>>> index 38a0da4..f80632a 100644
>>>> --- a/include/f2fs_fs.h
>>>> +++ b/include/f2fs_fs.h
>>>> @@ -279,6 +279,7 @@ static inline uint64_t bswap_64(uint64_t val)
>>>>  #define BITS_PER_BYTE		8
>>>>  #define F2FS_SUPER_MAGIC	0xF2F52010	/* F2FS Magic Number */
>>>>  #define CP_CHKSUM_OFFSET	4092
>>>> +#define SB_CHKSUM_OFFSET	3068
>>>>  #define MAX_PATH_LEN		64
>>>>  #define MAX_DEVICES		8
>>>>  
>>>> @@ -579,6 +580,7 @@ enum {
>>>>  #define F2FS_FEATURE_INODE_CRTIME	0x0100
>>>>  #define F2FS_FEATURE_LOST_FOUND		0x0200
>>>>  #define F2FS_FEATURE_VERITY		0x0400	/* reserved */
>>>> +#define F2FS_FEATURE_SB_CHKSUM		0x0800
>>>>  
>>>>  #define MAX_VOLUME_NAME		512
>>>>  
>>>> @@ -632,7 +634,8 @@ struct f2fs_super_block {
>>>>  	struct f2fs_device devs[MAX_DEVICES];	/* device list */
>>>>  	__le32 qf_ino[F2FS_MAX_QUOTAS];	/* quota inode numbers */
>>>>  	__u8 hot_ext_count;		/* # of hot file extension */
>>>> -	__u8 reserved[314];		/* valid reserved region */
>>>> +	__u8 reserved[310];		/* valid reserved region */
>>>> +	__le32 crc;			/* checksum of superblock */
>>>>  } __attribute__((packed));
>>>>  
>>>>  /*
>>>> @@ -1338,6 +1341,7 @@ struct feature feature_table[] = {					\
>>>>  	{ "inode_crtime",		F2FS_FEATURE_INODE_CRTIME },	\
>>>>  	{ "lost_found",			F2FS_FEATURE_LOST_FOUND },	\
>>>>  	{ "verity",			F2FS_FEATURE_VERITY },	/* reserved */ \
>>>> +	{ "sb_checksum",		F2FS_FEATURE_SB_CHKSUM },	\
>>>>  	{ NULL,				0x0},				\
>>>>  };
>>>>  
>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
>>>> index 621126c..9b0a0d1 100644
>>>> --- a/mkfs/f2fs_format.c
>>>> +++ b/mkfs/f2fs_format.c
>>>> @@ -504,6 +504,14 @@ static int f2fs_prepare_super_block(void)
>>>>  
>>>>  	sb->feature = c.feature;
>>>>  
>>>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
>>>> +		set_sb(checksum_offset, SB_CHKSUM_OFFSET);
>>>> +		set_sb(crc, f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
>>>> +						SB_CHKSUM_OFFSET));
>>>> +		MSG(1, "Info: SB CRC is set: offset (%d), crc (0x%x)\n",
>>>> +					get_sb(checksum_offset), get_sb(crc));
>>>> +	}
>>>> +
>>>>  	return 0;
>>>>  }
>>>>  
>>>> -- 
>>>> 2.19.0
>>>
>>> .
>>>
>>
> 
> .
> 

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

* Re: [PATCH 4/5] f2fs-tools: introduce sb checksum
  2018-09-21  3:09         ` Junling Zheng
@ 2018-09-26  1:57           ` Jaegeuk Kim
  2018-09-26  3:54             ` Junling Zheng
  0 siblings, 1 reply; 18+ messages in thread
From: Jaegeuk Kim @ 2018-09-26  1:57 UTC (permalink / raw)
  To: Junling Zheng; +Cc: miaoxie, linux-f2fs-devel

On 09/21, Junling Zheng wrote:
> On 2018/9/21 5:38, Jaegeuk Kim wrote:
> > On 09/20, Junling Zheng wrote:
> >> Hi, Jaegeuk
> >>
> >> On 2018/9/20 7:35, Jaegeuk Kim wrote:
> >>> On 09/19, Junling Zheng wrote:
> >>>> This patch introduced crc for superblock.
> >>>>
> >>>> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
> >>>> ---
> >>>>  fsck/mount.c       | 33 +++++++++++++++++++++++++++++++++
> >>>>  fsck/resize.c      | 12 ++++++------
> >>>>  include/f2fs_fs.h  |  6 +++++-
> >>>>  mkfs/f2fs_format.c |  8 ++++++++
> >>>>  4 files changed, 52 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/fsck/mount.c b/fsck/mount.c
> >>>> index 74ff7c6..9019921 100644
> >>>> --- a/fsck/mount.c
> >>>> +++ b/fsck/mount.c
> >>>> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
> >>>>  	DISP_u32(sb, node_ino);
> >>>>  	DISP_u32(sb, meta_ino);
> >>>>  	DISP_u32(sb, cp_payload);
> >>>> +	DISP_u32(sb, crc);
> >>>>  	DISP("%-.256s", sb, version);
> >>>>  	printf("\n");
> >>>>  }
> >>>> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
> >>>>  	if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
> >>>>  		MSG(0, "%s", " lost_found");
> >>>>  	}
> >>>> +	if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
> >>>> +		MSG(0, "%s", " sb_checksum");
> >>>> +	}
> >>>>  	MSG(0, "\n");
> >>>>  	MSG(0, "Info: superblock encrypt level = %d, salt = ",
> >>>>  					sb->encryption_level);
> >>>> @@ -479,10 +483,20 @@ void update_superblock(struct f2fs_super_block *sb, int sb_mask)
> >>>>  {
> >>>>  	int index, ret;
> >>>>  	u_int8_t *buf;
> >>>> +	u32 old_crc, new_crc;
> >>>>  
> >>>>  	buf = calloc(BLOCK_SZ, 1);
> >>>>  	ASSERT(buf);
> >>>>  
> >>>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
> >>>> +		old_crc = get_sb(crc);
> >>>> +		new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
> >>>> +						SB_CHKSUM_OFFSET);
> >>>> +		set_sb(crc, new_crc);
> >>>> +		MSG(1, "Info: SB CRC is updated (0x%x -> 0x%x)\n",
> >>>> +							old_crc, new_crc);
> >>>> +	}
> >>>> +
> >>>>  	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
> >>>>  	for (index = 0; index < SB_ADDR_MAX; index++) {
> >>>>  		if ((1 << index) & sb_mask) {
> >>>> @@ -575,10 +589,29 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
> >>>>  	return 0;
> >>>>  }
> >>>>  
> >>>> +static int verify_sb_chksum(struct f2fs_super_block *sb)
> >>>> +{
> >>>> +	if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
> >>>> +		MSG(0, "\tInvalid SB CRC offset: %u\n",
> >>>> +					get_sb(checksum_offset));
> >>>> +		return -1;
> >>>> +	}
> >>>> +	if (f2fs_crc_valid(get_sb(crc), sb,
> >>>> +			get_sb(checksum_offset))) {
> >>>> +		MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
> >>>> +		return -1;
> >>>> +	}
> >>>> +	return 0;
> >>>> +}
> >>>> +
> >>>>  int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
> >>>>  {
> >>>>  	unsigned int blocksize;
> >>>>  
> >>>> +	if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
> >>>> +					verify_sb_chksum(sb))
> >>>> +		return -1;
> >>>> +
> >>>>  	if (F2FS_SUPER_MAGIC != get_sb(magic))
> >>>>  		return -1;
> >>>>  
> >>>> diff --git a/fsck/resize.c b/fsck/resize.c
> >>>> index 5161a1f..3462165 100644
> >>>> --- a/fsck/resize.c
> >>>> +++ b/fsck/resize.c
> >>>> @@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
> >>>>  		}
> >>>>  	}
> >>>>  
> >>>> -	print_raw_sb_info(sb);
> >>>> -	print_raw_sb_info(new_sb);
> >>>
> >>> It'd be worth to keep this to show the previous states.
> >>>
> >>
> >> Here, I just want to move the printing of sb and new_sb to the place
> >> behind update_superblock(), where the crc in sb will be updated.
> > 
> > We'd better to see the changes, no?
> > 
> 
> Yeah, we print sb and new_sb here just for seeing the changes of superblock.
> However, the crc in new_sb will not be updated until calling update_superblock(),
> so if we keep the current printing location, we can't get the changes.

I mean...
		print_raw_sb_info(sb);
		print_raw_sb_info(new_sb);
		update...
		print_raw_sb_info(new_sb);

> 
> Thanks,
> 
> >>
> >>>> -
> >>>>  	old_main_blkaddr = get_sb(main_blkaddr);
> >>>>  	new_main_blkaddr = get_newsb(main_blkaddr);
> >>>>  	offset = new_main_blkaddr - old_main_blkaddr;
> >>>> @@ -629,6 +626,9 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
> >>>>  	migrate_sit(sbi, new_sb, offset_seg);
> >>>>  	rebuild_checkpoint(sbi, new_sb, offset_seg);
> >>>>  	update_superblock(new_sb, SB_ALL);
> >>>> +	print_raw_sb_info(sb);
> >>>> +	print_raw_sb_info(new_sb);
> >>>> +
> >>>>  	return 0;
> >>>>  }
> >>>>  
> >>>> @@ -658,9 +658,6 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
> >>>>  		}
> >>>>  	}
> >>>>  
> >>>> -	print_raw_sb_info(sb);
> >>>> -	print_raw_sb_info(new_sb);
> >>>
> >>> Ditto.
> >>>
> >>>> -
> >>>>  	old_main_blkaddr = get_sb(main_blkaddr);
> >>>>  	new_main_blkaddr = get_newsb(main_blkaddr);
> >>>>  	offset = old_main_blkaddr - new_main_blkaddr;
> >>>> @@ -690,6 +687,9 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
> >>>>  	/* move whole data region */
> >>>>  	//if (err)
> >>>>  	//	migrate_main(sbi, offset);
> >>>> +	print_raw_sb_info(sb);
> >>>> +	print_raw_sb_info(new_sb);
> >>>> +
> >>>>  	return 0;
> >>>>  }
> >>>>  
> >>>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> >>>> index 38a0da4..f80632a 100644
> >>>> --- a/include/f2fs_fs.h
> >>>> +++ b/include/f2fs_fs.h
> >>>> @@ -279,6 +279,7 @@ static inline uint64_t bswap_64(uint64_t val)
> >>>>  #define BITS_PER_BYTE		8
> >>>>  #define F2FS_SUPER_MAGIC	0xF2F52010	/* F2FS Magic Number */
> >>>>  #define CP_CHKSUM_OFFSET	4092
> >>>> +#define SB_CHKSUM_OFFSET	3068
> >>>>  #define MAX_PATH_LEN		64
> >>>>  #define MAX_DEVICES		8
> >>>>  
> >>>> @@ -579,6 +580,7 @@ enum {
> >>>>  #define F2FS_FEATURE_INODE_CRTIME	0x0100
> >>>>  #define F2FS_FEATURE_LOST_FOUND		0x0200
> >>>>  #define F2FS_FEATURE_VERITY		0x0400	/* reserved */
> >>>> +#define F2FS_FEATURE_SB_CHKSUM		0x0800
> >>>>  
> >>>>  #define MAX_VOLUME_NAME		512
> >>>>  
> >>>> @@ -632,7 +634,8 @@ struct f2fs_super_block {
> >>>>  	struct f2fs_device devs[MAX_DEVICES];	/* device list */
> >>>>  	__le32 qf_ino[F2FS_MAX_QUOTAS];	/* quota inode numbers */
> >>>>  	__u8 hot_ext_count;		/* # of hot file extension */
> >>>> -	__u8 reserved[314];		/* valid reserved region */
> >>>> +	__u8 reserved[310];		/* valid reserved region */
> >>>> +	__le32 crc;			/* checksum of superblock */
> >>>>  } __attribute__((packed));
> >>>>  
> >>>>  /*
> >>>> @@ -1338,6 +1341,7 @@ struct feature feature_table[] = {					\
> >>>>  	{ "inode_crtime",		F2FS_FEATURE_INODE_CRTIME },	\
> >>>>  	{ "lost_found",			F2FS_FEATURE_LOST_FOUND },	\
> >>>>  	{ "verity",			F2FS_FEATURE_VERITY },	/* reserved */ \
> >>>> +	{ "sb_checksum",		F2FS_FEATURE_SB_CHKSUM },	\
> >>>>  	{ NULL,				0x0},				\
> >>>>  };
> >>>>  
> >>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
> >>>> index 621126c..9b0a0d1 100644
> >>>> --- a/mkfs/f2fs_format.c
> >>>> +++ b/mkfs/f2fs_format.c
> >>>> @@ -504,6 +504,14 @@ static int f2fs_prepare_super_block(void)
> >>>>  
> >>>>  	sb->feature = c.feature;
> >>>>  
> >>>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
> >>>> +		set_sb(checksum_offset, SB_CHKSUM_OFFSET);
> >>>> +		set_sb(crc, f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
> >>>> +						SB_CHKSUM_OFFSET));
> >>>> +		MSG(1, "Info: SB CRC is set: offset (%d), crc (0x%x)\n",
> >>>> +					get_sb(checksum_offset), get_sb(crc));
> >>>> +	}
> >>>> +
> >>>>  	return 0;
> >>>>  }
> >>>>  
> >>>> -- 
> >>>> 2.19.0
> >>>
> >>> .
> >>>
> >>
> > 
> > .
> > 
> 

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

* Re: [PATCH 4/5] f2fs-tools: introduce sb checksum
  2018-09-26  1:57           ` Jaegeuk Kim
@ 2018-09-26  3:54             ` Junling Zheng
  2018-09-26 20:27               ` Jaegeuk Kim
  0 siblings, 1 reply; 18+ messages in thread
From: Junling Zheng @ 2018-09-26  3:54 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: miaoxie, linux-f2fs-devel

Hi, Jaegeuk

On 2018/9/26 9:57, Jaegeuk Kim wrote:
> On 09/21, Junling Zheng wrote:
>> On 2018/9/21 5:38, Jaegeuk Kim wrote:
>>> On 09/20, Junling Zheng wrote:
>>>> Hi, Jaegeuk
>>>>
>>>> On 2018/9/20 7:35, Jaegeuk Kim wrote:
>>>>> On 09/19, Junling Zheng wrote:
>>>>>> This patch introduced crc for superblock.
>>>>>>
>>>>>> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
>>>>>> ---
>>>>>>  fsck/mount.c       | 33 +++++++++++++++++++++++++++++++++
>>>>>>  fsck/resize.c      | 12 ++++++------
>>>>>>  include/f2fs_fs.h  |  6 +++++-
>>>>>>  mkfs/f2fs_format.c |  8 ++++++++
>>>>>>  4 files changed, 52 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/fsck/mount.c b/fsck/mount.c
>>>>>> index 74ff7c6..9019921 100644
>>>>>> --- a/fsck/mount.c
>>>>>> +++ b/fsck/mount.c
>>>>>> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
>>>>>>  	DISP_u32(sb, node_ino);
>>>>>>  	DISP_u32(sb, meta_ino);
>>>>>>  	DISP_u32(sb, cp_payload);
>>>>>> +	DISP_u32(sb, crc);
>>>>>>  	DISP("%-.256s", sb, version);
>>>>>>  	printf("\n");
>>>>>>  }
>>>>>> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
>>>>>>  	if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
>>>>>>  		MSG(0, "%s", " lost_found");
>>>>>>  	}
>>>>>> +	if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
>>>>>> +		MSG(0, "%s", " sb_checksum");
>>>>>> +	}
>>>>>>  	MSG(0, "\n");
>>>>>>  	MSG(0, "Info: superblock encrypt level = %d, salt = ",
>>>>>>  					sb->encryption_level);
>>>>>> @@ -479,10 +483,20 @@ void update_superblock(struct f2fs_super_block *sb, int sb_mask)
>>>>>>  {
>>>>>>  	int index, ret;
>>>>>>  	u_int8_t *buf;
>>>>>> +	u32 old_crc, new_crc;
>>>>>>  
>>>>>>  	buf = calloc(BLOCK_SZ, 1);
>>>>>>  	ASSERT(buf);
>>>>>>  
>>>>>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
>>>>>> +		old_crc = get_sb(crc);
>>>>>> +		new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
>>>>>> +						SB_CHKSUM_OFFSET);
>>>>>> +		set_sb(crc, new_crc);
>>>>>> +		MSG(1, "Info: SB CRC is updated (0x%x -> 0x%x)\n",
>>>>>> +							old_crc, new_crc);
>>>>>> +	}
>>>>>> +
>>>>>>  	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
>>>>>>  	for (index = 0; index < SB_ADDR_MAX; index++) {
>>>>>>  		if ((1 << index) & sb_mask) {
>>>>>> @@ -575,10 +589,29 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
>>>>>>  	return 0;
>>>>>>  }
>>>>>>  
>>>>>> +static int verify_sb_chksum(struct f2fs_super_block *sb)
>>>>>> +{
>>>>>> +	if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
>>>>>> +		MSG(0, "\tInvalid SB CRC offset: %u\n",
>>>>>> +					get_sb(checksum_offset));
>>>>>> +		return -1;
>>>>>> +	}
>>>>>> +	if (f2fs_crc_valid(get_sb(crc), sb,
>>>>>> +			get_sb(checksum_offset))) {
>>>>>> +		MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
>>>>>> +		return -1;
>>>>>> +	}
>>>>>> +	return 0;
>>>>>> +}
>>>>>> +
>>>>>>  int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
>>>>>>  {
>>>>>>  	unsigned int blocksize;
>>>>>>  
>>>>>> +	if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
>>>>>> +					verify_sb_chksum(sb))
>>>>>> +		return -1;
>>>>>> +
>>>>>>  	if (F2FS_SUPER_MAGIC != get_sb(magic))
>>>>>>  		return -1;
>>>>>>  
>>>>>> diff --git a/fsck/resize.c b/fsck/resize.c
>>>>>> index 5161a1f..3462165 100644
>>>>>> --- a/fsck/resize.c
>>>>>> +++ b/fsck/resize.c
>>>>>> @@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>>>>>>  		}
>>>>>>  	}
>>>>>>  
>>>>>> -	print_raw_sb_info(sb);
>>>>>> -	print_raw_sb_info(new_sb);
>>>>>
>>>>> It'd be worth to keep this to show the previous states.
>>>>>
>>>>
>>>> Here, I just want to move the printing of sb and new_sb to the place
>>>> behind update_superblock(), where the crc in sb will be updated.
>>>
>>> We'd better to see the changes, no?
>>>
>>
>> Yeah, we print sb and new_sb here just for seeing the changes of superblock.
>> However, the crc in new_sb will not be updated until calling update_superblock(),
>> so if we keep the current printing location, we can't get the changes.
> 
> I mean...
> 		print_raw_sb_info(sb);
> 		print_raw_sb_info(new_sb);

I don't think it's necessary to print new_sb here.
Before update_superblock() is called, the crc in new_sb hasn't been updated yet,
and is the same with that in sb.

Here, as we introduce sb checksum, all we have to do is delay printing new_sb
to show the latest crc.

Thanks,

> 		update...
> 		print_raw_sb_info(new_sb);
> 
>>
>> Thanks,
>>
>>>>
>>>>>> -
>>>>>>  	old_main_blkaddr = get_sb(main_blkaddr);
>>>>>>  	new_main_blkaddr = get_newsb(main_blkaddr);
>>>>>>  	offset = new_main_blkaddr - old_main_blkaddr;
>>>>>> @@ -629,6 +626,9 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>>>>>>  	migrate_sit(sbi, new_sb, offset_seg);
>>>>>>  	rebuild_checkpoint(sbi, new_sb, offset_seg);
>>>>>>  	update_superblock(new_sb, SB_ALL);
>>>>>> +	print_raw_sb_info(sb);
>>>>>> +	print_raw_sb_info(new_sb);
>>>>>> +
>>>>>>  	return 0;
>>>>>>  }
>>>>>>  
>>>>>> @@ -658,9 +658,6 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>>>>>>  		}
>>>>>>  	}
>>>>>>  
>>>>>> -	print_raw_sb_info(sb);
>>>>>> -	print_raw_sb_info(new_sb);
>>>>>
>>>>> Ditto.
>>>>>
>>>>>> -
>>>>>>  	old_main_blkaddr = get_sb(main_blkaddr);
>>>>>>  	new_main_blkaddr = get_newsb(main_blkaddr);
>>>>>>  	offset = old_main_blkaddr - new_main_blkaddr;
>>>>>> @@ -690,6 +687,9 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>>>>>>  	/* move whole data region */
>>>>>>  	//if (err)
>>>>>>  	//	migrate_main(sbi, offset);
>>>>>> +	print_raw_sb_info(sb);
>>>>>> +	print_raw_sb_info(new_sb);
>>>>>> +
>>>>>>  	return 0;
>>>>>>  }
>>>>>>  
>>>>>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>>>>>> index 38a0da4..f80632a 100644
>>>>>> --- a/include/f2fs_fs.h
>>>>>> +++ b/include/f2fs_fs.h
>>>>>> @@ -279,6 +279,7 @@ static inline uint64_t bswap_64(uint64_t val)
>>>>>>  #define BITS_PER_BYTE		8
>>>>>>  #define F2FS_SUPER_MAGIC	0xF2F52010	/* F2FS Magic Number */
>>>>>>  #define CP_CHKSUM_OFFSET	4092
>>>>>> +#define SB_CHKSUM_OFFSET	3068
>>>>>>  #define MAX_PATH_LEN		64
>>>>>>  #define MAX_DEVICES		8
>>>>>>  
>>>>>> @@ -579,6 +580,7 @@ enum {
>>>>>>  #define F2FS_FEATURE_INODE_CRTIME	0x0100
>>>>>>  #define F2FS_FEATURE_LOST_FOUND		0x0200
>>>>>>  #define F2FS_FEATURE_VERITY		0x0400	/* reserved */
>>>>>> +#define F2FS_FEATURE_SB_CHKSUM		0x0800
>>>>>>  
>>>>>>  #define MAX_VOLUME_NAME		512
>>>>>>  
>>>>>> @@ -632,7 +634,8 @@ struct f2fs_super_block {
>>>>>>  	struct f2fs_device devs[MAX_DEVICES];	/* device list */
>>>>>>  	__le32 qf_ino[F2FS_MAX_QUOTAS];	/* quota inode numbers */
>>>>>>  	__u8 hot_ext_count;		/* # of hot file extension */
>>>>>> -	__u8 reserved[314];		/* valid reserved region */
>>>>>> +	__u8 reserved[310];		/* valid reserved region */
>>>>>> +	__le32 crc;			/* checksum of superblock */
>>>>>>  } __attribute__((packed));
>>>>>>  
>>>>>>  /*
>>>>>> @@ -1338,6 +1341,7 @@ struct feature feature_table[] = {					\
>>>>>>  	{ "inode_crtime",		F2FS_FEATURE_INODE_CRTIME },	\
>>>>>>  	{ "lost_found",			F2FS_FEATURE_LOST_FOUND },	\
>>>>>>  	{ "verity",			F2FS_FEATURE_VERITY },	/* reserved */ \
>>>>>> +	{ "sb_checksum",		F2FS_FEATURE_SB_CHKSUM },	\
>>>>>>  	{ NULL,				0x0},				\
>>>>>>  };
>>>>>>  
>>>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
>>>>>> index 621126c..9b0a0d1 100644
>>>>>> --- a/mkfs/f2fs_format.c
>>>>>> +++ b/mkfs/f2fs_format.c
>>>>>> @@ -504,6 +504,14 @@ static int f2fs_prepare_super_block(void)
>>>>>>  
>>>>>>  	sb->feature = c.feature;
>>>>>>  
>>>>>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
>>>>>> +		set_sb(checksum_offset, SB_CHKSUM_OFFSET);
>>>>>> +		set_sb(crc, f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
>>>>>> +						SB_CHKSUM_OFFSET));
>>>>>> +		MSG(1, "Info: SB CRC is set: offset (%d), crc (0x%x)\n",
>>>>>> +					get_sb(checksum_offset), get_sb(crc));
>>>>>> +	}
>>>>>> +
>>>>>>  	return 0;
>>>>>>  }
>>>>>>  
>>>>>> -- 
>>>>>> 2.19.0
>>>>>
>>>>> .
>>>>>
>>>>
>>>
>>> .
>>>
>>
> 
> .
> 

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

* Re: [PATCH 4/5] f2fs-tools: introduce sb checksum
  2018-09-26  3:54             ` Junling Zheng
@ 2018-09-26 20:27               ` Jaegeuk Kim
  2018-09-27  2:46                 ` Junling Zheng
  0 siblings, 1 reply; 18+ messages in thread
From: Jaegeuk Kim @ 2018-09-26 20:27 UTC (permalink / raw)
  To: Junling Zheng; +Cc: miaoxie, linux-f2fs-devel

On 09/26, Junling Zheng wrote:
> Hi, Jaegeuk
> 
> On 2018/9/26 9:57, Jaegeuk Kim wrote:
> > On 09/21, Junling Zheng wrote:
> >> On 2018/9/21 5:38, Jaegeuk Kim wrote:
> >>> On 09/20, Junling Zheng wrote:
> >>>> Hi, Jaegeuk
> >>>>
> >>>> On 2018/9/20 7:35, Jaegeuk Kim wrote:
> >>>>> On 09/19, Junling Zheng wrote:
> >>>>>> This patch introduced crc for superblock.
> >>>>>>
> >>>>>> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
> >>>>>> ---
> >>>>>>  fsck/mount.c       | 33 +++++++++++++++++++++++++++++++++
> >>>>>>  fsck/resize.c      | 12 ++++++------
> >>>>>>  include/f2fs_fs.h  |  6 +++++-
> >>>>>>  mkfs/f2fs_format.c |  8 ++++++++
> >>>>>>  4 files changed, 52 insertions(+), 7 deletions(-)
> >>>>>>
> >>>>>> diff --git a/fsck/mount.c b/fsck/mount.c
> >>>>>> index 74ff7c6..9019921 100644
> >>>>>> --- a/fsck/mount.c
> >>>>>> +++ b/fsck/mount.c
> >>>>>> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
> >>>>>>  	DISP_u32(sb, node_ino);
> >>>>>>  	DISP_u32(sb, meta_ino);
> >>>>>>  	DISP_u32(sb, cp_payload);
> >>>>>> +	DISP_u32(sb, crc);
> >>>>>>  	DISP("%-.256s", sb, version);
> >>>>>>  	printf("\n");
> >>>>>>  }
> >>>>>> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
> >>>>>>  	if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
> >>>>>>  		MSG(0, "%s", " lost_found");
> >>>>>>  	}
> >>>>>> +	if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
> >>>>>> +		MSG(0, "%s", " sb_checksum");
> >>>>>> +	}
> >>>>>>  	MSG(0, "\n");
> >>>>>>  	MSG(0, "Info: superblock encrypt level = %d, salt = ",
> >>>>>>  					sb->encryption_level);
> >>>>>> @@ -479,10 +483,20 @@ void update_superblock(struct f2fs_super_block *sb, int sb_mask)
> >>>>>>  {
> >>>>>>  	int index, ret;
> >>>>>>  	u_int8_t *buf;
> >>>>>> +	u32 old_crc, new_crc;
> >>>>>>  
> >>>>>>  	buf = calloc(BLOCK_SZ, 1);
> >>>>>>  	ASSERT(buf);
> >>>>>>  
> >>>>>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
> >>>>>> +		old_crc = get_sb(crc);
> >>>>>> +		new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
> >>>>>> +						SB_CHKSUM_OFFSET);
> >>>>>> +		set_sb(crc, new_crc);
> >>>>>> +		MSG(1, "Info: SB CRC is updated (0x%x -> 0x%x)\n",
> >>>>>> +							old_crc, new_crc);
> >>>>>> +	}
> >>>>>> +
> >>>>>>  	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
> >>>>>>  	for (index = 0; index < SB_ADDR_MAX; index++) {
> >>>>>>  		if ((1 << index) & sb_mask) {
> >>>>>> @@ -575,10 +589,29 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
> >>>>>>  	return 0;
> >>>>>>  }
> >>>>>>  
> >>>>>> +static int verify_sb_chksum(struct f2fs_super_block *sb)
> >>>>>> +{
> >>>>>> +	if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
> >>>>>> +		MSG(0, "\tInvalid SB CRC offset: %u\n",
> >>>>>> +					get_sb(checksum_offset));
> >>>>>> +		return -1;
> >>>>>> +	}
> >>>>>> +	if (f2fs_crc_valid(get_sb(crc), sb,
> >>>>>> +			get_sb(checksum_offset))) {
> >>>>>> +		MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
> >>>>>> +		return -1;
> >>>>>> +	}
> >>>>>> +	return 0;
> >>>>>> +}
> >>>>>> +
> >>>>>>  int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
> >>>>>>  {
> >>>>>>  	unsigned int blocksize;
> >>>>>>  
> >>>>>> +	if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
> >>>>>> +					verify_sb_chksum(sb))
> >>>>>> +		return -1;
> >>>>>> +
> >>>>>>  	if (F2FS_SUPER_MAGIC != get_sb(magic))
> >>>>>>  		return -1;
> >>>>>>  
> >>>>>> diff --git a/fsck/resize.c b/fsck/resize.c
> >>>>>> index 5161a1f..3462165 100644
> >>>>>> --- a/fsck/resize.c
> >>>>>> +++ b/fsck/resize.c
> >>>>>> @@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
> >>>>>>  		}
> >>>>>>  	}
> >>>>>>  
> >>>>>> -	print_raw_sb_info(sb);
> >>>>>> -	print_raw_sb_info(new_sb);
> >>>>>
> >>>>> It'd be worth to keep this to show the previous states.
> >>>>>
> >>>>
> >>>> Here, I just want to move the printing of sb and new_sb to the place
> >>>> behind update_superblock(), where the crc in sb will be updated.
> >>>
> >>> We'd better to see the changes, no?
> >>>
> >>
> >> Yeah, we print sb and new_sb here just for seeing the changes of superblock.
> >> However, the crc in new_sb will not be updated until calling update_superblock(),
> >> so if we keep the current printing location, we can't get the changes.
> > 
> > I mean...
> > 		print_raw_sb_info(sb);
> > 		print_raw_sb_info(new_sb);
> 
> I don't think it's necessary to print new_sb here.
> Before update_superblock() is called, the crc in new_sb hasn't been updated yet,
> and is the same with that in sb.
> 
> Here, as we introduce sb checksum, all we have to do is delay printing new_sb
> to show the latest crc.

I see. The contents except crc in sb and new_sb weren't changed during
migration. If so, printing them later would make sense, right?

> 
> Thanks,
> 
> > 		update...
> > 		print_raw_sb_info(new_sb);
> > 
> >>
> >> Thanks,
> >>
> >>>>
> >>>>>> -
> >>>>>>  	old_main_blkaddr = get_sb(main_blkaddr);
> >>>>>>  	new_main_blkaddr = get_newsb(main_blkaddr);
> >>>>>>  	offset = new_main_blkaddr - old_main_blkaddr;
> >>>>>> @@ -629,6 +626,9 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
> >>>>>>  	migrate_sit(sbi, new_sb, offset_seg);
> >>>>>>  	rebuild_checkpoint(sbi, new_sb, offset_seg);
> >>>>>>  	update_superblock(new_sb, SB_ALL);
> >>>>>> +	print_raw_sb_info(sb);
> >>>>>> +	print_raw_sb_info(new_sb);
> >>>>>> +
> >>>>>>  	return 0;
> >>>>>>  }
> >>>>>>  
> >>>>>> @@ -658,9 +658,6 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
> >>>>>>  		}
> >>>>>>  	}
> >>>>>>  
> >>>>>> -	print_raw_sb_info(sb);
> >>>>>> -	print_raw_sb_info(new_sb);
> >>>>>
> >>>>> Ditto.
> >>>>>
> >>>>>> -
> >>>>>>  	old_main_blkaddr = get_sb(main_blkaddr);
> >>>>>>  	new_main_blkaddr = get_newsb(main_blkaddr);
> >>>>>>  	offset = old_main_blkaddr - new_main_blkaddr;
> >>>>>> @@ -690,6 +687,9 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
> >>>>>>  	/* move whole data region */
> >>>>>>  	//if (err)
> >>>>>>  	//	migrate_main(sbi, offset);
> >>>>>> +	print_raw_sb_info(sb);
> >>>>>> +	print_raw_sb_info(new_sb);
> >>>>>> +
> >>>>>>  	return 0;
> >>>>>>  }
> >>>>>>  
> >>>>>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> >>>>>> index 38a0da4..f80632a 100644
> >>>>>> --- a/include/f2fs_fs.h
> >>>>>> +++ b/include/f2fs_fs.h
> >>>>>> @@ -279,6 +279,7 @@ static inline uint64_t bswap_64(uint64_t val)
> >>>>>>  #define BITS_PER_BYTE		8
> >>>>>>  #define F2FS_SUPER_MAGIC	0xF2F52010	/* F2FS Magic Number */
> >>>>>>  #define CP_CHKSUM_OFFSET	4092
> >>>>>> +#define SB_CHKSUM_OFFSET	3068
> >>>>>>  #define MAX_PATH_LEN		64
> >>>>>>  #define MAX_DEVICES		8
> >>>>>>  
> >>>>>> @@ -579,6 +580,7 @@ enum {
> >>>>>>  #define F2FS_FEATURE_INODE_CRTIME	0x0100
> >>>>>>  #define F2FS_FEATURE_LOST_FOUND		0x0200
> >>>>>>  #define F2FS_FEATURE_VERITY		0x0400	/* reserved */
> >>>>>> +#define F2FS_FEATURE_SB_CHKSUM		0x0800
> >>>>>>  
> >>>>>>  #define MAX_VOLUME_NAME		512
> >>>>>>  
> >>>>>> @@ -632,7 +634,8 @@ struct f2fs_super_block {
> >>>>>>  	struct f2fs_device devs[MAX_DEVICES];	/* device list */
> >>>>>>  	__le32 qf_ino[F2FS_MAX_QUOTAS];	/* quota inode numbers */
> >>>>>>  	__u8 hot_ext_count;		/* # of hot file extension */
> >>>>>> -	__u8 reserved[314];		/* valid reserved region */
> >>>>>> +	__u8 reserved[310];		/* valid reserved region */
> >>>>>> +	__le32 crc;			/* checksum of superblock */
> >>>>>>  } __attribute__((packed));
> >>>>>>  
> >>>>>>  /*
> >>>>>> @@ -1338,6 +1341,7 @@ struct feature feature_table[] = {					\
> >>>>>>  	{ "inode_crtime",		F2FS_FEATURE_INODE_CRTIME },	\
> >>>>>>  	{ "lost_found",			F2FS_FEATURE_LOST_FOUND },	\
> >>>>>>  	{ "verity",			F2FS_FEATURE_VERITY },	/* reserved */ \
> >>>>>> +	{ "sb_checksum",		F2FS_FEATURE_SB_CHKSUM },	\
> >>>>>>  	{ NULL,				0x0},				\
> >>>>>>  };
> >>>>>>  
> >>>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
> >>>>>> index 621126c..9b0a0d1 100644
> >>>>>> --- a/mkfs/f2fs_format.c
> >>>>>> +++ b/mkfs/f2fs_format.c
> >>>>>> @@ -504,6 +504,14 @@ static int f2fs_prepare_super_block(void)
> >>>>>>  
> >>>>>>  	sb->feature = c.feature;
> >>>>>>  
> >>>>>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
> >>>>>> +		set_sb(checksum_offset, SB_CHKSUM_OFFSET);
> >>>>>> +		set_sb(crc, f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
> >>>>>> +						SB_CHKSUM_OFFSET));
> >>>>>> +		MSG(1, "Info: SB CRC is set: offset (%d), crc (0x%x)\n",
> >>>>>> +					get_sb(checksum_offset), get_sb(crc));
> >>>>>> +	}
> >>>>>> +
> >>>>>>  	return 0;
> >>>>>>  }
> >>>>>>  
> >>>>>> -- 
> >>>>>> 2.19.0
> >>>>>
> >>>>> .
> >>>>>
> >>>>
> >>>
> >>> .
> >>>
> >>
> > 
> > .
> > 
> 

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

* Re: [PATCH 4/5] f2fs-tools: introduce sb checksum
  2018-09-26 20:27               ` Jaegeuk Kim
@ 2018-09-27  2:46                 ` Junling Zheng
  0 siblings, 0 replies; 18+ messages in thread
From: Junling Zheng @ 2018-09-27  2:46 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: miaoxie, linux-f2fs-devel

On 2018/9/27 4:27, Jaegeuk Kim wrote:
> On 09/26, Junling Zheng wrote:
>> Hi, Jaegeuk
>>
>> On 2018/9/26 9:57, Jaegeuk Kim wrote:
>>> On 09/21, Junling Zheng wrote:
>>>> On 2018/9/21 5:38, Jaegeuk Kim wrote:
>>>>> On 09/20, Junling Zheng wrote:
>>>>>> Hi, Jaegeuk
>>>>>>
>>>>>> On 2018/9/20 7:35, Jaegeuk Kim wrote:
>>>>>>> On 09/19, Junling Zheng wrote:
>>>>>>>> This patch introduced crc for superblock.
>>>>>>>>
>>>>>>>> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
>>>>>>>> ---
>>>>>>>>  fsck/mount.c       | 33 +++++++++++++++++++++++++++++++++
>>>>>>>>  fsck/resize.c      | 12 ++++++------
>>>>>>>>  include/f2fs_fs.h  |  6 +++++-
>>>>>>>>  mkfs/f2fs_format.c |  8 ++++++++
>>>>>>>>  4 files changed, 52 insertions(+), 7 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/fsck/mount.c b/fsck/mount.c
>>>>>>>> index 74ff7c6..9019921 100644
>>>>>>>> --- a/fsck/mount.c
>>>>>>>> +++ b/fsck/mount.c
>>>>>>>> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
>>>>>>>>  	DISP_u32(sb, node_ino);
>>>>>>>>  	DISP_u32(sb, meta_ino);
>>>>>>>>  	DISP_u32(sb, cp_payload);
>>>>>>>> +	DISP_u32(sb, crc);
>>>>>>>>  	DISP("%-.256s", sb, version);
>>>>>>>>  	printf("\n");
>>>>>>>>  }
>>>>>>>> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
>>>>>>>>  	if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
>>>>>>>>  		MSG(0, "%s", " lost_found");
>>>>>>>>  	}
>>>>>>>> +	if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
>>>>>>>> +		MSG(0, "%s", " sb_checksum");
>>>>>>>> +	}
>>>>>>>>  	MSG(0, "\n");
>>>>>>>>  	MSG(0, "Info: superblock encrypt level = %d, salt = ",
>>>>>>>>  					sb->encryption_level);
>>>>>>>> @@ -479,10 +483,20 @@ void update_superblock(struct f2fs_super_block *sb, int sb_mask)
>>>>>>>>  {
>>>>>>>>  	int index, ret;
>>>>>>>>  	u_int8_t *buf;
>>>>>>>> +	u32 old_crc, new_crc;
>>>>>>>>  
>>>>>>>>  	buf = calloc(BLOCK_SZ, 1);
>>>>>>>>  	ASSERT(buf);
>>>>>>>>  
>>>>>>>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
>>>>>>>> +		old_crc = get_sb(crc);
>>>>>>>> +		new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
>>>>>>>> +						SB_CHKSUM_OFFSET);
>>>>>>>> +		set_sb(crc, new_crc);
>>>>>>>> +		MSG(1, "Info: SB CRC is updated (0x%x -> 0x%x)\n",
>>>>>>>> +							old_crc, new_crc);
>>>>>>>> +	}
>>>>>>>> +
>>>>>>>>  	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
>>>>>>>>  	for (index = 0; index < SB_ADDR_MAX; index++) {
>>>>>>>>  		if ((1 << index) & sb_mask) {
>>>>>>>> @@ -575,10 +589,29 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
>>>>>>>>  	return 0;
>>>>>>>>  }
>>>>>>>>  
>>>>>>>> +static int verify_sb_chksum(struct f2fs_super_block *sb)
>>>>>>>> +{
>>>>>>>> +	if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
>>>>>>>> +		MSG(0, "\tInvalid SB CRC offset: %u\n",
>>>>>>>> +					get_sb(checksum_offset));
>>>>>>>> +		return -1;
>>>>>>>> +	}
>>>>>>>> +	if (f2fs_crc_valid(get_sb(crc), sb,
>>>>>>>> +			get_sb(checksum_offset))) {
>>>>>>>> +		MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
>>>>>>>> +		return -1;
>>>>>>>> +	}
>>>>>>>> +	return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>>  int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
>>>>>>>>  {
>>>>>>>>  	unsigned int blocksize;
>>>>>>>>  
>>>>>>>> +	if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
>>>>>>>> +					verify_sb_chksum(sb))
>>>>>>>> +		return -1;
>>>>>>>> +
>>>>>>>>  	if (F2FS_SUPER_MAGIC != get_sb(magic))
>>>>>>>>  		return -1;
>>>>>>>>  
>>>>>>>> diff --git a/fsck/resize.c b/fsck/resize.c
>>>>>>>> index 5161a1f..3462165 100644
>>>>>>>> --- a/fsck/resize.c
>>>>>>>> +++ b/fsck/resize.c
>>>>>>>> @@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>>>>>>>>  		}
>>>>>>>>  	}
>>>>>>>>  
>>>>>>>> -	print_raw_sb_info(sb);
>>>>>>>> -	print_raw_sb_info(new_sb);
>>>>>>>
>>>>>>> It'd be worth to keep this to show the previous states.
>>>>>>>
>>>>>>
>>>>>> Here, I just want to move the printing of sb and new_sb to the place
>>>>>> behind update_superblock(), where the crc in sb will be updated.
>>>>>
>>>>> We'd better to see the changes, no?
>>>>>
>>>>
>>>> Yeah, we print sb and new_sb here just for seeing the changes of superblock.
>>>> However, the crc in new_sb will not be updated until calling update_superblock(),
>>>> so if we keep the current printing location, we can't get the changes.
>>>
>>> I mean...
>>> 		print_raw_sb_info(sb);
>>> 		print_raw_sb_info(new_sb);
>>
>> I don't think it's necessary to print new_sb here.
>> Before update_superblock() is called, the crc in new_sb hasn't been updated yet,
>> and is the same with that in sb.
>>
>> Here, as we introduce sb checksum, all we have to do is delay printing new_sb
>> to show the latest crc.
> 
> I see. The contents except crc in sb and new_sb weren't changed during
> migration. If so, printing them later would make sense, right?
> 

Yes, migration wouldn't change the contents of sb and new_sb. And besides,
in my opinion, we don't neet to print the middle status of sb and new_sb,
which would make log too long :)

>>
>> Thanks,
>>
>>> 		update...
>>> 		print_raw_sb_info(new_sb);
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>>
>>>>>>>> -
>>>>>>>>  	old_main_blkaddr = get_sb(main_blkaddr);
>>>>>>>>  	new_main_blkaddr = get_newsb(main_blkaddr);
>>>>>>>>  	offset = new_main_blkaddr - old_main_blkaddr;
>>>>>>>> @@ -629,6 +626,9 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>>>>>>>>  	migrate_sit(sbi, new_sb, offset_seg);
>>>>>>>>  	rebuild_checkpoint(sbi, new_sb, offset_seg);
>>>>>>>>  	update_superblock(new_sb, SB_ALL);
>>>>>>>> +	print_raw_sb_info(sb);
>>>>>>>> +	print_raw_sb_info(new_sb);
>>>>>>>> +
>>>>>>>>  	return 0;
>>>>>>>>  }
>>>>>>>>  
>>>>>>>> @@ -658,9 +658,6 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>>>>>>>>  		}
>>>>>>>>  	}
>>>>>>>>  
>>>>>>>> -	print_raw_sb_info(sb);
>>>>>>>> -	print_raw_sb_info(new_sb);
>>>>>>>
>>>>>>> Ditto.
>>>>>>>
>>>>>>>> -
>>>>>>>>  	old_main_blkaddr = get_sb(main_blkaddr);
>>>>>>>>  	new_main_blkaddr = get_newsb(main_blkaddr);
>>>>>>>>  	offset = old_main_blkaddr - new_main_blkaddr;
>>>>>>>> @@ -690,6 +687,9 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>>>>>>>>  	/* move whole data region */
>>>>>>>>  	//if (err)
>>>>>>>>  	//	migrate_main(sbi, offset);
>>>>>>>> +	print_raw_sb_info(sb);
>>>>>>>> +	print_raw_sb_info(new_sb);
>>>>>>>> +
>>>>>>>>  	return 0;
>>>>>>>>  }
>>>>>>>>  
>>>>>>>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>>>>>>>> index 38a0da4..f80632a 100644
>>>>>>>> --- a/include/f2fs_fs.h
>>>>>>>> +++ b/include/f2fs_fs.h
>>>>>>>> @@ -279,6 +279,7 @@ static inline uint64_t bswap_64(uint64_t val)
>>>>>>>>  #define BITS_PER_BYTE		8
>>>>>>>>  #define F2FS_SUPER_MAGIC	0xF2F52010	/* F2FS Magic Number */
>>>>>>>>  #define CP_CHKSUM_OFFSET	4092
>>>>>>>> +#define SB_CHKSUM_OFFSET	3068
>>>>>>>>  #define MAX_PATH_LEN		64
>>>>>>>>  #define MAX_DEVICES		8
>>>>>>>>  
>>>>>>>> @@ -579,6 +580,7 @@ enum {
>>>>>>>>  #define F2FS_FEATURE_INODE_CRTIME	0x0100
>>>>>>>>  #define F2FS_FEATURE_LOST_FOUND		0x0200
>>>>>>>>  #define F2FS_FEATURE_VERITY		0x0400	/* reserved */
>>>>>>>> +#define F2FS_FEATURE_SB_CHKSUM		0x0800
>>>>>>>>  
>>>>>>>>  #define MAX_VOLUME_NAME		512
>>>>>>>>  
>>>>>>>> @@ -632,7 +634,8 @@ struct f2fs_super_block {
>>>>>>>>  	struct f2fs_device devs[MAX_DEVICES];	/* device list */
>>>>>>>>  	__le32 qf_ino[F2FS_MAX_QUOTAS];	/* quota inode numbers */
>>>>>>>>  	__u8 hot_ext_count;		/* # of hot file extension */
>>>>>>>> -	__u8 reserved[314];		/* valid reserved region */
>>>>>>>> +	__u8 reserved[310];		/* valid reserved region */
>>>>>>>> +	__le32 crc;			/* checksum of superblock */
>>>>>>>>  } __attribute__((packed));
>>>>>>>>  
>>>>>>>>  /*
>>>>>>>> @@ -1338,6 +1341,7 @@ struct feature feature_table[] = {					\
>>>>>>>>  	{ "inode_crtime",		F2FS_FEATURE_INODE_CRTIME },	\
>>>>>>>>  	{ "lost_found",			F2FS_FEATURE_LOST_FOUND },	\
>>>>>>>>  	{ "verity",			F2FS_FEATURE_VERITY },	/* reserved */ \
>>>>>>>> +	{ "sb_checksum",		F2FS_FEATURE_SB_CHKSUM },	\
>>>>>>>>  	{ NULL,				0x0},				\
>>>>>>>>  };
>>>>>>>>  
>>>>>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
>>>>>>>> index 621126c..9b0a0d1 100644
>>>>>>>> --- a/mkfs/f2fs_format.c
>>>>>>>> +++ b/mkfs/f2fs_format.c
>>>>>>>> @@ -504,6 +504,14 @@ static int f2fs_prepare_super_block(void)
>>>>>>>>  
>>>>>>>>  	sb->feature = c.feature;
>>>>>>>>  
>>>>>>>> +	if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
>>>>>>>> +		set_sb(checksum_offset, SB_CHKSUM_OFFSET);
>>>>>>>> +		set_sb(crc, f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
>>>>>>>> +						SB_CHKSUM_OFFSET));
>>>>>>>> +		MSG(1, "Info: SB CRC is set: offset (%d), crc (0x%x)\n",
>>>>>>>> +					get_sb(checksum_offset), get_sb(crc));
>>>>>>>> +	}
>>>>>>>> +
>>>>>>>>  	return 0;
>>>>>>>>  }
>>>>>>>>  
>>>>>>>> -- 
>>>>>>>> 2.19.0
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>
>>> .
>>>
>>
> 
> .
> 

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

* [PATCH 2/5] f2fs-tools: rename CHECKSUM_OFFSET to CP_CHKSUM_OFFSET
  2018-09-28 12:25 [PATCH 1/5] f2fs: support superblock checksum Junling Zheng
@ 2018-09-28 12:25 ` Junling Zheng
  0 siblings, 0 replies; 18+ messages in thread
From: Junling Zheng @ 2018-09-28 12:25 UTC (permalink / raw)
  To: jaegeuk, yuchao0; +Cc: miaoxie, linux-f2fs-devel

This patch renamed CHECKSUM_OFFSET to CP_CHKSUM_OFFSET.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
---
 fsck/fsck.c        |  4 ++--
 fsck/mount.c       |  4 ++--
 fsck/resize.c      |  8 ++++----
 include/f2fs_fs.h  |  6 +++---
 mkfs/f2fs_format.c | 14 +++++++-------
 5 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index d550403..f080d3c 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -2010,8 +2010,8 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi)
 	set_cp(valid_node_count, fsck->chk.valid_node_cnt);
 	set_cp(valid_inode_count, fsck->chk.valid_inode_cnt);
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) = cpu_to_le32(crc);
+	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
+	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) = cpu_to_le32(crc);
 
 	cp_blk_no = get_sb(cp_blkaddr);
 	if (sbi->cur_cp == 2)
diff --git a/fsck/mount.c b/fsck/mount.c
index 2c2473d..1daef75 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -2252,8 +2252,8 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
 	flags = update_nat_bits_flags(sb, cp, flags);
 	set_cp(ckpt_flags, flags);
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) = cpu_to_le32(crc);
+	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
+	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) = cpu_to_le32(crc);
 
 	cp_blk_no = get_sb(cp_blkaddr);
 	if (sbi->cur_cp == 2)
diff --git a/fsck/resize.c b/fsck/resize.c
index fe8a61a..e9612b3 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -90,10 +90,10 @@ static int get_new_sb(struct f2fs_super_block *sb)
 		 * It requires more pages for cp.
 		 */
 		if (max_sit_bitmap_size > MAX_SIT_BITMAP_SIZE_IN_CKPT) {
-			max_nat_bitmap_size = CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1;
+			max_nat_bitmap_size = CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1;
 			set_sb(cp_payload, F2FS_BLK_ALIGN(max_sit_bitmap_size));
 		} else {
-			max_nat_bitmap_size = CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1
+			max_nat_bitmap_size = CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1
 				- max_sit_bitmap_size;
 			set_sb(cp_payload, 0);
 		}
@@ -520,8 +520,8 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
 						(unsigned char *)cp);
 	new_cp->checkpoint_ver = cpu_to_le64(cp_ver + 1);
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, new_cp, CHECKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)new_cp + CHECKSUM_OFFSET)) =
+	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, new_cp, CP_CHKSUM_OFFSET);
+	*((__le32 *)((unsigned char *)new_cp + CP_CHKSUM_OFFSET)) =
 							cpu_to_le32(crc);
 
 	/* Write a new checkpoint in the other set */
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 2c086a9..38a0da4 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -278,7 +278,7 @@ static inline uint64_t bswap_64(uint64_t val)
 #define PAGE_CACHE_SIZE		4096
 #define BITS_PER_BYTE		8
 #define F2FS_SUPER_MAGIC	0xF2F52010	/* F2FS Magic Number */
-#define CHECKSUM_OFFSET		4092
+#define CP_CHKSUM_OFFSET	4092
 #define MAX_PATH_LEN		64
 #define MAX_DEVICES		8
 
@@ -682,9 +682,9 @@ struct f2fs_checkpoint {
 } __attribute__((packed));
 
 #define MAX_SIT_BITMAP_SIZE_IN_CKPT    \
-	(CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1 - 64)
+	(CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1 - 64)
 #define MAX_BITMAP_SIZE_IN_CKPT	\
-	(CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1)
+	(CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1)
 
 /*
  * For orphan inode management
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 4b88d93..621126c 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -342,12 +342,12 @@ static int f2fs_prepare_super_block(void)
 		 * It requires more pages for cp.
 		 */
 		if (max_sit_bitmap_size > MAX_SIT_BITMAP_SIZE_IN_CKPT) {
-			max_nat_bitmap_size = CHECKSUM_OFFSET -
+			max_nat_bitmap_size = CP_CHKSUM_OFFSET -
 					sizeof(struct f2fs_checkpoint) + 1;
 			set_sb(cp_payload, F2FS_BLK_ALIGN(max_sit_bitmap_size));
 	        } else {
 			max_nat_bitmap_size =
-				CHECKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1
+				CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1
 				- max_sit_bitmap_size;
 			set_sb(cp_payload, 0);
 		}
@@ -684,10 +684,10 @@ static int f2fs_write_check_point_pack(void)
 	set_cp(nat_ver_bitmap_bytesize, ((get_sb(segment_count_nat) / 2) <<
 			 get_sb(log_blocks_per_seg)) / 8);
 
-	set_cp(checksum_offset, CHECKSUM_OFFSET);
+	set_cp(checksum_offset, CP_CHKSUM_OFFSET);
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) =
+	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
+	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) =
 							cpu_to_le32(crc);
 
 	blk_size_bytes = 1 << get_sb(log_blocksize);
@@ -932,8 +932,8 @@ static int f2fs_write_check_point_pack(void)
 	 */
 	cp->checkpoint_ver = 0;
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CHECKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CHECKSUM_OFFSET)) =
+	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
+	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) =
 							cpu_to_le32(crc);
 	cp_seg_blk = get_sb(segment0_blkaddr) + c.blks_per_seg;
 	DBG(1, "\tWriting cp page 1 of checkpoint pack 2, at offset 0x%08"PRIx64"\n",
-- 
2.19.0

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

end of thread, other threads:[~2018-09-28 12:26 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-19 13:53 [PATCH 1/5] f2fs: support superblock checksum Junling Zheng
2018-09-19 13:53 ` [PATCH 2/5] f2fs-tools: rename CHECKSUM_OFFSET to CP_CHKSUM_OFFSET Junling Zheng
2018-09-19 13:53 ` [PATCH 3/5] fsck.f2fs: unify the updating of superblocks Junling Zheng
2018-09-19 23:33   ` Jaegeuk Kim
2018-09-19 13:53 ` [PATCH 4/5] f2fs-tools: introduce sb checksum Junling Zheng
2018-09-19 23:35   ` Jaegeuk Kim
2018-09-20  2:17     ` Junling Zheng
2018-09-20 21:38       ` Jaegeuk Kim
2018-09-21  3:09         ` Junling Zheng
2018-09-26  1:57           ` Jaegeuk Kim
2018-09-26  3:54             ` Junling Zheng
2018-09-26 20:27               ` Jaegeuk Kim
2018-09-27  2:46                 ` Junling Zheng
2018-09-19 13:53 ` [PATCH 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack Junling Zheng
2018-09-19 23:38   ` Jaegeuk Kim
2018-09-20  2:38     ` Junling Zheng
2018-09-20  6:45       ` Chao Yu
2018-09-28 12:25 [PATCH 1/5] f2fs: support superblock checksum Junling Zheng
2018-09-28 12:25 ` [PATCH 2/5] f2fs-tools: rename CHECKSUM_OFFSET to CP_CHKSUM_OFFSET Junling Zheng

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.