linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] f2fs-tools: allow unfixed f2fs_checkpoint.checksum_offset
@ 2019-04-22  9:34 Chao Yu
  2019-04-22  9:34 ` [PATCH 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature Chao Yu
  0 siblings, 1 reply; 4+ messages in thread
From: Chao Yu @ 2019-04-22  9:34 UTC (permalink / raw)
  To: linux-f2fs-devel, jaegeuk, chao; +Cc: linux-kernel, Chao Yu

Previously, f2fs_checkpoint.checksum_offset points fixed position of
f2fs_checkpoint structure:

"#define CP_CHKSUM_OFFSET	4092"

It is unnecessary, and it breaks the consecutiveness of nat and sit
bitmap stored across checkpoint park block and payload blocks.

This patch allows f2fs-tools to handle unfixed .checksum_offset.

In addition, for the case checksum value is stored in the middle of
checkpoint park, calculating checksum value with superposition method
like we did for inode_checksum.

In addition, using MAX_BITMAP_SIZE_IN_CKPT to clean up codes.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fsck/fsck.c        |  5 +++--
 fsck/mount.c       |  5 +++--
 fsck/resize.c      | 10 +++++-----
 include/f2fs_fs.h  |  1 +
 lib/libf2fs.c      | 14 ++++++++++++++
 mkfs/f2fs_format.c | 16 +++++++---------
 6 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index a17555c..7ecdee1 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -2056,8 +2056,9 @@ 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, CP_CHKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) = cpu_to_le32(crc);
+	crc = f2fs_checkpoint_chksum(cp);
+	*((__le32 *)((unsigned char *)cp + get_cp(checksum_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 aa64e93..da1d4c9 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -2373,8 +2373,9 @@ 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, CP_CHKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) = cpu_to_le32(crc);
+	crc = f2fs_checkpoint_chksum(cp);
+	*((__le32 *)((unsigned char *)cp + get_cp(checksum_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 56c8103..2c623c5 100644
--- a/fsck/resize.c
+++ b/fsck/resize.c
@@ -90,11 +90,11 @@ 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 = CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1;
+			max_nat_bitmap_size = MAX_BITMAP_SIZE_IN_CKPT;
 			set_sb(cp_payload, F2FS_BLK_ALIGN(max_sit_bitmap_size));
 		} else {
-			max_nat_bitmap_size = CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1
-				- max_sit_bitmap_size;
+			max_nat_bitmap_size = MAX_BITMAP_SIZE_IN_CKPT -
+							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, CP_CHKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)new_cp + CP_CHKSUM_OFFSET)) =
+	crc = f2fs_checkpoint_chksum(cp);
+	*((__le32 *)((unsigned char *)new_cp + get_cp(checksum_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 c6669c9..1bd935c 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -1133,6 +1133,7 @@ extern int utf16_to_utf8(char *, const u_int16_t *, size_t, size_t);
 extern int log_base_2(u_int32_t);
 extern unsigned int addrs_per_inode(struct f2fs_inode *);
 extern __u32 f2fs_inode_chksum(struct f2fs_node *);
+extern __u32 f2fs_checkpoint_chksum(struct f2fs_checkpoint *);
 
 extern int get_bits_in_byte(unsigned char n);
 extern int test_and_set_bit_le(u32, u8 *);
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index 853e713..4202cd1 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -532,6 +532,20 @@ __u32 f2fs_inode_chksum(struct f2fs_node *node)
 	return chksum;
 }
 
+__u32 f2fs_checkpoint_chksum(struct f2fs_checkpoint *cp)
+{
+	unsigned int chksum_ofs = le32_to_cpu(cp->checksum_offset);
+	__u32 chksum;
+
+	chksum = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, chksum_ofs);
+	if (chksum_ofs < CP_CHKSUM_OFFSET) {
+		chksum_ofs += sizeof(chksum);
+		chksum = f2fs_cal_crc32(chksum, (__u8 *)cp + chksum_ofs,
+						F2FS_BLKSIZE - chksum_ofs);
+	}
+	return chksum;
+}
+
 /*
  * try to identify the root device
  */
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 4560611..a534293 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -358,13 +358,11 @@ 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 = CP_CHKSUM_OFFSET -
-					sizeof(struct f2fs_checkpoint) + 1;
+			max_nat_bitmap_size = MAX_BITMAP_SIZE_IN_CKPT;
 			set_sb(cp_payload, F2FS_BLK_ALIGN(max_sit_bitmap_size));
 	        } else {
-			max_nat_bitmap_size =
-				CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1
-				- max_sit_bitmap_size;
+			max_nat_bitmap_size = MAX_BITMAP_SIZE_IN_CKPT -
+							max_sit_bitmap_size;
 			set_sb(cp_payload, 0);
 		}
 		max_nat_segments = (max_nat_bitmap_size * 8) >> log_blks_per_seg;
@@ -710,8 +708,8 @@ static int f2fs_write_check_point_pack(void)
 
 	set_cp(checksum_offset, CP_CHKSUM_OFFSET);
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) =
+	crc = f2fs_checkpoint_chksum(cp);
+	*((__le32 *)((unsigned char *)cp + get_cp(checksum_offset))) =
 							cpu_to_le32(crc);
 
 	blk_size_bytes = 1 << get_sb(log_blocksize);
@@ -956,8 +954,8 @@ static int f2fs_write_check_point_pack(void)
 	 */
 	cp->checkpoint_ver = 0;
 
-	crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, cp, CP_CHKSUM_OFFSET);
-	*((__le32 *)((unsigned char *)cp + CP_CHKSUM_OFFSET)) =
+	crc = f2fs_checkpoint_chksum(cp);
+	*((__le32 *)((unsigned char *)cp + get_cp(checksum_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.18.0.rc1


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

* [PATCH 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature
  2019-04-22  9:34 [PATCH 1/2] f2fs-tools: allow unfixed f2fs_checkpoint.checksum_offset Chao Yu
@ 2019-04-22  9:34 ` Chao Yu
  2019-04-24 12:00   ` [f2fs-dev] " Ju Hyung Park
  0 siblings, 1 reply; 4+ messages in thread
From: Chao Yu @ 2019-04-22  9:34 UTC (permalink / raw)
  To: linux-f2fs-devel, jaegeuk, chao; +Cc: linux-kernel, Chao Yu

For large_nat_bitmap feature, there is a design flaw:

Previous:

struct f2fs_checkpoint layout:
+--------------------------+  0x0000
| checkpoint_ver           |
| ......                   |
| checksum_offset          |------+
| ......                   |      |
| sit_nat_version_bitmap[] |<-----|-------+
| ......                   |      |       |
| checksum_value           |<-----+       |
+--------------------------+  0x1000      |
|                          |      nat_bitmap + sit_bitmap
| payload blocks           |              |
|                          |              |
+--------------------------|<-------------+

Obviously, if nat_bitmap size + sit_bitmap size is larger than
MAX_BITMAP_SIZE_IN_CKPT, nat_bitmap or sit_bitmap may overlap
checkpoint checksum's position, once checkpoint() is triggered
from kernel, nat or sit bitmap will be damaged by checksum field.

In order to fix this, let's relocate checksum_value's position
to the head of sit_nat_version_bitmap as below, then nat/sit
bitmap and chksum value update will become safe.

After:

struct f2fs_checkpoint layout:
+--------------------------+  0x0000
| checkpoint_ver           |
| ......                   |
| checksum_offset          |------+
| ......                   |      |
| sit_nat_version_bitmap[] |<-----+
| ......                   |<-------------+
|                          |              |
+--------------------------+  0x1000      |
|                          |      nat_bitmap + sit_bitmap
| payload blocks           |              |
|                          |              |
+--------------------------|<-------------+

Related report and discussion:

https://sourceforge.net/p/linux-f2fs/mailman/message/36642346/

In addition, during writing checkpoint, if large_nat_bitmap feature is
enabled, we need to set CP_LARGE_NAT_BITMAP_FLAG flag in checkpoint.

Reported-by: Park Ju Hyung <qkrwngud825@gmail.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fsck/f2fs.h        |  6 +++++-
 fsck/fsck.c        | 16 ++++++++++++++++
 fsck/fsck.h        |  1 +
 fsck/main.c        |  2 ++
 fsck/mount.c       | 42 +++++++++++++++++++++++++++---------------
 include/f2fs_fs.h  |  9 +++++++--
 mkfs/f2fs_format.c |  5 ++++-
 7 files changed, 62 insertions(+), 19 deletions(-)

diff --git a/fsck/f2fs.h b/fsck/f2fs.h
index 93f01e5..6a6c4a5 100644
--- a/fsck/f2fs.h
+++ b/fsck/f2fs.h
@@ -272,7 +272,11 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
 	if (is_set_ckpt_flags(ckpt, CP_LARGE_NAT_BITMAP_FLAG)) {
 		offset = (flag == SIT_BITMAP) ?
 			le32_to_cpu(ckpt->nat_ver_bitmap_bytesize) : 0;
-		return &ckpt->sit_nat_version_bitmap + offset;
+		/*
+		 * if large_nat_bitmap feature is enabled, leave checksum
+		 * protection for all nat/sit bitmaps.
+		 */
+		return &ckpt->sit_nat_version_bitmap + offset + sizeof(__le32);
 	}
 
 	if (le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload) > 0) {
diff --git a/fsck/fsck.c b/fsck/fsck.c
index 7ecdee1..8d7deb5 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -1917,6 +1917,18 @@ int fsck_chk_meta(struct f2fs_sb_info *sbi)
 	return 0;
 }
 
+void fsck_chk_checkpoint(struct f2fs_sb_info *sbi)
+{
+	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
+
+	if (get_cp(ckpt_flags) & CP_LARGE_NAT_BITMAP_FLAG) {
+		if (get_cp(checksum_offset) != CP_MIN_CHKSUM_OFFSET) {
+			ASSERT_MSG("Deprecated layout of large_nat_bitmap, "
+				"chksum_offset:%u", get_cp(checksum_offset));
+		}
+	}
+}
+
 void fsck_init(struct f2fs_sb_info *sbi)
 {
 	struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
@@ -2038,6 +2050,10 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi)
 		flags |= CP_TRIMMED_FLAG;
 	if (is_set_ckpt_flags(cp, CP_DISABLED_FLAG))
 		flags |= CP_DISABLED_FLAG;
+	if (is_set_ckpt_flags(cp, CP_LARGE_NAT_BITMAP_FLAG)) {
+		flags |= CP_LARGE_NAT_BITMAP_FLAG;
+		set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
+	}
 
 	if (flags & CP_UMOUNT_FLAG)
 		cp_blocks = 8;
diff --git a/fsck/fsck.h b/fsck/fsck.h
index 376ced7..dd831de 100644
--- a/fsck/fsck.h
+++ b/fsck/fsck.h
@@ -154,6 +154,7 @@ extern int fsck_chk_dentry_blk(struct f2fs_sb_info *, u32, struct child_info *,
 		int, int);
 int fsck_chk_inline_dentries(struct f2fs_sb_info *, struct f2fs_node *,
 		struct child_info *);
+void fsck_chk_checkpoint(struct f2fs_sb_info *sbi);
 int fsck_chk_meta(struct f2fs_sb_info *sbi);
 int fsck_chk_curseg_info(struct f2fs_sb_info *);
 int convert_encrypted_name(unsigned char *, u32, unsigned char *, int);
diff --git a/fsck/main.c b/fsck/main.c
index d3f0c0d..e61bb91 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -616,6 +616,8 @@ static void do_fsck(struct f2fs_sb_info *sbi)
 		c.fix_on = 1;
 	}
 
+	fsck_chk_checkpoint(sbi);
+
 	fsck_chk_quota_node(sbi);
 
 	/* Traverse all block recursively from root inode */
diff --git a/fsck/mount.c b/fsck/mount.c
index da1d4c9..843742e 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -769,15 +769,33 @@ int init_sb_info(struct f2fs_sb_info *sbi)
 	return 0;
 }
 
+static int verify_checksum_chksum(struct f2fs_checkpoint *cp)
+{
+	unsigned int chksum_offset = get_cp(checksum_offset);
+	unsigned int crc, cal_crc;
+
+	if (chksum_offset < CP_MIN_CHKSUM_OFFSET ||
+			chksum_offset > CP_CHKSUM_OFFSET) {
+		MSG(0, "\tInvalid CP CRC offset: %u\n", chksum_offset);
+		return -1;
+	}
+
+	crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + chksum_offset));
+	cal_crc = f2fs_checkpoint_chksum(cp);
+	if (cal_crc != crc) {
+		MSG(0, "\tInvalid CP CRC: offset:%u, crc:0x%x, calc:0x%x\n",
+			chksum_offset, crc, cal_crc);
+		return -1;
+	}
+	return 0;
+}
+
 void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
 				unsigned long long *version)
 {
 	void *cp_page_1, *cp_page_2;
 	struct f2fs_checkpoint *cp;
-	unsigned long blk_size = sbi->blocksize;
 	unsigned long long cur_version = 0, pre_version = 0;
-	unsigned int crc = 0;
-	size_t crc_offset;
 
 	/* Read the 1st cp block in this CP pack */
 	cp_page_1 = malloc(PAGE_SIZE);
@@ -787,12 +805,7 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
 		goto invalid_cp1;
 
 	cp = (struct f2fs_checkpoint *)cp_page_1;
-	crc_offset = get_cp(checksum_offset);
-	if (crc_offset > (blk_size - sizeof(__le32)))
-		goto invalid_cp1;
-
-	crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + crc_offset));
-	if (f2fs_crc_valid(crc, cp, crc_offset))
+	if (verify_checksum_chksum(cp))
 		goto invalid_cp1;
 
 	if (get_cp(cp_pack_total_block_count) > sbi->blocks_per_seg)
@@ -810,12 +823,7 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
 		goto invalid_cp2;
 
 	cp = (struct f2fs_checkpoint *)cp_page_2;
-	crc_offset = get_cp(checksum_offset);
-	if (crc_offset > (blk_size - sizeof(__le32)))
-		goto invalid_cp2;
-
-	crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + crc_offset));
-	if (f2fs_crc_valid(crc, cp, crc_offset))
+	if (verify_checksum_chksum(cp))
 		goto invalid_cp2;
 
 	cur_version = get_cp(checkpoint_ver);
@@ -2365,6 +2373,10 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
 		flags |= CP_TRIMMED_FLAG;
 	if (is_set_ckpt_flags(cp, CP_DISABLED_FLAG))
 		flags |= CP_DISABLED_FLAG;
+	if (is_set_ckpt_flags(cp, CP_LARGE_NAT_BITMAP_FLAG)) {
+		flags |= CP_LARGE_NAT_BITMAP_FLAG;
+		set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
+	}
 
 	set_cp(free_segment_count, get_free_segments(sbi));
 	set_cp(valid_block_count, sbi->total_valid_block_count);
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 1bd935c..89b8a0c 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -692,10 +692,15 @@ struct f2fs_checkpoint {
 	unsigned char sit_nat_version_bitmap[1];
 } __attribute__((packed));
 
+#define CP_BITMAP_OFFSET	\
+	(offsetof(struct f2fs_checkpoint, sit_nat_version_bitmap))
+#define CP_MIN_CHKSUM_OFFSET	CP_BITMAP_OFFSET
+
+#define MIN_NAT_BITMAP_SIZE	64
 #define MAX_SIT_BITMAP_SIZE_IN_CKPT    \
-	(CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1 - 64)
+	(CP_CHKSUM_OFFSET - CP_BITMAP_OFFSET - MIN_NAT_BITMAP_SIZE)
 #define MAX_BITMAP_SIZE_IN_CKPT	\
-	(CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1)
+	(CP_CHKSUM_OFFSET - CP_BITMAP_OFFSET)
 
 /*
  * For orphan inode management
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index a534293..eabffce 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -706,7 +706,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, CP_CHKSUM_OFFSET);
+	if (c.large_nat_bitmap)
+		set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
+	else
+		set_cp(checksum_offset, CP_CHKSUM_OFFSET);
 
 	crc = f2fs_checkpoint_chksum(cp);
 	*((__le32 *)((unsigned char *)cp + get_cp(checksum_offset))) =
-- 
2.18.0.rc1


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

* Re: [f2fs-dev] [PATCH 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature
  2019-04-22  9:34 ` [PATCH 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature Chao Yu
@ 2019-04-24 12:00   ` Ju Hyung Park
  2019-04-26  8:44     ` Chao Yu
  0 siblings, 1 reply; 4+ messages in thread
From: Ju Hyung Park @ 2019-04-24 12:00 UTC (permalink / raw)
  To: Chao Yu, Jaegeuk Kim; +Cc: linux-f2fs-devel, Chao Yu, linux-kernel

Hi Chao and Jaegeuk,

I'm currently unavailable to recompile the kernel and use the new
layout scheme but I ran the new fsck and attempted to mount it on the
older kernel.

The used fsck-tools is at f50234c248532528b217efff5861b29fe6ec1b36
("f2fs-tools: Fix device zoned model detection").
fsck still behaves weird, I believe.

The new logs are at
http://arter97.com/f2fs/f50234c248532528b217efff5861b29fe6ec1b36

The first run spits out 600MB worth of log
and the second run spits out 60MB worth of log
and the third run fails to fix "other corrupted bugs".

Runs after the third prints out the same log.

When I try to mount the image after third run on the older kernel, it fails:
[ 1710.824598] F2FS-fs (loop1): invalid crc value
[ 1710.855240] F2FS-fs (loop1): SIT is corrupted node# 2174236 vs 2517451
[ 1710.855243] F2FS-fs (loop1): Failed to initialize F2FS segment manager

I'll report back after I compile and run the kernel with the new
layout scheme, but I don't feel good about this after fsck printing
that much logs, and the fact that the 2nd run still spits out a lot of
logs.

Thanks.

On Mon, Apr 22, 2019 at 6:35 PM Chao Yu <yuchao0@huawei.com> wrote:
>
> For large_nat_bitmap feature, there is a design flaw:
>
> Previous:
>
> struct f2fs_checkpoint layout:
> +--------------------------+  0x0000
> | checkpoint_ver           |
> | ......                   |
> | checksum_offset          |------+
> | ......                   |      |
> | sit_nat_version_bitmap[] |<-----|-------+
> | ......                   |      |       |
> | checksum_value           |<-----+       |
> +--------------------------+  0x1000      |
> |                          |      nat_bitmap + sit_bitmap
> | payload blocks           |              |
> |                          |              |
> +--------------------------|<-------------+
>
> Obviously, if nat_bitmap size + sit_bitmap size is larger than
> MAX_BITMAP_SIZE_IN_CKPT, nat_bitmap or sit_bitmap may overlap
> checkpoint checksum's position, once checkpoint() is triggered
> from kernel, nat or sit bitmap will be damaged by checksum field.
>
> In order to fix this, let's relocate checksum_value's position
> to the head of sit_nat_version_bitmap as below, then nat/sit
> bitmap and chksum value update will become safe.
>
> After:
>
> struct f2fs_checkpoint layout:
> +--------------------------+  0x0000
> | checkpoint_ver           |
> | ......                   |
> | checksum_offset          |------+
> | ......                   |      |
> | sit_nat_version_bitmap[] |<-----+
> | ......                   |<-------------+
> |                          |              |
> +--------------------------+  0x1000      |
> |                          |      nat_bitmap + sit_bitmap
> | payload blocks           |              |
> |                          |              |
> +--------------------------|<-------------+
>
> Related report and discussion:
>
> https://sourceforge.net/p/linux-f2fs/mailman/message/36642346/
>
> In addition, during writing checkpoint, if large_nat_bitmap feature is
> enabled, we need to set CP_LARGE_NAT_BITMAP_FLAG flag in checkpoint.
>
> Reported-by: Park Ju Hyung <qkrwngud825@gmail.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fsck/f2fs.h        |  6 +++++-
>  fsck/fsck.c        | 16 ++++++++++++++++
>  fsck/fsck.h        |  1 +
>  fsck/main.c        |  2 ++
>  fsck/mount.c       | 42 +++++++++++++++++++++++++++---------------
>  include/f2fs_fs.h  |  9 +++++++--
>  mkfs/f2fs_format.c |  5 ++++-
>  7 files changed, 62 insertions(+), 19 deletions(-)
>
> diff --git a/fsck/f2fs.h b/fsck/f2fs.h
> index 93f01e5..6a6c4a5 100644
> --- a/fsck/f2fs.h
> +++ b/fsck/f2fs.h
> @@ -272,7 +272,11 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
>         if (is_set_ckpt_flags(ckpt, CP_LARGE_NAT_BITMAP_FLAG)) {
>                 offset = (flag == SIT_BITMAP) ?
>                         le32_to_cpu(ckpt->nat_ver_bitmap_bytesize) : 0;
> -               return &ckpt->sit_nat_version_bitmap + offset;
> +               /*
> +                * if large_nat_bitmap feature is enabled, leave checksum
> +                * protection for all nat/sit bitmaps.
> +                */
> +               return &ckpt->sit_nat_version_bitmap + offset + sizeof(__le32);
>         }
>
>         if (le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload) > 0) {
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index 7ecdee1..8d7deb5 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -1917,6 +1917,18 @@ int fsck_chk_meta(struct f2fs_sb_info *sbi)
>         return 0;
>  }
>
> +void fsck_chk_checkpoint(struct f2fs_sb_info *sbi)
> +{
> +       struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
> +
> +       if (get_cp(ckpt_flags) & CP_LARGE_NAT_BITMAP_FLAG) {
> +               if (get_cp(checksum_offset) != CP_MIN_CHKSUM_OFFSET) {
> +                       ASSERT_MSG("Deprecated layout of large_nat_bitmap, "
> +                               "chksum_offset:%u", get_cp(checksum_offset));
> +               }
> +       }
> +}
> +
>  void fsck_init(struct f2fs_sb_info *sbi)
>  {
>         struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
> @@ -2038,6 +2050,10 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi)
>                 flags |= CP_TRIMMED_FLAG;
>         if (is_set_ckpt_flags(cp, CP_DISABLED_FLAG))
>                 flags |= CP_DISABLED_FLAG;
> +       if (is_set_ckpt_flags(cp, CP_LARGE_NAT_BITMAP_FLAG)) {
> +               flags |= CP_LARGE_NAT_BITMAP_FLAG;
> +               set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
> +       }
>
>         if (flags & CP_UMOUNT_FLAG)
>                 cp_blocks = 8;
> diff --git a/fsck/fsck.h b/fsck/fsck.h
> index 376ced7..dd831de 100644
> --- a/fsck/fsck.h
> +++ b/fsck/fsck.h
> @@ -154,6 +154,7 @@ extern int fsck_chk_dentry_blk(struct f2fs_sb_info *, u32, struct child_info *,
>                 int, int);
>  int fsck_chk_inline_dentries(struct f2fs_sb_info *, struct f2fs_node *,
>                 struct child_info *);
> +void fsck_chk_checkpoint(struct f2fs_sb_info *sbi);
>  int fsck_chk_meta(struct f2fs_sb_info *sbi);
>  int fsck_chk_curseg_info(struct f2fs_sb_info *);
>  int convert_encrypted_name(unsigned char *, u32, unsigned char *, int);
> diff --git a/fsck/main.c b/fsck/main.c
> index d3f0c0d..e61bb91 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -616,6 +616,8 @@ static void do_fsck(struct f2fs_sb_info *sbi)
>                 c.fix_on = 1;
>         }
>
> +       fsck_chk_checkpoint(sbi);
> +
>         fsck_chk_quota_node(sbi);
>
>         /* Traverse all block recursively from root inode */
> diff --git a/fsck/mount.c b/fsck/mount.c
> index da1d4c9..843742e 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -769,15 +769,33 @@ int init_sb_info(struct f2fs_sb_info *sbi)
>         return 0;
>  }
>
> +static int verify_checksum_chksum(struct f2fs_checkpoint *cp)
> +{
> +       unsigned int chksum_offset = get_cp(checksum_offset);
> +       unsigned int crc, cal_crc;
> +
> +       if (chksum_offset < CP_MIN_CHKSUM_OFFSET ||
> +                       chksum_offset > CP_CHKSUM_OFFSET) {
> +               MSG(0, "\tInvalid CP CRC offset: %u\n", chksum_offset);
> +               return -1;
> +       }
> +
> +       crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + chksum_offset));
> +       cal_crc = f2fs_checkpoint_chksum(cp);
> +       if (cal_crc != crc) {
> +               MSG(0, "\tInvalid CP CRC: offset:%u, crc:0x%x, calc:0x%x\n",
> +                       chksum_offset, crc, cal_crc);
> +               return -1;
> +       }
> +       return 0;
> +}
> +
>  void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
>                                 unsigned long long *version)
>  {
>         void *cp_page_1, *cp_page_2;
>         struct f2fs_checkpoint *cp;
> -       unsigned long blk_size = sbi->blocksize;
>         unsigned long long cur_version = 0, pre_version = 0;
> -       unsigned int crc = 0;
> -       size_t crc_offset;
>
>         /* Read the 1st cp block in this CP pack */
>         cp_page_1 = malloc(PAGE_SIZE);
> @@ -787,12 +805,7 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
>                 goto invalid_cp1;
>
>         cp = (struct f2fs_checkpoint *)cp_page_1;
> -       crc_offset = get_cp(checksum_offset);
> -       if (crc_offset > (blk_size - sizeof(__le32)))
> -               goto invalid_cp1;
> -
> -       crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + crc_offset));
> -       if (f2fs_crc_valid(crc, cp, crc_offset))
> +       if (verify_checksum_chksum(cp))
>                 goto invalid_cp1;
>
>         if (get_cp(cp_pack_total_block_count) > sbi->blocks_per_seg)
> @@ -810,12 +823,7 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
>                 goto invalid_cp2;
>
>         cp = (struct f2fs_checkpoint *)cp_page_2;
> -       crc_offset = get_cp(checksum_offset);
> -       if (crc_offset > (blk_size - sizeof(__le32)))
> -               goto invalid_cp2;
> -
> -       crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + crc_offset));
> -       if (f2fs_crc_valid(crc, cp, crc_offset))
> +       if (verify_checksum_chksum(cp))
>                 goto invalid_cp2;
>
>         cur_version = get_cp(checkpoint_ver);
> @@ -2365,6 +2373,10 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
>                 flags |= CP_TRIMMED_FLAG;
>         if (is_set_ckpt_flags(cp, CP_DISABLED_FLAG))
>                 flags |= CP_DISABLED_FLAG;
> +       if (is_set_ckpt_flags(cp, CP_LARGE_NAT_BITMAP_FLAG)) {
> +               flags |= CP_LARGE_NAT_BITMAP_FLAG;
> +               set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
> +       }
>
>         set_cp(free_segment_count, get_free_segments(sbi));
>         set_cp(valid_block_count, sbi->total_valid_block_count);
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index 1bd935c..89b8a0c 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -692,10 +692,15 @@ struct f2fs_checkpoint {
>         unsigned char sit_nat_version_bitmap[1];
>  } __attribute__((packed));
>
> +#define CP_BITMAP_OFFSET       \
> +       (offsetof(struct f2fs_checkpoint, sit_nat_version_bitmap))
> +#define CP_MIN_CHKSUM_OFFSET   CP_BITMAP_OFFSET
> +
> +#define MIN_NAT_BITMAP_SIZE    64
>  #define MAX_SIT_BITMAP_SIZE_IN_CKPT    \
> -       (CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1 - 64)
> +       (CP_CHKSUM_OFFSET - CP_BITMAP_OFFSET - MIN_NAT_BITMAP_SIZE)
>  #define MAX_BITMAP_SIZE_IN_CKPT        \
> -       (CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1)
> +       (CP_CHKSUM_OFFSET - CP_BITMAP_OFFSET)
>
>  /*
>   * For orphan inode management
> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
> index a534293..eabffce 100644
> --- a/mkfs/f2fs_format.c
> +++ b/mkfs/f2fs_format.c
> @@ -706,7 +706,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, CP_CHKSUM_OFFSET);
> +       if (c.large_nat_bitmap)
> +               set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
> +       else
> +               set_cp(checksum_offset, CP_CHKSUM_OFFSET);
>
>         crc = f2fs_checkpoint_chksum(cp);
>         *((__le32 *)((unsigned char *)cp + get_cp(checksum_offset))) =
> --
> 2.18.0.rc1
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature
  2019-04-24 12:00   ` [f2fs-dev] " Ju Hyung Park
@ 2019-04-26  8:44     ` Chao Yu
  0 siblings, 0 replies; 4+ messages in thread
From: Chao Yu @ 2019-04-26  8:44 UTC (permalink / raw)
  To: Ju Hyung Park, Jaegeuk Kim; +Cc: linux-f2fs-devel, Chao Yu, linux-kernel

Hi Ju Hyung,

On 2019/4/24 20:00, Ju Hyung Park wrote:
> Hi Chao and Jaegeuk,
> 
> I'm currently unavailable to recompile the kernel and use the new
> layout scheme but I ran the new fsck and attempted to mount it on the
> older kernel.
> 
> The used fsck-tools is at f50234c248532528b217efff5861b29fe6ec1b36
> ("f2fs-tools: Fix device zoned model detection").
> fsck still behaves weird, I believe.
> 
> The new logs are at
> http://arter97.com/f2fs/f50234c248532528b217efff5861b29fe6ec1b36
> 
> The first run spits out 600MB worth of log
> and the second run spits out 60MB worth of log
> and the third run fails to fix "other corrupted bugs".

I found one problem in old fixing patch, could you please try new one?

Thanks,

> 
> Runs after the third prints out the same log.
> 
> When I try to mount the image after third run on the older kernel, it fails:
> [ 1710.824598] F2FS-fs (loop1): invalid crc value
> [ 1710.855240] F2FS-fs (loop1): SIT is corrupted node# 2174236 vs 2517451
> [ 1710.855243] F2FS-fs (loop1): Failed to initialize F2FS segment manager
> 
> I'll report back after I compile and run the kernel with the new
> layout scheme, but I don't feel good about this after fsck printing
> that much logs, and the fact that the 2nd run still spits out a lot of
> logs.
> 
> Thanks.
> 
> On Mon, Apr 22, 2019 at 6:35 PM Chao Yu <yuchao0@huawei.com> wrote:
>>
>> For large_nat_bitmap feature, there is a design flaw:
>>
>> Previous:
>>
>> struct f2fs_checkpoint layout:
>> +--------------------------+  0x0000
>> | checkpoint_ver           |
>> | ......                   |
>> | checksum_offset          |------+
>> | ......                   |      |
>> | sit_nat_version_bitmap[] |<-----|-------+
>> | ......                   |      |       |
>> | checksum_value           |<-----+       |
>> +--------------------------+  0x1000      |
>> |                          |      nat_bitmap + sit_bitmap
>> | payload blocks           |              |
>> |                          |              |
>> +--------------------------|<-------------+
>>
>> Obviously, if nat_bitmap size + sit_bitmap size is larger than
>> MAX_BITMAP_SIZE_IN_CKPT, nat_bitmap or sit_bitmap may overlap
>> checkpoint checksum's position, once checkpoint() is triggered
>> from kernel, nat or sit bitmap will be damaged by checksum field.
>>
>> In order to fix this, let's relocate checksum_value's position
>> to the head of sit_nat_version_bitmap as below, then nat/sit
>> bitmap and chksum value update will become safe.
>>
>> After:
>>
>> struct f2fs_checkpoint layout:
>> +--------------------------+  0x0000
>> | checkpoint_ver           |
>> | ......                   |
>> | checksum_offset          |------+
>> | ......                   |      |
>> | sit_nat_version_bitmap[] |<-----+
>> | ......                   |<-------------+
>> |                          |              |
>> +--------------------------+  0x1000      |
>> |                          |      nat_bitmap + sit_bitmap
>> | payload blocks           |              |
>> |                          |              |
>> +--------------------------|<-------------+
>>
>> Related report and discussion:
>>
>> https://sourceforge.net/p/linux-f2fs/mailman/message/36642346/
>>
>> In addition, during writing checkpoint, if large_nat_bitmap feature is
>> enabled, we need to set CP_LARGE_NAT_BITMAP_FLAG flag in checkpoint.
>>
>> Reported-by: Park Ju Hyung <qkrwngud825@gmail.com>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fsck/f2fs.h        |  6 +++++-
>>  fsck/fsck.c        | 16 ++++++++++++++++
>>  fsck/fsck.h        |  1 +
>>  fsck/main.c        |  2 ++
>>  fsck/mount.c       | 42 +++++++++++++++++++++++++++---------------
>>  include/f2fs_fs.h  |  9 +++++++--
>>  mkfs/f2fs_format.c |  5 ++++-
>>  7 files changed, 62 insertions(+), 19 deletions(-)
>>
>> diff --git a/fsck/f2fs.h b/fsck/f2fs.h
>> index 93f01e5..6a6c4a5 100644
>> --- a/fsck/f2fs.h
>> +++ b/fsck/f2fs.h
>> @@ -272,7 +272,11 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
>>         if (is_set_ckpt_flags(ckpt, CP_LARGE_NAT_BITMAP_FLAG)) {
>>                 offset = (flag == SIT_BITMAP) ?
>>                         le32_to_cpu(ckpt->nat_ver_bitmap_bytesize) : 0;
>> -               return &ckpt->sit_nat_version_bitmap + offset;
>> +               /*
>> +                * if large_nat_bitmap feature is enabled, leave checksum
>> +                * protection for all nat/sit bitmaps.
>> +                */
>> +               return &ckpt->sit_nat_version_bitmap + offset + sizeof(__le32);
>>         }
>>
>>         if (le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload) > 0) {
>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>> index 7ecdee1..8d7deb5 100644
>> --- a/fsck/fsck.c
>> +++ b/fsck/fsck.c
>> @@ -1917,6 +1917,18 @@ int fsck_chk_meta(struct f2fs_sb_info *sbi)
>>         return 0;
>>  }
>>
>> +void fsck_chk_checkpoint(struct f2fs_sb_info *sbi)
>> +{
>> +       struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
>> +
>> +       if (get_cp(ckpt_flags) & CP_LARGE_NAT_BITMAP_FLAG) {
>> +               if (get_cp(checksum_offset) != CP_MIN_CHKSUM_OFFSET) {
>> +                       ASSERT_MSG("Deprecated layout of large_nat_bitmap, "
>> +                               "chksum_offset:%u", get_cp(checksum_offset));
>> +               }
>> +       }
>> +}
>> +
>>  void fsck_init(struct f2fs_sb_info *sbi)
>>  {
>>         struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
>> @@ -2038,6 +2050,10 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi)
>>                 flags |= CP_TRIMMED_FLAG;
>>         if (is_set_ckpt_flags(cp, CP_DISABLED_FLAG))
>>                 flags |= CP_DISABLED_FLAG;
>> +       if (is_set_ckpt_flags(cp, CP_LARGE_NAT_BITMAP_FLAG)) {
>> +               flags |= CP_LARGE_NAT_BITMAP_FLAG;
>> +               set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
>> +       }
>>
>>         if (flags & CP_UMOUNT_FLAG)
>>                 cp_blocks = 8;
>> diff --git a/fsck/fsck.h b/fsck/fsck.h
>> index 376ced7..dd831de 100644
>> --- a/fsck/fsck.h
>> +++ b/fsck/fsck.h
>> @@ -154,6 +154,7 @@ extern int fsck_chk_dentry_blk(struct f2fs_sb_info *, u32, struct child_info *,
>>                 int, int);
>>  int fsck_chk_inline_dentries(struct f2fs_sb_info *, struct f2fs_node *,
>>                 struct child_info *);
>> +void fsck_chk_checkpoint(struct f2fs_sb_info *sbi);
>>  int fsck_chk_meta(struct f2fs_sb_info *sbi);
>>  int fsck_chk_curseg_info(struct f2fs_sb_info *);
>>  int convert_encrypted_name(unsigned char *, u32, unsigned char *, int);
>> diff --git a/fsck/main.c b/fsck/main.c
>> index d3f0c0d..e61bb91 100644
>> --- a/fsck/main.c
>> +++ b/fsck/main.c
>> @@ -616,6 +616,8 @@ static void do_fsck(struct f2fs_sb_info *sbi)
>>                 c.fix_on = 1;
>>         }
>>
>> +       fsck_chk_checkpoint(sbi);
>> +
>>         fsck_chk_quota_node(sbi);
>>
>>         /* Traverse all block recursively from root inode */
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index da1d4c9..843742e 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -769,15 +769,33 @@ int init_sb_info(struct f2fs_sb_info *sbi)
>>         return 0;
>>  }
>>
>> +static int verify_checksum_chksum(struct f2fs_checkpoint *cp)
>> +{
>> +       unsigned int chksum_offset = get_cp(checksum_offset);
>> +       unsigned int crc, cal_crc;
>> +
>> +       if (chksum_offset < CP_MIN_CHKSUM_OFFSET ||
>> +                       chksum_offset > CP_CHKSUM_OFFSET) {
>> +               MSG(0, "\tInvalid CP CRC offset: %u\n", chksum_offset);
>> +               return -1;
>> +       }
>> +
>> +       crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + chksum_offset));
>> +       cal_crc = f2fs_checkpoint_chksum(cp);
>> +       if (cal_crc != crc) {
>> +               MSG(0, "\tInvalid CP CRC: offset:%u, crc:0x%x, calc:0x%x\n",
>> +                       chksum_offset, crc, cal_crc);
>> +               return -1;
>> +       }
>> +       return 0;
>> +}
>> +
>>  void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
>>                                 unsigned long long *version)
>>  {
>>         void *cp_page_1, *cp_page_2;
>>         struct f2fs_checkpoint *cp;
>> -       unsigned long blk_size = sbi->blocksize;
>>         unsigned long long cur_version = 0, pre_version = 0;
>> -       unsigned int crc = 0;
>> -       size_t crc_offset;
>>
>>         /* Read the 1st cp block in this CP pack */
>>         cp_page_1 = malloc(PAGE_SIZE);
>> @@ -787,12 +805,7 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
>>                 goto invalid_cp1;
>>
>>         cp = (struct f2fs_checkpoint *)cp_page_1;
>> -       crc_offset = get_cp(checksum_offset);
>> -       if (crc_offset > (blk_size - sizeof(__le32)))
>> -               goto invalid_cp1;
>> -
>> -       crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + crc_offset));
>> -       if (f2fs_crc_valid(crc, cp, crc_offset))
>> +       if (verify_checksum_chksum(cp))
>>                 goto invalid_cp1;
>>
>>         if (get_cp(cp_pack_total_block_count) > sbi->blocks_per_seg)
>> @@ -810,12 +823,7 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr,
>>                 goto invalid_cp2;
>>
>>         cp = (struct f2fs_checkpoint *)cp_page_2;
>> -       crc_offset = get_cp(checksum_offset);
>> -       if (crc_offset > (blk_size - sizeof(__le32)))
>> -               goto invalid_cp2;
>> -
>> -       crc = le32_to_cpu(*(__le32 *)((unsigned char *)cp + crc_offset));
>> -       if (f2fs_crc_valid(crc, cp, crc_offset))
>> +       if (verify_checksum_chksum(cp))
>>                 goto invalid_cp2;
>>
>>         cur_version = get_cp(checkpoint_ver);
>> @@ -2365,6 +2373,10 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
>>                 flags |= CP_TRIMMED_FLAG;
>>         if (is_set_ckpt_flags(cp, CP_DISABLED_FLAG))
>>                 flags |= CP_DISABLED_FLAG;
>> +       if (is_set_ckpt_flags(cp, CP_LARGE_NAT_BITMAP_FLAG)) {
>> +               flags |= CP_LARGE_NAT_BITMAP_FLAG;
>> +               set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
>> +       }
>>
>>         set_cp(free_segment_count, get_free_segments(sbi));
>>         set_cp(valid_block_count, sbi->total_valid_block_count);
>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>> index 1bd935c..89b8a0c 100644
>> --- a/include/f2fs_fs.h
>> +++ b/include/f2fs_fs.h
>> @@ -692,10 +692,15 @@ struct f2fs_checkpoint {
>>         unsigned char sit_nat_version_bitmap[1];
>>  } __attribute__((packed));
>>
>> +#define CP_BITMAP_OFFSET       \
>> +       (offsetof(struct f2fs_checkpoint, sit_nat_version_bitmap))
>> +#define CP_MIN_CHKSUM_OFFSET   CP_BITMAP_OFFSET
>> +
>> +#define MIN_NAT_BITMAP_SIZE    64
>>  #define MAX_SIT_BITMAP_SIZE_IN_CKPT    \
>> -       (CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1 - 64)
>> +       (CP_CHKSUM_OFFSET - CP_BITMAP_OFFSET - MIN_NAT_BITMAP_SIZE)
>>  #define MAX_BITMAP_SIZE_IN_CKPT        \
>> -       (CP_CHKSUM_OFFSET - sizeof(struct f2fs_checkpoint) + 1)
>> +       (CP_CHKSUM_OFFSET - CP_BITMAP_OFFSET)
>>
>>  /*
>>   * For orphan inode management
>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
>> index a534293..eabffce 100644
>> --- a/mkfs/f2fs_format.c
>> +++ b/mkfs/f2fs_format.c
>> @@ -706,7 +706,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, CP_CHKSUM_OFFSET);
>> +       if (c.large_nat_bitmap)
>> +               set_cp(checksum_offset, CP_MIN_CHKSUM_OFFSET);
>> +       else
>> +               set_cp(checksum_offset, CP_CHKSUM_OFFSET);
>>
>>         crc = f2fs_checkpoint_chksum(cp);
>>         *((__le32 *)((unsigned char *)cp + get_cp(checksum_offset))) =
>> --
>> 2.18.0.rc1
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
> 

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

end of thread, other threads:[~2019-04-26  8:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-22  9:34 [PATCH 1/2] f2fs-tools: allow unfixed f2fs_checkpoint.checksum_offset Chao Yu
2019-04-22  9:34 ` [PATCH 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature Chao Yu
2019-04-24 12:00   ` [f2fs-dev] " Ju Hyung Park
2019-04-26  8:44     ` 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).