linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] f2fs-tools: support inode checksum
@ 2017-07-26 15:01 Chao Yu
  2017-07-26 21:40 ` Jaegeuk Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2017-07-26 15:01 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

From: Chao Yu <yuchao0@huawei.com>

This patch introduce a new option 'inode_checksum' for enabling inode
checksum functionality in mkfs/fsck/sload.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fsck/dir.c              |  4 ++++
 fsck/fsck.c             | 27 +++++++++++++++++++++++++--
 fsck/mount.c            |  9 +++++++++
 fsck/segment.c          |  4 ++++
 include/f2fs_fs.h       |  5 +++++
 lib/libf2fs.c           | 22 ++++++++++++++++++++++
 mkfs/f2fs_format.c      |  8 ++++++++
 mkfs/f2fs_format_main.c |  7 +++++++
 8 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/fsck/dir.c b/fsck/dir.c
index 3aa67ec..bbf28aa 100644
--- a/fsck/dir.c
+++ b/fsck/dir.c
@@ -447,6 +447,10 @@ static void init_inode_block(struct f2fs_sb_info *sbi,
 		make_empty_dir(sbi, node_blk);
 	else if (S_ISLNK(mode))
 		page_symlink(sbi, node_blk, de->link, size);
+
+	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
+		node_blk->i.i_inode_checksum =
+			cpu_to_le32(f2fs_inode_chksum(node_blk));
 }
 
 int convert_inline_dentry(struct f2fs_sb_info *sbi, struct f2fs_node *node,
diff --git a/fsck/fsck.c b/fsck/fsck.c
index 7a81855..2787b35 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -857,9 +857,32 @@ skip_blkcnt_fix:
 					nid, i_links);
 		}
 	}
-	if (need_fix && !c.ro) {
-		/* drop extent information to avoid potential wrong access */
+
+	/* drop extent information to avoid potential wrong access */
+	if (need_fix && !c.ro)
 		node_blk->i.i_ext.len = 0;
+
+	if ((c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM)) &&
+				f2fs_has_extra_isize(&node_blk->i)) {
+		__u32 provided, calculated;
+
+		provided = le32_to_cpu(node_blk->i.i_inode_checksum);
+		calculated = f2fs_inode_chksum(node_blk);
+
+		if (provided != calculated) {
+			ASSERT_MSG("ino: 0x%x chksum:0x%x, but calculated one is: 0x%x",
+				nid, provided, calculated);
+			if (c.fix_on) {
+				node_blk->i.i_inode_checksum =
+							cpu_to_le32(calculated);
+				need_fix = 1;
+				FIX_MSG("ino: 0x%x recover, i_inode_checksum= 0x%x -> 0x%x",
+						provided, calculated);
+			}
+		}
+	}
+
+	if (need_fix && !c.ro) {
 		ret = dev_write_block(node_blk, ni->blk_addr);
 		ASSERT(ret >= 0);
 	}
diff --git a/fsck/mount.c b/fsck/mount.c
index 0453f64..8b30264 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -91,6 +91,7 @@ void print_inode_info(struct f2fs_inode *inode, int name)
 	DISP_u16(inode, i_extra_isize);
 	DISP_u16(inode, i_padding);
 	DISP_u32(inode, i_projid);
+	DISP_u32(inode, i_inode_checksum);
 
 	DISP_u32(inode, i_addr[ofs]);		/* Pointers to data blocks */
 	DISP_u32(inode, i_addr[ofs + 1]);	/* Pointers to data blocks */
@@ -291,6 +292,9 @@ void print_sb_state(struct f2fs_super_block *sb)
 	if (f & cpu_to_le32(F2FS_FEATURE_PRJQUOTA)) {
 		MSG(0, "%s", " project quota");
 	}
+	if (f & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM)) {
+		MSG(0, "%s", " inode checksum");
+	}
 	MSG(0, "\n");
 	MSG(0, "Info: superblock encrypt level = %d, salt = ",
 					sb->encryption_level);
@@ -2157,6 +2161,11 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
 	}
 
 	c.bug_on = 0;
+	c.feature = sb->feature;
+
+	/* precompute checksum seed for metadata */
+	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
+		c.chksum_seed = f2fs_cal_crc32(~0, sb->uuid, sizeof(sb->uuid));
 
 	sbi->total_valid_node_count = get_cp(valid_node_count);
 	sbi->total_valid_inode_count = get_cp(valid_inode_count);
diff --git a/fsck/segment.c b/fsck/segment.c
index 5e23b6b..d568d61 100644
--- a/fsck/segment.c
+++ b/fsck/segment.c
@@ -206,6 +206,10 @@ int f2fs_build_file(struct f2fs_sb_info *sbi, struct dentry *de)
 
 		node_blk->i.i_size = cpu_to_le64(de->size);
 
+		if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
+			node_blk->i.i_inode_checksum =
+				cpu_to_le32(f2fs_inode_chksum(node_blk));
+
 		ret = dev_write_block(node_blk, ni.blk_addr);
 		ASSERT(ret >= 0);
 		free(node_blk);
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 380bc2a..dfbe36d 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -316,6 +316,9 @@ struct f2fs_configuration {
 	/* sload parameters */
 	char *from_dir;
 	char *mount_point;
+
+	/* precomputed fs UUID checksum for seeding other checksums */
+	u_int32_t chksum_seed;
 };
 
 #ifdef CONFIG_64BIT
@@ -473,6 +476,7 @@ enum {
 #define F2FS_FEATURE_ATOMIC_WRITE	0x0004
 #define F2FS_FEATURE_EXTRA_ATTR		0x0008
 #define F2FS_FEATURE_PRJQUOTA		0x0010
+#define F2FS_FEATURE_INODE_CHKSUM	0x0020
 
 #define MAX_VOLUME_NAME		512
 
@@ -683,6 +687,7 @@ struct f2fs_inode {
 			__le16 i_extra_isize;	/* extra inode attribute size */
 			__le16 i_padding;	/* padding */
 			__le32 i_projid;	/* project id */
+			__le32 i_inode_checksum;/* inode meta checksum */
 			__le32 i_extra_end[0];	/* for attribute size calculation */
 		};
 		__le32 i_addr[DEF_ADDRS_PER_INODE];	/* Pointers to data blocks */
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index a3091bb..3c0465c 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -493,6 +493,28 @@ int f2fs_crc_valid(u_int32_t blk_crc, void *buf, int len)
 	return 0;
 }
 
+__u32 f2fs_inode_chksum(struct f2fs_node *node)
+{
+	struct f2fs_inode *ri = &node->i;
+	__le32 ino = node->footer.ino;
+	__le32 gen = ri->i_generation;
+	__u32 chksum, chksum_seed;
+	__u32 dummy_cs = 0;
+	unsigned int offset = offsetof(struct f2fs_inode, i_inode_checksum);
+	unsigned int cs_size = sizeof(dummy_cs);
+
+	chksum = f2fs_cal_crc32(c.chksum_seed, (__u8 *)&ino,
+							sizeof(ino));
+	chksum_seed = f2fs_cal_crc32(chksum, (__u8 *)&gen, sizeof(gen));
+
+	chksum = f2fs_cal_crc32(chksum_seed, (__u8 *)ri, offset);
+	chksum = f2fs_cal_crc32(chksum, (__u8 *)&dummy_cs, cs_size);
+	offset += cs_size;
+	chksum = f2fs_cal_crc32(chksum, (__u8 *)ri + offset,
+						F2FS_BLKSIZE - offset);
+	return chksum;
+}
+
 /*
  * try to identify the root device
  */
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 8e068e9..92876b8 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -369,6 +369,10 @@ static int f2fs_prepare_super_block(void)
 
 	uuid_generate(sb->uuid);
 
+	/* precompute checksum seed for metadata */
+	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
+		c.chksum_seed = f2fs_cal_crc32(~0, sb->uuid, sizeof(sb->uuid));
+
 	utf8_to_utf16(sb->volume_name, (const char *)c.vol_label,
 				MAX_VOLUME_NAME, strlen(c.vol_label));
 	set_sb(node_ino, 1);
@@ -940,6 +944,10 @@ static int f2fs_write_root_inode(void)
 	raw_node->i.i_ext.blk_addr = 0;
 	raw_node->i.i_ext.len = 0;
 
+	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
+		raw_node->i.i_inode_checksum =
+			cpu_to_le32(f2fs_inode_chksum(raw_node));
+
 	main_area_node_seg_blk_offset = get_sb(main_blkaddr);
 	main_area_node_seg_blk_offset += c.cur_seg[CURSEG_HOT_NODE] *
 					c.blks_per_seg;
diff --git a/mkfs/f2fs_format_main.c b/mkfs/f2fs_format_main.c
index 39e8e3f..adbbff9 100644
--- a/mkfs/f2fs_format_main.c
+++ b/mkfs/f2fs_format_main.c
@@ -84,6 +84,8 @@ static void parse_feature(const char *features)
 		c.feature |= cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR);
 	} else if (!strcmp(features, "project_quota")) {
 		c.feature |= cpu_to_le32(F2FS_FEATURE_PRJQUOTA);
+	} else if (!strcmp(features, "inode_checksum")) {
+		c.feature |= cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM);
 	} else {
 		MSG(0, "Error: Wrong features\n");
 		mkfs_usage();
@@ -169,6 +171,11 @@ static void f2fs_parse_options(int argc, char *argv[])
 				"enabled with extra attr feature\n");
 			exit(1);
 		}
+		if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM)) {
+			MSG(0, "\tInfo: inode checksum feature should always been"
+				"enabled with extra attr feature\n");
+			exit(1);
+		}
 	}
 
 	if (optind >= argc) {
-- 
2.13.0.90.g1eb437020

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

* Re: [PATCH] f2fs-tools: support inode checksum
  2017-07-26 15:01 [PATCH] f2fs-tools: support inode checksum Chao Yu
@ 2017-07-26 21:40 ` Jaegeuk Kim
  2017-07-26 22:23   ` Chao Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Jaegeuk Kim @ 2017-07-26 21:40 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

Hi Chao,

Just for testing purpose, I filed them in dev-test.git.
Note that, I fixed two build errors below.

Thanks,

On 07/26, Chao Yu wrote:
> From: Chao Yu <yuchao0@huawei.com>
> 
> This patch introduce a new option 'inode_checksum' for enabling inode
> checksum functionality in mkfs/fsck/sload.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fsck/dir.c              |  4 ++++
>  fsck/fsck.c             | 27 +++++++++++++++++++++++++--
>  fsck/mount.c            |  9 +++++++++
>  fsck/segment.c          |  4 ++++
>  include/f2fs_fs.h       |  5 +++++
>  lib/libf2fs.c           | 22 ++++++++++++++++++++++
>  mkfs/f2fs_format.c      |  8 ++++++++
>  mkfs/f2fs_format_main.c |  7 +++++++
>  8 files changed, 84 insertions(+), 2 deletions(-)
> 
> diff --git a/fsck/dir.c b/fsck/dir.c
> index 3aa67ec..bbf28aa 100644
> --- a/fsck/dir.c
> +++ b/fsck/dir.c
> @@ -447,6 +447,10 @@ static void init_inode_block(struct f2fs_sb_info *sbi,
>  		make_empty_dir(sbi, node_blk);
>  	else if (S_ISLNK(mode))
>  		page_symlink(sbi, node_blk, de->link, size);
> +
> +	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
> +		node_blk->i.i_inode_checksum =
> +			cpu_to_le32(f2fs_inode_chksum(node_blk));
>  }
>  
>  int convert_inline_dentry(struct f2fs_sb_info *sbi, struct f2fs_node *node,
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index 7a81855..2787b35 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -857,9 +857,32 @@ skip_blkcnt_fix:
>  					nid, i_links);
>  		}
>  	}
> -	if (need_fix && !c.ro) {
> -		/* drop extent information to avoid potential wrong access */
> +
> +	/* drop extent information to avoid potential wrong access */
> +	if (need_fix && !c.ro)
>  		node_blk->i.i_ext.len = 0;
> +
> +	if ((c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM)) &&
> +				f2fs_has_extra_isize(&node_blk->i)) {
> +		__u32 provided, calculated;
> +
> +		provided = le32_to_cpu(node_blk->i.i_inode_checksum);
> +		calculated = f2fs_inode_chksum(node_blk);
> +
> +		if (provided != calculated) {
> +			ASSERT_MSG("ino: 0x%x chksum:0x%x, but calculated one is: 0x%x",
> +				nid, provided, calculated);
> +			if (c.fix_on) {
> +				node_blk->i.i_inode_checksum =
> +							cpu_to_le32(calculated);
> +				need_fix = 1;
> +				FIX_MSG("ino: 0x%x recover, i_inode_checksum= 0x%x -> 0x%x",
> +						provided, calculated);

						nid, provided, calculated);

Thanks,

> +			}
> +		}
> +	}
> +
> +	if (need_fix && !c.ro) {
>  		ret = dev_write_block(node_blk, ni->blk_addr);
>  		ASSERT(ret >= 0);
>  	}
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 0453f64..8b30264 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -91,6 +91,7 @@ void print_inode_info(struct f2fs_inode *inode, int name)
>  	DISP_u16(inode, i_extra_isize);
>  	DISP_u16(inode, i_padding);
>  	DISP_u32(inode, i_projid);
> +	DISP_u32(inode, i_inode_checksum);
>  
>  	DISP_u32(inode, i_addr[ofs]);		/* Pointers to data blocks */
>  	DISP_u32(inode, i_addr[ofs + 1]);	/* Pointers to data blocks */
> @@ -291,6 +292,9 @@ void print_sb_state(struct f2fs_super_block *sb)
>  	if (f & cpu_to_le32(F2FS_FEATURE_PRJQUOTA)) {
>  		MSG(0, "%s", " project quota");
>  	}
> +	if (f & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM)) {
> +		MSG(0, "%s", " inode checksum");
> +	}
>  	MSG(0, "\n");
>  	MSG(0, "Info: superblock encrypt level = %d, salt = ",
>  					sb->encryption_level);
> @@ -2157,6 +2161,11 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
>  	}
>  
>  	c.bug_on = 0;
> +	c.feature = sb->feature;
> +
> +	/* precompute checksum seed for metadata */
> +	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
> +		c.chksum_seed = f2fs_cal_crc32(~0, sb->uuid, sizeof(sb->uuid));
>  
>  	sbi->total_valid_node_count = get_cp(valid_node_count);
>  	sbi->total_valid_inode_count = get_cp(valid_inode_count);
> diff --git a/fsck/segment.c b/fsck/segment.c
> index 5e23b6b..d568d61 100644
> --- a/fsck/segment.c
> +++ b/fsck/segment.c
> @@ -206,6 +206,10 @@ int f2fs_build_file(struct f2fs_sb_info *sbi, struct dentry *de)
>  
>  		node_blk->i.i_size = cpu_to_le64(de->size);
>  
> +		if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
> +			node_blk->i.i_inode_checksum =
> +				cpu_to_le32(f2fs_inode_chksum(node_blk));
> +
>  		ret = dev_write_block(node_blk, ni.blk_addr);
>  		ASSERT(ret >= 0);
>  		free(node_blk);
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index 380bc2a..dfbe36d 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -316,6 +316,9 @@ struct f2fs_configuration {
>  	/* sload parameters */
>  	char *from_dir;
>  	char *mount_point;
> +
> +	/* precomputed fs UUID checksum for seeding other checksums */
> +	u_int32_t chksum_seed;
>  };
>  
>  #ifdef CONFIG_64BIT
> @@ -473,6 +476,7 @@ enum {
>  #define F2FS_FEATURE_ATOMIC_WRITE	0x0004
>  #define F2FS_FEATURE_EXTRA_ATTR		0x0008
>  #define F2FS_FEATURE_PRJQUOTA		0x0010
> +#define F2FS_FEATURE_INODE_CHKSUM	0x0020
>  
>  #define MAX_VOLUME_NAME		512
>  
> @@ -683,6 +687,7 @@ struct f2fs_inode {
>  			__le16 i_extra_isize;	/* extra inode attribute size */
>  			__le16 i_padding;	/* padding */
>  			__le32 i_projid;	/* project id */
> +			__le32 i_inode_checksum;/* inode meta checksum */
>  			__le32 i_extra_end[0];	/* for attribute size calculation */
>  		};
>  		__le32 i_addr[DEF_ADDRS_PER_INODE];	/* Pointers to data blocks */
> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
> index a3091bb..3c0465c 100644
> --- a/lib/libf2fs.c
> +++ b/lib/libf2fs.c
> @@ -493,6 +493,28 @@ int f2fs_crc_valid(u_int32_t blk_crc, void *buf, int len)
>  	return 0;
>  }
>  
> +__u32 f2fs_inode_chksum(struct f2fs_node *node)

Declare this in f2fs_fs.h.

> +{
> +	struct f2fs_inode *ri = &node->i;
> +	__le32 ino = node->footer.ino;
> +	__le32 gen = ri->i_generation;
> +	__u32 chksum, chksum_seed;
> +	__u32 dummy_cs = 0;
> +	unsigned int offset = offsetof(struct f2fs_inode, i_inode_checksum);
> +	unsigned int cs_size = sizeof(dummy_cs);
> +
> +	chksum = f2fs_cal_crc32(c.chksum_seed, (__u8 *)&ino,
> +							sizeof(ino));
> +	chksum_seed = f2fs_cal_crc32(chksum, (__u8 *)&gen, sizeof(gen));
> +
> +	chksum = f2fs_cal_crc32(chksum_seed, (__u8 *)ri, offset);
> +	chksum = f2fs_cal_crc32(chksum, (__u8 *)&dummy_cs, cs_size);
> +	offset += cs_size;
> +	chksum = f2fs_cal_crc32(chksum, (__u8 *)ri + offset,
> +						F2FS_BLKSIZE - offset);
> +	return chksum;
> +}
> +
>  /*
>   * try to identify the root device
>   */
> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
> index 8e068e9..92876b8 100644
> --- a/mkfs/f2fs_format.c
> +++ b/mkfs/f2fs_format.c
> @@ -369,6 +369,10 @@ static int f2fs_prepare_super_block(void)
>  
>  	uuid_generate(sb->uuid);
>  
> +	/* precompute checksum seed for metadata */
> +	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
> +		c.chksum_seed = f2fs_cal_crc32(~0, sb->uuid, sizeof(sb->uuid));
> +
>  	utf8_to_utf16(sb->volume_name, (const char *)c.vol_label,
>  				MAX_VOLUME_NAME, strlen(c.vol_label));
>  	set_sb(node_ino, 1);
> @@ -940,6 +944,10 @@ static int f2fs_write_root_inode(void)
>  	raw_node->i.i_ext.blk_addr = 0;
>  	raw_node->i.i_ext.len = 0;
>  
> +	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
> +		raw_node->i.i_inode_checksum =
> +			cpu_to_le32(f2fs_inode_chksum(raw_node));
> +
>  	main_area_node_seg_blk_offset = get_sb(main_blkaddr);
>  	main_area_node_seg_blk_offset += c.cur_seg[CURSEG_HOT_NODE] *
>  					c.blks_per_seg;
> diff --git a/mkfs/f2fs_format_main.c b/mkfs/f2fs_format_main.c
> index 39e8e3f..adbbff9 100644
> --- a/mkfs/f2fs_format_main.c
> +++ b/mkfs/f2fs_format_main.c
> @@ -84,6 +84,8 @@ static void parse_feature(const char *features)
>  		c.feature |= cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR);
>  	} else if (!strcmp(features, "project_quota")) {
>  		c.feature |= cpu_to_le32(F2FS_FEATURE_PRJQUOTA);
> +	} else if (!strcmp(features, "inode_checksum")) {
> +		c.feature |= cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM);
>  	} else {
>  		MSG(0, "Error: Wrong features\n");
>  		mkfs_usage();
> @@ -169,6 +171,11 @@ static void f2fs_parse_options(int argc, char *argv[])
>  				"enabled with extra attr feature\n");
>  			exit(1);
>  		}
> +		if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM)) {
> +			MSG(0, "\tInfo: inode checksum feature should always been"
> +				"enabled with extra attr feature\n");
> +			exit(1);
> +		}
>  	}
>  
>  	if (optind >= argc) {
> -- 
> 2.13.0.90.g1eb437020

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

* Re: [PATCH] f2fs-tools: support inode checksum
  2017-07-26 21:40 ` Jaegeuk Kim
@ 2017-07-26 22:23   ` Chao Yu
  2017-08-04  1:38     ` Jaegeuk Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2017-07-26 22:23 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

Hi Jaegeuk,

On 2017/7/27 5:40, Jaegeuk Kim wrote:
> Hi Chao,
> 
> Just for testing purpose, I filed them in dev-test.git.
> Note that, I fixed two build errors below.

Oops, my bad, actually I just note that there is warning in compile flow,
I need -Werror anyway during compiling.

Thanks for fixing.

Thanks,

> 
> Thanks,
> 
> On 07/26, Chao Yu wrote:
>> From: Chao Yu <yuchao0@huawei.com>
>>
>> This patch introduce a new option 'inode_checksum' for enabling inode
>> checksum functionality in mkfs/fsck/sload.
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fsck/dir.c              |  4 ++++
>>  fsck/fsck.c             | 27 +++++++++++++++++++++++++--
>>  fsck/mount.c            |  9 +++++++++
>>  fsck/segment.c          |  4 ++++
>>  include/f2fs_fs.h       |  5 +++++
>>  lib/libf2fs.c           | 22 ++++++++++++++++++++++
>>  mkfs/f2fs_format.c      |  8 ++++++++
>>  mkfs/f2fs_format_main.c |  7 +++++++
>>  8 files changed, 84 insertions(+), 2 deletions(-)
>>
>> diff --git a/fsck/dir.c b/fsck/dir.c
>> index 3aa67ec..bbf28aa 100644
>> --- a/fsck/dir.c
>> +++ b/fsck/dir.c
>> @@ -447,6 +447,10 @@ static void init_inode_block(struct f2fs_sb_info *sbi,
>>  		make_empty_dir(sbi, node_blk);
>>  	else if (S_ISLNK(mode))
>>  		page_symlink(sbi, node_blk, de->link, size);
>> +
>> +	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
>> +		node_blk->i.i_inode_checksum =
>> +			cpu_to_le32(f2fs_inode_chksum(node_blk));
>>  }
>>  
>>  int convert_inline_dentry(struct f2fs_sb_info *sbi, struct f2fs_node *node,
>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>> index 7a81855..2787b35 100644
>> --- a/fsck/fsck.c
>> +++ b/fsck/fsck.c
>> @@ -857,9 +857,32 @@ skip_blkcnt_fix:
>>  					nid, i_links);
>>  		}
>>  	}
>> -	if (need_fix && !c.ro) {
>> -		/* drop extent information to avoid potential wrong access */
>> +
>> +	/* drop extent information to avoid potential wrong access */
>> +	if (need_fix && !c.ro)
>>  		node_blk->i.i_ext.len = 0;
>> +
>> +	if ((c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM)) &&
>> +				f2fs_has_extra_isize(&node_blk->i)) {
>> +		__u32 provided, calculated;
>> +
>> +		provided = le32_to_cpu(node_blk->i.i_inode_checksum);
>> +		calculated = f2fs_inode_chksum(node_blk);
>> +
>> +		if (provided != calculated) {
>> +			ASSERT_MSG("ino: 0x%x chksum:0x%x, but calculated one is: 0x%x",
>> +				nid, provided, calculated);
>> +			if (c.fix_on) {
>> +				node_blk->i.i_inode_checksum =
>> +							cpu_to_le32(calculated);
>> +				need_fix = 1;
>> +				FIX_MSG("ino: 0x%x recover, i_inode_checksum= 0x%x -> 0x%x",
>> +						provided, calculated);
> 
> 						nid, provided, calculated);
> 
> Thanks,
> 
>> +			}
>> +		}
>> +	}
>> +
>> +	if (need_fix && !c.ro) {
>>  		ret = dev_write_block(node_blk, ni->blk_addr);
>>  		ASSERT(ret >= 0);
>>  	}
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index 0453f64..8b30264 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -91,6 +91,7 @@ void print_inode_info(struct f2fs_inode *inode, int name)
>>  	DISP_u16(inode, i_extra_isize);
>>  	DISP_u16(inode, i_padding);
>>  	DISP_u32(inode, i_projid);
>> +	DISP_u32(inode, i_inode_checksum);
>>  
>>  	DISP_u32(inode, i_addr[ofs]);		/* Pointers to data blocks */
>>  	DISP_u32(inode, i_addr[ofs + 1]);	/* Pointers to data blocks */
>> @@ -291,6 +292,9 @@ void print_sb_state(struct f2fs_super_block *sb)
>>  	if (f & cpu_to_le32(F2FS_FEATURE_PRJQUOTA)) {
>>  		MSG(0, "%s", " project quota");
>>  	}
>> +	if (f & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM)) {
>> +		MSG(0, "%s", " inode checksum");
>> +	}
>>  	MSG(0, "\n");
>>  	MSG(0, "Info: superblock encrypt level = %d, salt = ",
>>  					sb->encryption_level);
>> @@ -2157,6 +2161,11 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
>>  	}
>>  
>>  	c.bug_on = 0;
>> +	c.feature = sb->feature;
>> +
>> +	/* precompute checksum seed for metadata */
>> +	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
>> +		c.chksum_seed = f2fs_cal_crc32(~0, sb->uuid, sizeof(sb->uuid));
>>  
>>  	sbi->total_valid_node_count = get_cp(valid_node_count);
>>  	sbi->total_valid_inode_count = get_cp(valid_inode_count);
>> diff --git a/fsck/segment.c b/fsck/segment.c
>> index 5e23b6b..d568d61 100644
>> --- a/fsck/segment.c
>> +++ b/fsck/segment.c
>> @@ -206,6 +206,10 @@ int f2fs_build_file(struct f2fs_sb_info *sbi, struct dentry *de)
>>  
>>  		node_blk->i.i_size = cpu_to_le64(de->size);
>>  
>> +		if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
>> +			node_blk->i.i_inode_checksum =
>> +				cpu_to_le32(f2fs_inode_chksum(node_blk));
>> +
>>  		ret = dev_write_block(node_blk, ni.blk_addr);
>>  		ASSERT(ret >= 0);
>>  		free(node_blk);
>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>> index 380bc2a..dfbe36d 100644
>> --- a/include/f2fs_fs.h
>> +++ b/include/f2fs_fs.h
>> @@ -316,6 +316,9 @@ struct f2fs_configuration {
>>  	/* sload parameters */
>>  	char *from_dir;
>>  	char *mount_point;
>> +
>> +	/* precomputed fs UUID checksum for seeding other checksums */
>> +	u_int32_t chksum_seed;
>>  };
>>  
>>  #ifdef CONFIG_64BIT
>> @@ -473,6 +476,7 @@ enum {
>>  #define F2FS_FEATURE_ATOMIC_WRITE	0x0004
>>  #define F2FS_FEATURE_EXTRA_ATTR		0x0008
>>  #define F2FS_FEATURE_PRJQUOTA		0x0010
>> +#define F2FS_FEATURE_INODE_CHKSUM	0x0020
>>  
>>  #define MAX_VOLUME_NAME		512
>>  
>> @@ -683,6 +687,7 @@ struct f2fs_inode {
>>  			__le16 i_extra_isize;	/* extra inode attribute size */
>>  			__le16 i_padding;	/* padding */
>>  			__le32 i_projid;	/* project id */
>> +			__le32 i_inode_checksum;/* inode meta checksum */
>>  			__le32 i_extra_end[0];	/* for attribute size calculation */
>>  		};
>>  		__le32 i_addr[DEF_ADDRS_PER_INODE];	/* Pointers to data blocks */
>> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
>> index a3091bb..3c0465c 100644
>> --- a/lib/libf2fs.c
>> +++ b/lib/libf2fs.c
>> @@ -493,6 +493,28 @@ int f2fs_crc_valid(u_int32_t blk_crc, void *buf, int len)
>>  	return 0;
>>  }
>>  
>> +__u32 f2fs_inode_chksum(struct f2fs_node *node)
> 
> Declare this in f2fs_fs.h.
> 
>> +{
>> +	struct f2fs_inode *ri = &node->i;
>> +	__le32 ino = node->footer.ino;
>> +	__le32 gen = ri->i_generation;
>> +	__u32 chksum, chksum_seed;
>> +	__u32 dummy_cs = 0;
>> +	unsigned int offset = offsetof(struct f2fs_inode, i_inode_checksum);
>> +	unsigned int cs_size = sizeof(dummy_cs);
>> +
>> +	chksum = f2fs_cal_crc32(c.chksum_seed, (__u8 *)&ino,
>> +							sizeof(ino));
>> +	chksum_seed = f2fs_cal_crc32(chksum, (__u8 *)&gen, sizeof(gen));
>> +
>> +	chksum = f2fs_cal_crc32(chksum_seed, (__u8 *)ri, offset);
>> +	chksum = f2fs_cal_crc32(chksum, (__u8 *)&dummy_cs, cs_size);
>> +	offset += cs_size;
>> +	chksum = f2fs_cal_crc32(chksum, (__u8 *)ri + offset,
>> +						F2FS_BLKSIZE - offset);
>> +	return chksum;
>> +}
>> +
>>  /*
>>   * try to identify the root device
>>   */
>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
>> index 8e068e9..92876b8 100644
>> --- a/mkfs/f2fs_format.c
>> +++ b/mkfs/f2fs_format.c
>> @@ -369,6 +369,10 @@ static int f2fs_prepare_super_block(void)
>>  
>>  	uuid_generate(sb->uuid);
>>  
>> +	/* precompute checksum seed for metadata */
>> +	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
>> +		c.chksum_seed = f2fs_cal_crc32(~0, sb->uuid, sizeof(sb->uuid));
>> +
>>  	utf8_to_utf16(sb->volume_name, (const char *)c.vol_label,
>>  				MAX_VOLUME_NAME, strlen(c.vol_label));
>>  	set_sb(node_ino, 1);
>> @@ -940,6 +944,10 @@ static int f2fs_write_root_inode(void)
>>  	raw_node->i.i_ext.blk_addr = 0;
>>  	raw_node->i.i_ext.len = 0;
>>  
>> +	if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM))
>> +		raw_node->i.i_inode_checksum =
>> +			cpu_to_le32(f2fs_inode_chksum(raw_node));
>> +
>>  	main_area_node_seg_blk_offset = get_sb(main_blkaddr);
>>  	main_area_node_seg_blk_offset += c.cur_seg[CURSEG_HOT_NODE] *
>>  					c.blks_per_seg;
>> diff --git a/mkfs/f2fs_format_main.c b/mkfs/f2fs_format_main.c
>> index 39e8e3f..adbbff9 100644
>> --- a/mkfs/f2fs_format_main.c
>> +++ b/mkfs/f2fs_format_main.c
>> @@ -84,6 +84,8 @@ static void parse_feature(const char *features)
>>  		c.feature |= cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR);
>>  	} else if (!strcmp(features, "project_quota")) {
>>  		c.feature |= cpu_to_le32(F2FS_FEATURE_PRJQUOTA);
>> +	} else if (!strcmp(features, "inode_checksum")) {
>> +		c.feature |= cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM);
>>  	} else {
>>  		MSG(0, "Error: Wrong features\n");
>>  		mkfs_usage();
>> @@ -169,6 +171,11 @@ static void f2fs_parse_options(int argc, char *argv[])
>>  				"enabled with extra attr feature\n");
>>  			exit(1);
>>  		}
>> +		if (c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM)) {
>> +			MSG(0, "\tInfo: inode checksum feature should always been"
>> +				"enabled with extra attr feature\n");
>> +			exit(1);
>> +		}
>>  	}
>>  
>>  	if (optind >= argc) {
>> -- 
>> 2.13.0.90.g1eb437020

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

* Re: [PATCH] f2fs-tools: support inode checksum
  2017-07-26 22:23   ` Chao Yu
@ 2017-08-04  1:38     ` Jaegeuk Kim
  2017-08-04  3:15       ` Chao Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Jaegeuk Kim @ 2017-08-04  1:38 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu

Hi Chao,

It seems three is missing case when verifying the checksum, if the page is
got from cache with some updates. We need to verify it when actually reading
the node block.

Let me modify your patch like this. Is that okay for you?

Thanks,

---
 fs/f2fs/f2fs.h    |  3 ++-
 fs/f2fs/inode.c   | 62 +++++++++++++++++++++++++++++--------------------------
 fs/f2fs/node.c    |  7 ++++++-
 fs/f2fs/segment.c |  2 +-
 4 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 5a078cd6f68d..44e46a31509b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2362,7 +2362,8 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
  * inode.c
  */
 void f2fs_set_inode_flags(struct inode *inode);
-void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct f2fs_node *node);
+bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi, struct page *page);
+void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct page *page);
 struct inode *f2fs_iget(struct super_block *sb, unsigned long ino);
 struct inode *f2fs_iget_retry(struct super_block *sb, unsigned long ino);
 int try_to_free_nats(struct f2fs_sb_info *sbi, int nr_shrink);
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 05c8aeb0101e..b4c401d456e7 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -108,9 +108,26 @@ static void __recover_inline_status(struct inode *inode, struct page *ipage)
 	return;
 }
 
-static __u32 f2fs_inode_chksum(struct f2fs_sb_info *sbi,
-					struct f2fs_node *node)
+static bool f2fs_enable_inode_chksum(struct f2fs_sb_info *sbi, struct page *page)
 {
+	struct f2fs_inode *ri = &F2FS_NODE(page)->i;
+	int extra_isize = le32_to_cpu(ri->i_extra_isize);
+
+	if (!f2fs_sb_has_inode_chksum(sbi->sb))
+		return false;
+
+	if (!RAW_IS_INODE(F2FS_NODE(page)) || !(ri->i_inline & F2FS_EXTRA_ATTR))
+		return false;
+
+	if (!F2FS_FITS_IN_INODE(ri, extra_isize, i_inode_checksum))
+		return false;
+
+	return true;
+}
+
+static __u32 f2fs_inode_chksum(struct f2fs_sb_info *sbi, struct page *page)
+{
+	struct f2fs_node *node = F2FS_NODE(page);
 	struct f2fs_inode *ri = &node->i;
 	__le32 ino = node->footer.ino;
 	__le32 gen = ri->i_generation;
@@ -131,39 +148,34 @@ static __u32 f2fs_inode_chksum(struct f2fs_sb_info *sbi,
 	return chksum;
 }
 
-static bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi,
-			struct f2fs_node *node, struct f2fs_inode_info *fi)
+bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi, struct page *page)
 {
-	struct f2fs_inode *ri = &node->i;
+	struct f2fs_inode *ri;
 	__u32 provided, calculated;
 
-	if (!f2fs_sb_has_inode_chksum(sbi->sb))
-		return true;
-
-	if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_inode_checksum))
+	if (!f2fs_enable_inode_chksum(sbi, page))
 		return true;
 
+	ri = &F2FS_NODE(page)->i;
 	provided = le32_to_cpu(ri->i_inode_checksum);
-	calculated = f2fs_inode_chksum(sbi, node);
+	calculated = f2fs_inode_chksum(sbi, page);
+
+	if (provided != calculated)
+		f2fs_msg(sbi->sb, KERN_WARNING,
+			"checksum invalid, ino = %x, %x vs. %x",
+			ino_of_node(page), provided, calculated);
 
 	return provided == calculated;
 }
 
-void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct f2fs_node *node)
+void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct page *page)
 {
-	struct f2fs_inode *ri = &node->i;
-	int extra_isize = le32_to_cpu(ri->i_extra_isize);
-
-	if (!f2fs_sb_has_inode_chksum(sbi->sb))
-		return;
-
-	if (!RAW_IS_INODE(node) || !(ri->i_inline & F2FS_EXTRA_ATTR))
-		return;
+	struct f2fs_inode *ri = &F2FS_NODE(page)->i;
 
-	if (!F2FS_FITS_IN_INODE(ri, extra_isize, i_inode_checksum))
+	if (!f2fs_enable_inode_chksum(sbi, page))
 		return;
 
-	ri->i_inode_checksum = cpu_to_le32(f2fs_inode_chksum(sbi, node));
+	ri->i_inode_checksum = cpu_to_le32(f2fs_inode_chksum(sbi, page));
 }
 
 static int do_read_inode(struct inode *inode)
@@ -219,14 +231,6 @@ static int do_read_inode(struct inode *inode)
 	fi->i_extra_isize = f2fs_has_extra_attr(inode) ?
 					le16_to_cpu(ri->i_extra_isize) : 0;
 
-	if (!f2fs_inode_chksum_verify(sbi, F2FS_NODE(node_page), fi)) {
-		f2fs_msg(sbi->sb, KERN_WARNING,
-			"checksum invalid, ino:%lu, on-disk:%u",
-			inode->i_ino, le32_to_cpu(ri->i_inode_checksum));
-		f2fs_put_page(node_page, 1);
-		return -EBADMSG;
-	}
-
 	/* check data exist */
 	if (f2fs_has_inline_data(inode) && !f2fs_exist_data(inode))
 		__recover_inline_status(inode, node_page);
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index b08f0d9bd86f..9168c304fd58 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1171,6 +1171,11 @@ static struct page *__get_node_page(struct f2fs_sb_info *sbi, pgoff_t nid,
 		err = -EIO;
 		goto out_err;
 	}
+
+	if (!f2fs_inode_chksum_verify(sbi, page)) {
+		err = -EBADMSG;
+		goto out_err;
+	}
 page_hit:
 	if(unlikely(nid != nid_of_node(page))) {
 		f2fs_msg(sbi->sb, KERN_WARNING, "inconsistent node block, "
@@ -2279,7 +2284,7 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)
 								i_projid))
 			dst->i_projid = src->i_projid;
 
-		f2fs_inode_chksum_set(sbi, F2FS_NODE(ipage));
+		f2fs_inode_chksum_set(sbi, ipage);
 	}
 
 	new_ni = old_ni;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 151968ecc694..d9f4497890d7 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2222,7 +2222,7 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
 	if (page && IS_NODESEG(type)) {
 		fill_node_footer_blkaddr(page, NEXT_FREE_BLKADDR(sbi, curseg));
 
-		f2fs_inode_chksum_set(sbi, F2FS_NODE(page));
+		f2fs_inode_chksum_set(sbi, page);
 	}
 
 	if (add_list) {
-- 
2.13.0.rc1.294.g07d810a77f-goog

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

* Re: [PATCH] f2fs-tools: support inode checksum
  2017-08-04  1:38     ` Jaegeuk Kim
@ 2017-08-04  3:15       ` Chao Yu
  0 siblings, 0 replies; 5+ messages in thread
From: Chao Yu @ 2017-08-04  3:15 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel, linux-kernel

Hi Jaegeuk,

On 2017/8/4 9:38, Jaegeuk Kim wrote:
> Hi Chao,
> 
> It seems three is missing case when verifying the checksum, if the page is
> got from cache with some updates. We need to verify it when actually reading
> the node block.

Agreed.

> 
> Let me modify your patch like this. Is that okay for you?

Looks good to me, thank you. ;)

Thanks,

> 
> Thanks,
> 
> ---
>  fs/f2fs/f2fs.h    |  3 ++-
>  fs/f2fs/inode.c   | 62 +++++++++++++++++++++++++++++--------------------------
>  fs/f2fs/node.c    |  7 ++++++-
>  fs/f2fs/segment.c |  2 +-
>  4 files changed, 42 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 5a078cd6f68d..44e46a31509b 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2362,7 +2362,8 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
>   * inode.c
>   */
>  void f2fs_set_inode_flags(struct inode *inode);
> -void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct f2fs_node *node);
> +bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi, struct page *page);
> +void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct page *page);
>  struct inode *f2fs_iget(struct super_block *sb, unsigned long ino);
>  struct inode *f2fs_iget_retry(struct super_block *sb, unsigned long ino);
>  int try_to_free_nats(struct f2fs_sb_info *sbi, int nr_shrink);
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index 05c8aeb0101e..b4c401d456e7 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -108,9 +108,26 @@ static void __recover_inline_status(struct inode *inode, struct page *ipage)
>  	return;
>  }
>  
> -static __u32 f2fs_inode_chksum(struct f2fs_sb_info *sbi,
> -					struct f2fs_node *node)
> +static bool f2fs_enable_inode_chksum(struct f2fs_sb_info *sbi, struct page *page)
>  {
> +	struct f2fs_inode *ri = &F2FS_NODE(page)->i;
> +	int extra_isize = le32_to_cpu(ri->i_extra_isize);
> +
> +	if (!f2fs_sb_has_inode_chksum(sbi->sb))
> +		return false;
> +
> +	if (!RAW_IS_INODE(F2FS_NODE(page)) || !(ri->i_inline & F2FS_EXTRA_ATTR))
> +		return false;
> +
> +	if (!F2FS_FITS_IN_INODE(ri, extra_isize, i_inode_checksum))
> +		return false;
> +
> +	return true;
> +}
> +
> +static __u32 f2fs_inode_chksum(struct f2fs_sb_info *sbi, struct page *page)
> +{
> +	struct f2fs_node *node = F2FS_NODE(page);
>  	struct f2fs_inode *ri = &node->i;
>  	__le32 ino = node->footer.ino;
>  	__le32 gen = ri->i_generation;
> @@ -131,39 +148,34 @@ static __u32 f2fs_inode_chksum(struct f2fs_sb_info *sbi,
>  	return chksum;
>  }
>  
> -static bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi,
> -			struct f2fs_node *node, struct f2fs_inode_info *fi)
> +bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi, struct page *page)
>  {
> -	struct f2fs_inode *ri = &node->i;
> +	struct f2fs_inode *ri;
>  	__u32 provided, calculated;
>  
> -	if (!f2fs_sb_has_inode_chksum(sbi->sb))
> -		return true;
> -
> -	if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_inode_checksum))
> +	if (!f2fs_enable_inode_chksum(sbi, page))
>  		return true;
>  
> +	ri = &F2FS_NODE(page)->i;
>  	provided = le32_to_cpu(ri->i_inode_checksum);
> -	calculated = f2fs_inode_chksum(sbi, node);
> +	calculated = f2fs_inode_chksum(sbi, page);
> +
> +	if (provided != calculated)
> +		f2fs_msg(sbi->sb, KERN_WARNING,
> +			"checksum invalid, ino = %x, %x vs. %x",
> +			ino_of_node(page), provided, calculated);
>  
>  	return provided == calculated;
>  }
>  
> -void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct f2fs_node *node)
> +void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct page *page)
>  {
> -	struct f2fs_inode *ri = &node->i;
> -	int extra_isize = le32_to_cpu(ri->i_extra_isize);
> -
> -	if (!f2fs_sb_has_inode_chksum(sbi->sb))
> -		return;
> -
> -	if (!RAW_IS_INODE(node) || !(ri->i_inline & F2FS_EXTRA_ATTR))
> -		return;
> +	struct f2fs_inode *ri = &F2FS_NODE(page)->i;
>  
> -	if (!F2FS_FITS_IN_INODE(ri, extra_isize, i_inode_checksum))
> +	if (!f2fs_enable_inode_chksum(sbi, page))
>  		return;
>  
> -	ri->i_inode_checksum = cpu_to_le32(f2fs_inode_chksum(sbi, node));
> +	ri->i_inode_checksum = cpu_to_le32(f2fs_inode_chksum(sbi, page));
>  }
>  
>  static int do_read_inode(struct inode *inode)
> @@ -219,14 +231,6 @@ static int do_read_inode(struct inode *inode)
>  	fi->i_extra_isize = f2fs_has_extra_attr(inode) ?
>  					le16_to_cpu(ri->i_extra_isize) : 0;
>  
> -	if (!f2fs_inode_chksum_verify(sbi, F2FS_NODE(node_page), fi)) {
> -		f2fs_msg(sbi->sb, KERN_WARNING,
> -			"checksum invalid, ino:%lu, on-disk:%u",
> -			inode->i_ino, le32_to_cpu(ri->i_inode_checksum));
> -		f2fs_put_page(node_page, 1);
> -		return -EBADMSG;
> -	}
> -
>  	/* check data exist */
>  	if (f2fs_has_inline_data(inode) && !f2fs_exist_data(inode))
>  		__recover_inline_status(inode, node_page);
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index b08f0d9bd86f..9168c304fd58 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1171,6 +1171,11 @@ static struct page *__get_node_page(struct f2fs_sb_info *sbi, pgoff_t nid,
>  		err = -EIO;
>  		goto out_err;
>  	}
> +
> +	if (!f2fs_inode_chksum_verify(sbi, page)) {
> +		err = -EBADMSG;
> +		goto out_err;
> +	}
>  page_hit:
>  	if(unlikely(nid != nid_of_node(page))) {
>  		f2fs_msg(sbi->sb, KERN_WARNING, "inconsistent node block, "
> @@ -2279,7 +2284,7 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)
>  								i_projid))
>  			dst->i_projid = src->i_projid;
>  
> -		f2fs_inode_chksum_set(sbi, F2FS_NODE(ipage));
> +		f2fs_inode_chksum_set(sbi, ipage);
>  	}
>  
>  	new_ni = old_ni;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 151968ecc694..d9f4497890d7 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2222,7 +2222,7 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>  	if (page && IS_NODESEG(type)) {
>  		fill_node_footer_blkaddr(page, NEXT_FREE_BLKADDR(sbi, curseg));
>  
> -		f2fs_inode_chksum_set(sbi, F2FS_NODE(page));
> +		f2fs_inode_chksum_set(sbi, page);
>  	}
>  
>  	if (add_list) {
> 

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

end of thread, other threads:[~2017-08-04  3:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-26 15:01 [PATCH] f2fs-tools: support inode checksum Chao Yu
2017-07-26 21:40 ` Jaegeuk Kim
2017-07-26 22:23   ` Chao Yu
2017-08-04  1:38     ` Jaegeuk Kim
2017-08-04  3:15       ` Chao Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).