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?
>
> 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
