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.

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

Reply via email to