On 01/27, Chao Yu wrote:
> This patch adds to support {d,id,did,x}node checksum in kernel side.
> 
> Signed-off-by: Chao Yu <yuch...@huawei.com>
> ---
>  fs/f2fs/f2fs.h          | 15 +++++++-
>  fs/f2fs/inode.c         | 98 
> +++++++++++++++++++++++++++++++------------------
>  fs/f2fs/node.c          |  2 +-
>  fs/f2fs/segment.c       |  2 +-
>  fs/f2fs/sysfs.c         |  7 ++++
>  include/linux/f2fs_fs.h | 10 ++++-
>  6 files changed, 93 insertions(+), 41 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 6f5e41657c62..ad48d389fea4 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -128,6 +128,7 @@ struct f2fs_mount_info {
>  #define F2FS_FEATURE_QUOTA_INO               0x0080
>  #define F2FS_FEATURE_INODE_CRTIME    0x0100
>  #define F2FS_FEATURE_EXTENDED_NODE   0x0200
> +#define F2FS_FEATURE_NODE_CHECKSUM   0x0400
>  
>  #define F2FS_HAS_FEATURE(sb, mask)                                   \
>       ((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
> @@ -2570,6 +2571,11 @@ static inline int get_inline_xattr_addrs(struct inode 
> *inode)
>               sizeof((f2fs_inode)->field))                    \
>               <= (F2FS_OLD_ATTRIBUTE_SIZE + extra_isize))     \
>  
> +#define F2FS_FITS_IN_NODE(extra_nsize, field)                \
> +             ((offsetof(struct f2fs_node, footer) -  \
> +             offsetof(struct f2fs_node, field))      \
> +             <= (extra_nsize))                       \
> +
>  static inline void f2fs_reset_iostat(struct f2fs_sb_info *sbi)
>  {
>       int i;
> @@ -2616,8 +2622,8 @@ int f2fs_pin_file_control(struct inode *inode, bool 
> inc);
>   * inode.c
>   */
>  void f2fs_set_inode_flags(struct inode *inode);
> -bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi, struct page *page);
> -void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct page *page);
> +bool f2fs_node_chksum_verify(struct f2fs_sb_info *sbi, struct page *page);
> +void f2fs_node_chksum_set(struct f2fs_sb_info *sbi, struct page *page);
>  struct inode *f2fs_iget(struct super_block *sb, unsigned long ino);
>  struct inode *f2fs_iget_retry(struct super_block *sb, unsigned long ino);
>  int try_to_free_nats(struct f2fs_sb_info *sbi, int nr_shrink);
> @@ -3252,6 +3258,11 @@ static inline int f2fs_sb_has_extended_node(struct 
> super_block *sb)
>       return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_EXTENDED_NODE);
>  }
>  
> +static inline int f2fs_sb_has_node_checksum(struct super_block *sb)
> +{
> +     return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_NODE_CHECKSUM);
> +}
> +
>  #ifdef CONFIG_BLK_DEV_ZONED
>  static inline int get_blkz_type(struct f2fs_sb_info *sbi,
>                       struct block_device *bdev, block_t blkaddr)
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index f5b5606ca273..f879f0a65986 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -111,75 +111,103 @@ static void __recover_inline_status(struct inode 
> *inode, struct page *ipage)
>       return;
>  }
>  
> -static bool f2fs_enable_inode_chksum(struct f2fs_sb_info *sbi, struct page 
> *page)
> +static bool f2fs_enable_node_chksum(struct f2fs_sb_info *sbi, struct page 
> *page)
>  {
> -     struct f2fs_inode *ri = &F2FS_NODE(page)->i;
> -     int extra_isize = le32_to_cpu(ri->i_extra_isize);
> -
> -     if (!f2fs_sb_has_inode_chksum(sbi->sb))
> -             return false;
> -
> -     if (!RAW_IS_INODE(F2FS_NODE(page)) || !(ri->i_inline & F2FS_EXTRA_ATTR))
> -             return false;
> -
> -     if (!F2FS_FITS_IN_INODE(ri, extra_isize, i_inode_checksum))
> -             return false;
> +     if (IS_INODE(page)) {
> +             struct f2fs_inode *ri = &F2FS_NODE(page)->i;
> +             int extra_isize = le32_to_cpu(ri->i_extra_isize);
> +
> +             if (!(ri->i_inline & F2FS_EXTRA_ATTR))
> +                     return false;
> +             if (!f2fs_sb_has_inode_chksum(sbi->sb))
> +                     return false;
> +             if (!F2FS_FITS_IN_INODE(ri, extra_isize, i_inode_checksum))
> +                     return false;
> +     } else {
> +             if (!f2fs_sb_has_extended_node(sbi->sb))
> +                     return false;

What if we want to add more entries in addition to node_checksum? Do we have
to add a new feature flag at every time? How about adding a layout value instead
of extra_nsize? For example, layout #1 means node_checksum with extra_nsize=X?

> +             if (!f2fs_sb_has_node_checksum(sbi->sb))
> +                     return false;
> +             if (!F2FS_FITS_IN_NODE(sbi->extra_nsize, node_checksum))
> +                     return false;
> +     }
>  
>       return true;
>  }
>  
> -static __u32 f2fs_inode_chksum(struct f2fs_sb_info *sbi, struct page *page)
> +static __u32 f2fs_node_chksum(struct f2fs_sb_info *sbi, struct page *page)
>  {
>       struct f2fs_node *node = F2FS_NODE(page);
> -     struct f2fs_inode *ri = &node->i;
> -     __le32 ino = node->footer.ino;
> -     __le32 gen = ri->i_generation;
> -     __u32 chksum, chksum_seed;
> -     __u32 dummy_cs = 0;
> -     unsigned int offset = offsetof(struct f2fs_inode, i_inode_checksum);
> -     unsigned int cs_size = sizeof(dummy_cs);
> +     __le32 append, ino = node->footer.ino;
> +     __u32 chksum, chksum_seed, dummy_cs = 0;
> +     unsigned int offset, cs_size = sizeof(dummy_cs);
> +     bool is_inode = RAW_IS_INODE(node);
> +
> +     if (is_inode) {
> +             append = node->i.i_generation;
> +             offset = offsetof(struct f2fs_inode, i_inode_checksum);
> +     } else {
> +             append = node->footer.nid;
> +             offset = offsetof(struct f2fs_node, node_checksum);
> +     }
>  
>       chksum = f2fs_chksum(sbi, sbi->s_chksum_seed, (__u8 *)&ino,
>                                                       sizeof(ino));
> -     chksum_seed = f2fs_chksum(sbi, chksum, (__u8 *)&gen, sizeof(gen));
> +     chksum_seed = f2fs_chksum(sbi, chksum, (__u8 *)&append, sizeof(append));
>  
> -     chksum = f2fs_chksum(sbi, chksum_seed, (__u8 *)ri, offset);
> +     chksum = f2fs_chksum(sbi, chksum_seed, (__u8 *)node, offset);
>       chksum = f2fs_chksum(sbi, chksum, (__u8 *)&dummy_cs, cs_size);
>       offset += cs_size;
> -     chksum = f2fs_chksum(sbi, chksum, (__u8 *)ri + offset,
> +     chksum = f2fs_chksum(sbi, chksum, (__u8 *)node + offset,
>                                               F2FS_BLKSIZE - offset);
>       return chksum;
>  }
>  
> -bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi, struct page *page)
> +bool f2fs_node_chksum_verify(struct f2fs_sb_info *sbi, struct page *page)
>  {
> -     struct f2fs_inode *ri;
> +     struct f2fs_node *rn;
>       __u32 provided, calculated;
>  
> -     if (!f2fs_enable_inode_chksum(sbi, page) ||
> +     if (!f2fs_enable_node_chksum(sbi, page) ||
>                       PageDirty(page) || PageWriteback(page))
>               return true;
>  
> -     ri = &F2FS_NODE(page)->i;
> -     provided = le32_to_cpu(ri->i_inode_checksum);
> -     calculated = f2fs_inode_chksum(sbi, page);
> +     rn = F2FS_NODE(page);
> +     if (RAW_IS_INODE(rn))
> +             provided = le32_to_cpu(rn->i.i_inode_checksum);
> +     else
> +             provided = le32_to_cpu(rn->node_checksum);
> +     calculated = f2fs_node_chksum(sbi, page);
>  
>       if (provided != calculated)
>               f2fs_msg(sbi->sb, KERN_WARNING,
> -                     "checksum invalid, ino = %x, %x vs. %x",
> -                     ino_of_node(page), provided, calculated);
> +                     "checksum invalid, ino = %u, nid = %u, %x vs. %x",
> +                     ino_of_node(page), nid_of_node(page),
> +                     provided, calculated);
> +
> +     else
> +             f2fs_msg(sbi->sb, KERN_WARNING,
> +                     "checksum is valid, ino = %u, nid = %u, %x vs. %x",
> +                     ino_of_node(page), nid_of_node(page),
> +                     provided, calculated);

Need to clean up.

>  
>       return provided == calculated;
>  }
>  
> -void f2fs_inode_chksum_set(struct f2fs_sb_info *sbi, struct page *page)
> +void f2fs_node_chksum_set(struct f2fs_sb_info *sbi, struct page *page)
>  {
> -     struct f2fs_inode *ri = &F2FS_NODE(page)->i;
> +     struct f2fs_node *rn = F2FS_NODE(page);
> +     __le32 chksum;
>  
> -     if (!f2fs_enable_inode_chksum(sbi, page))
> +     if (!f2fs_enable_node_chksum(sbi, page))
>               return;
>  
> -     ri->i_inode_checksum = cpu_to_le32(f2fs_inode_chksum(sbi, page));
> +     chksum = cpu_to_le32(f2fs_node_chksum(sbi, page));
> +
> +     if (RAW_IS_INODE(rn))
> +             rn->i.i_inode_checksum = chksum;
> +     else
> +             rn->node_checksum = chksum;
>  }
>  
>  static int do_read_inode(struct inode *inode)
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index bc8424babf36..613e6bc65f9c 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1194,7 +1194,7 @@ static struct page *__get_node_page(struct f2fs_sb_info 
> *sbi, pgoff_t nid,
>               goto out_err;
>       }
>  
> -     if (!f2fs_inode_chksum_verify(sbi, page)) {
> +     if (!f2fs_node_chksum_verify(sbi, page)) {
>               err = -EBADMSG;
>               goto out_err;
>       }
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index ba8cd4e5ad75..5aa2c24cdcd8 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2678,7 +2678,7 @@ void allocate_data_block(struct f2fs_sb_info *sbi, 
> struct page *page,
>       if (page && IS_NODESEG(type)) {
>               fill_node_footer_blkaddr(page, NEXT_FREE_BLKADDR(sbi, curseg));
>  
> -             f2fs_inode_chksum_set(sbi, page);
> +             f2fs_node_chksum_set(sbi, page);
>       }
>  
>       if (add_list) {
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index f25a6fc0a17e..24d3b82c7bf0 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -119,6 +119,9 @@ static ssize_t features_show(struct f2fs_attr *a,
>       if (f2fs_sb_has_extended_node(sb))
>               len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
>                               len ? ", " : "", "extended_node");
> +     if (f2fs_sb_has_node_checksum(sb))
> +             len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
> +                             len ? ", " : "", "node_checksum");
>       len += snprintf(buf + len, PAGE_SIZE - len, "\n");
>       return len;
>  }
> @@ -240,6 +243,7 @@ enum feat_id {
>       FEAT_QUOTA_INO,
>       FEAT_INODE_CRTIME,
>       FEAT_EXTENDED_NODE,
> +     FEAT_NODE_CHECKSUM,
>  };
>  
>  static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> @@ -256,6 +260,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
>       case FEAT_QUOTA_INO:
>       case FEAT_INODE_CRTIME:
>       case FEAT_EXTENDED_NODE:
> +     case FEAT_NODE_CHECKSUM:
>               return snprintf(buf, PAGE_SIZE, "supported\n");
>       }
>       return 0;
> @@ -335,6 +340,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(extended_node, FEAT_EXTENDED_NODE);
> +F2FS_FEATURE_RO_ATTR(node_checksum, FEAT_NODE_CHECKSUM);
>  
>  #define ATTR_LIST(name) (&f2fs_attr_##name.attr)
>  static struct attribute *f2fs_attrs[] = {
> @@ -390,6 +396,7 @@ static struct attribute *f2fs_feat_attrs[] = {
>       ATTR_LIST(quota_ino),
>       ATTR_LIST(inode_crtime),
>       ATTR_LIST(extended_node),
> +     ATTR_LIST(node_checksum),
>       NULL,
>  };
>  
> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> index a6bacfdd378d..941b5330c504 100644
> --- a/include/linux/f2fs_fs.h
> +++ b/include/linux/f2fs_fs.h
> @@ -295,8 +295,14 @@ struct f2fs_node {
>       /* can be one of three types: inode, direct, and indirect types */
>       union {
>               struct f2fs_inode i;
> -             struct direct_node dn;
> -             struct indirect_node in;
> +             union {
> +                     struct {
> +                             __le32 data[1017];
> +                             __le32 node_checksum;

What does 1017 mean? We need to make this structure more flexibly for new
entries. Like this?
                union {
                        struct node_v1;
                        struct node_v2;
                        struct node_v3;
                        ...
                        struct direct_node dn;
                        struct indirect_node in;
                };
        };

        struct node_v1 {
                __le32 data[DEF_ADDRS_PER_BLOCK - V1_NSIZE=1];
                __le32 node_checksum;
        }

        struct node_v2 {
                __le32 data[DEF_ADDRS_PER_BLOCK - V2_NSIZE=500];
                __le32 comp[V2_NSIZE];
        }
        ...

> +                     };
> +                     struct direct_node dn;
> +                     struct indirect_node in;
> +             };
>       };
>       struct node_footer footer;
>  } __packed;
> -- 
> 2.15.0.55.gc2ece9dc4de6

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