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 <zhengjunl...@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 >>>>>>> >>>>>>> . >>>>>>> >>>>>> >>>>> >>>>> . >>>>> >>>> >>> >>> . >>> >> > > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel