On 2018/8/8 20:24, Chao Yu wrote:
> On 2018/8/8 20:21, Junling Zheng wrote:
>> On 2018/8/8 20:03, Chao Yu wrote:
>>> On 2018/8/8 18:27, Junling Zheng wrote:
>>>> Now we support crc32 checksum for superblock.
>>>>
>>>> Signed-off-by: Junling Zheng <zhengjunl...@huawei.com>
>>>> ---
>>>> v2 -> v3:
>>>>  - add sysfs entry for superblock checksum.
>>>>  - move the crc checking to the beginning of sanity_check_raw_super().
>>>>  - fix the NULL pointer dereference as sb->s_fs_info hasn't be set yet in
>>>>    sanity_check_raw_super().
>>>> v1 -> v2:
>>>>  - fix to switch endian of crc.
>>>>  fs/f2fs/f2fs.h          |  2 ++
>>>>  fs/f2fs/super.c         | 29 +++++++++++++++++++++++++++++
>>>>  fs/f2fs/sysfs.c         |  7 +++++++
>>>>  include/linux/f2fs_fs.h |  3 ++-
>>>>  4 files changed, 40 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index 4525f4f82af0..d50d6efda96b 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -147,6 +147,7 @@ struct f2fs_mount_info {
>>>>  #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 F2FS_HAS_FEATURE(sb, mask)                                        
>>>> \
>>>>    ((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
>>>> @@ -3376,6 +3377,7 @@ F2FS_FEATURE_FUNCS(flexible_inline_xattr, 
>>>> FLEXIBLE_INLINE_XATTR);
>>>>  F2FS_FEATURE_FUNCS(quota_ino, QUOTA_INO);
>>>>  F2FS_FEATURE_FUNCS(inode_crtime, INODE_CRTIME);
>>>>  F2FS_FEATURE_FUNCS(lost_found, LOST_FOUND);
>>>> +F2FS_FEATURE_FUNCS(sb_chksum, SB_CHKSUM);
>>>>  
>>>>  #ifdef CONFIG_BLK_DEV_ZONED
>>>>  static inline int get_blkz_type(struct f2fs_sb_info *sbi,
>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>> index bd57be470e23..c3dbafa31613 100644
>>>> --- a/fs/f2fs/super.c
>>>> +++ b/fs/f2fs/super.c
>>>> @@ -2149,6 +2149,26 @@ static int sanity_check_raw_super(struct 
>>>> f2fs_sb_info *sbi,
>>>>                                    (bh->b_data + F2FS_SUPER_OFFSET);
>>>>    struct super_block *sb = sbi->sb;
>>>>    unsigned int blocksize;
>>>> +  size_t crc_offset = 0;
>>>> +  __u32 crc = 0;
>>>> +
>>>> +  /* Check checksum_offset and crc in superblock */
>>>> +  if (le32_to_cpu(raw_super->feature) & F2FS_FEATURE_SB_CHKSUM) {
>>>> +          crc_offset = le32_to_cpu(raw_super->checksum_offset);
>>>> +          if (crc_offset !=
>>>> +                  offsetof(struct f2fs_super_block, crc)) {
>>>> +                  f2fs_msg(sb, KERN_INFO,
>>>> +                          "Invalid SB checksum offset: %lu",
>>>
>>> warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but 
>>> argument
>>> 4 has type ‘size_t’ [-Wformat=]
>>>      crc_offset);
>>>
>>
>> :( sorry, I'll send a v4 patch.
>>
>>> Otherwise, it looks good to me, you can add in next version:
>>>
>>> Reviewed-by: Chao Yu <yuch...@huawei.com>
>>>
>>> Thanks,
>>>
>>>> +                          crc_offset);
>>>> +                  return 1;
>>>> +          }
>>>> +          crc = le32_to_cpu(raw_super->crc);
>>>> +          if (!f2fs_crc_valid(sbi, crc, raw_super, crc_offset)) {
>>>> +                  f2fs_msg(sb, KERN_INFO,
>>>> +                          "Invalid SB checksum value: %u", crc);
>>>> +                  return 1;
>>>> +          }
>>>> +  }
>>>>  
>>>>    if (F2FS_SUPER_MAGIC != le32_to_cpu(raw_super->magic)) {
>>>>            f2fs_msg(sb, KERN_INFO,
>>>> @@ -2568,6 +2588,7 @@ static int read_raw_super_block(struct f2fs_sb_info 
>>>> *sbi,
>>>>  int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
>>>>  {
>>>>    struct buffer_head *bh;
>>>> +  __u32 crc = 0;
>>>>    int err;
>>>>  
>>>>    if ((recover && f2fs_readonly(sbi->sb)) ||
>>>> @@ -2576,6 +2597,14 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, 
>>>> bool recover)
>>>>            return -EROFS;
>>>>    }
>>>>  
>>>> +  /* we should update superblock crc here */
>>>> +  if (!recover &&
>>>> +          F2FS_HAS_FEATURE(sbi->sb, F2FS_FEATURE_SB_CHKSUM)) {
> 
> Can we change to use f2fs_sb_has_sb_chksum()?
> 

Sure, that will be better :)

> Thanks,
> 
>>>> +          crc = f2fs_crc32(sbi, F2FS_RAW_SUPER(sbi),
>>>> +                          offsetof(struct f2fs_super_block, crc));
>>>> +          F2FS_RAW_SUPER(sbi)->crc = cpu_to_le32(crc);
>>>> +  }
>>>> +
>>>>    /* write back-up superblock first */
>>>>    bh = sb_bread(sbi->sb, sbi->valid_super_block ? 0 : 1);
>>>>    if (!bh)
>>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>>> index cd2e030e47b8..c86d91be6c48 100644
>>>> --- a/fs/f2fs/sysfs.c
>>>> +++ b/fs/f2fs/sysfs.c
>>>> @@ -120,6 +120,9 @@ static ssize_t features_show(struct f2fs_attr *a,
>>>>    if (f2fs_sb_has_lost_found(sb))
>>>>            len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
>>>>                            len ? ", " : "", "lost_found");
>>>> +  if (f2fs_sb_has_sb_chksum(sb))
>>>> +          len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
>>>> +                          len ? ", " : "", "sb_checksum");
>>>>    len += snprintf(buf + len, PAGE_SIZE - len, "\n");
>>>>    return len;
>>>>  }
>>>> @@ -337,6 +340,7 @@ enum feat_id {
>>>>    FEAT_QUOTA_INO,
>>>>    FEAT_INODE_CRTIME,
>>>>    FEAT_LOST_FOUND,
>>>> +  FEAT_SB_CHECKSUM,
>>>>  };
>>>>  
>>>>  static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>>> @@ -353,6 +357,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>>>>    case FEAT_QUOTA_INO:
>>>>    case FEAT_INODE_CRTIME:
>>>>    case FEAT_LOST_FOUND:
>>>> +  case FEAT_SB_CHECKSUM:
>>>>            return snprintf(buf, PAGE_SIZE, "supported\n");
>>>>    }
>>>>    return 0;
>>>> @@ -433,6 +438,7 @@ F2FS_FEATURE_RO_ATTR(flexible_inline_xattr, 
>>>> FEAT_FLEXIBLE_INLINE_XATTR);
>>>>  F2FS_FEATURE_RO_ATTR(quota_ino, FEAT_QUOTA_INO);
>>>>  F2FS_FEATURE_RO_ATTR(inode_crtime, FEAT_INODE_CRTIME);
>>>>  F2FS_FEATURE_RO_ATTR(lost_found, FEAT_LOST_FOUND);
>>>> +F2FS_FEATURE_RO_ATTR(sb_checksum, FEAT_SB_CHECKSUM);
>>>>  
>>>>  #define ATTR_LIST(name) (&f2fs_attr_##name.attr)
>>>>  static struct attribute *f2fs_attrs[] = {
>>>> @@ -489,6 +495,7 @@ static struct attribute *f2fs_feat_attrs[] = {
>>>>    ATTR_LIST(quota_ino),
>>>>    ATTR_LIST(inode_crtime),
>>>>    ATTR_LIST(lost_found),
>>>> +  ATTR_LIST(sb_checksum),
>>>>    NULL,
>>>>  };
>>>>  
>>>> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
>>>> index f70f8ac9c4f4..aa4b586569c1 100644
>>>> --- a/include/linux/f2fs_fs.h
>>>> +++ b/include/linux/f2fs_fs.h
>>>> @@ -112,7 +112,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 */
>>>>  } __packed;
>>>>  
>>>>  /*
>>>>
>>>
>>> .
>>>
>>
>>
> 
> .
> 



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to