From: Chao Yu <yuchao0@huawei.com> To: Jaegeuk Kim <jaegeuk@kernel.org> Cc: <linux-kernel@vger.kernel.org>, <linux-f2fs-devel@lists.sourceforge.net> Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: allow unfixed f2fs_checkpoint.checksum_offset Date: Wed, 24 Apr 2019 15:14:24 +0800 [thread overview] Message-ID: <a2bf9e24-2c33-b9cd-c214-2887f7f099ba@huawei.com> (raw) In-Reply-To: <20190423205619.GB87257@jaegeuk-macbookpro.roam.corp.google.com> On 2019/4/24 4:56, Jaegeuk Kim wrote: > On 04/23, Jaegeuk Kim wrote: >> On 04/22, Chao Yu wrote: >>> Previously, f2fs_checkpoint.checksum_offset points fixed position of >>> f2fs_checkpoint structure: >>> >>> "#define CP_CHKSUM_OFFSET 4092" >>> >>> It is unnecessary, and it breaks the consecutiveness of nat and sit >>> bitmap stored across checkpoint park block and payload blocks. >>> >>> This patch allows f2fs to handle unfixed .checksum_offset. >>> >>> In addition, for the case checksum value is stored in the middle of >>> checkpoint park, calculating checksum value with superposition method >>> like we did for inode_checksum. >>> >>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>> --- >>> fs/f2fs/checkpoint.c | 27 +++++++++++++++++++++------ >>> include/linux/f2fs_fs.h | 4 ++++ >>> 2 files changed, 25 insertions(+), 6 deletions(-) >>> >>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >>> index 441814607b13..a25556aef8cc 100644 >>> --- a/fs/f2fs/checkpoint.c >>> +++ b/fs/f2fs/checkpoint.c >>> @@ -794,13 +794,27 @@ static void write_orphan_inodes(struct f2fs_sb_info *sbi, block_t start_blk) >>> } >>> } >>> >>> +static __u32 f2fs_checkpoint_chksum(struct f2fs_sb_info *sbi, >>> + struct f2fs_checkpoint *ckpt) >>> +{ >>> + unsigned int chksum_ofs = le32_to_cpu(ckpt->checksum_offset); >>> + __u32 chksum; >>> + >>> + chksum = f2fs_crc32(sbi, ckpt, chksum_ofs); >>> + if (chksum_ofs < CP_CHKSUM_OFFSET) { >>> + chksum_ofs += sizeof(chksum); >>> + chksum = f2fs_chksum(sbi, chksum, (__u8 *)ckpt + chksum_ofs, >>> + F2FS_BLKSIZE - chksum_ofs); >> >> Do we need to cover __cp_payload(sbi) * blksize - chksum_ofs? > > Self answer - it'd be fine to get 4KB only, since payload will be covered > by entire checkpoint pack. Yup, ;) Thanks, > >> >>> + } >>> + return chksum; >>> +} >>> + >>> static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr, >>> struct f2fs_checkpoint **cp_block, struct page **cp_page, >>> unsigned long long *version) >>> { >>> - unsigned long blk_size = sbi->blocksize; >>> size_t crc_offset = 0; >>> - __u32 crc = 0; >>> + __u32 crc; >>> >>> *cp_page = f2fs_get_meta_page(sbi, cp_addr); >>> if (IS_ERR(*cp_page)) >>> @@ -809,15 +823,16 @@ static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr, >>> *cp_block = (struct f2fs_checkpoint *)page_address(*cp_page); >>> >>> crc_offset = le32_to_cpu((*cp_block)->checksum_offset); >>> - if (crc_offset > (blk_size - sizeof(__le32))) { >>> + if (crc_offset < CP_MIN_CHKSUM_OFFSET || >>> + crc_offset > CP_CHKSUM_OFFSET) { >>> f2fs_put_page(*cp_page, 1); >>> f2fs_msg(sbi->sb, KERN_WARNING, >>> "invalid crc_offset: %zu", crc_offset); >>> return -EINVAL; >>> } >>> >>> - crc = cur_cp_crc(*cp_block); >>> - if (!f2fs_crc_valid(sbi, crc, *cp_block, crc_offset)) { >>> + crc = f2fs_checkpoint_chksum(sbi, *cp_block); >>> + if (crc != cur_cp_crc(*cp_block)) { >>> f2fs_put_page(*cp_page, 1); >>> f2fs_msg(sbi->sb, KERN_WARNING, "invalid crc value"); >>> return -EINVAL; >>> @@ -1425,7 +1440,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) >>> get_sit_bitmap(sbi, __bitmap_ptr(sbi, SIT_BITMAP)); >>> get_nat_bitmap(sbi, __bitmap_ptr(sbi, NAT_BITMAP)); >>> >>> - crc32 = f2fs_crc32(sbi, ckpt, le32_to_cpu(ckpt->checksum_offset)); >>> + crc32 = f2fs_checkpoint_chksum(sbi, ckpt); >>> *((__le32 *)((unsigned char *)ckpt + >>> le32_to_cpu(ckpt->checksum_offset))) >>> = cpu_to_le32(crc32); >>> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h >>> index 55da9abed023..65559900d4d7 100644 >>> --- a/include/linux/f2fs_fs.h >>> +++ b/include/linux/f2fs_fs.h >>> @@ -164,6 +164,10 @@ struct f2fs_checkpoint { >>> unsigned char sit_nat_version_bitmap[1]; >>> } __packed; >>> >>> +#define CP_CHKSUM_OFFSET 4092 /* default chksum offset in checkpoint */ >>> +#define CP_MIN_CHKSUM_OFFSET \ >>> + (offsetof(struct f2fs_checkpoint, sit_nat_version_bitmap)) >>> + >>> /* >>> * For orphan inode management >>> */ >>> -- >>> 2.18.0.rc1 >> >> >> _______________________________________________ >> Linux-f2fs-devel mailing list >> Linux-f2fs-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > . >
WARNING: multiple messages have this Message-ID (diff)
From: Chao Yu <yuchao0@huawei.com> To: Jaegeuk Kim <jaegeuk@kernel.org> Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: allow unfixed f2fs_checkpoint.checksum_offset Date: Wed, 24 Apr 2019 15:14:24 +0800 [thread overview] Message-ID: <a2bf9e24-2c33-b9cd-c214-2887f7f099ba@huawei.com> (raw) In-Reply-To: <20190423205619.GB87257@jaegeuk-macbookpro.roam.corp.google.com> On 2019/4/24 4:56, Jaegeuk Kim wrote: > On 04/23, Jaegeuk Kim wrote: >> On 04/22, Chao Yu wrote: >>> Previously, f2fs_checkpoint.checksum_offset points fixed position of >>> f2fs_checkpoint structure: >>> >>> "#define CP_CHKSUM_OFFSET 4092" >>> >>> It is unnecessary, and it breaks the consecutiveness of nat and sit >>> bitmap stored across checkpoint park block and payload blocks. >>> >>> This patch allows f2fs to handle unfixed .checksum_offset. >>> >>> In addition, for the case checksum value is stored in the middle of >>> checkpoint park, calculating checksum value with superposition method >>> like we did for inode_checksum. >>> >>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>> --- >>> fs/f2fs/checkpoint.c | 27 +++++++++++++++++++++------ >>> include/linux/f2fs_fs.h | 4 ++++ >>> 2 files changed, 25 insertions(+), 6 deletions(-) >>> >>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >>> index 441814607b13..a25556aef8cc 100644 >>> --- a/fs/f2fs/checkpoint.c >>> +++ b/fs/f2fs/checkpoint.c >>> @@ -794,13 +794,27 @@ static void write_orphan_inodes(struct f2fs_sb_info *sbi, block_t start_blk) >>> } >>> } >>> >>> +static __u32 f2fs_checkpoint_chksum(struct f2fs_sb_info *sbi, >>> + struct f2fs_checkpoint *ckpt) >>> +{ >>> + unsigned int chksum_ofs = le32_to_cpu(ckpt->checksum_offset); >>> + __u32 chksum; >>> + >>> + chksum = f2fs_crc32(sbi, ckpt, chksum_ofs); >>> + if (chksum_ofs < CP_CHKSUM_OFFSET) { >>> + chksum_ofs += sizeof(chksum); >>> + chksum = f2fs_chksum(sbi, chksum, (__u8 *)ckpt + chksum_ofs, >>> + F2FS_BLKSIZE - chksum_ofs); >> >> Do we need to cover __cp_payload(sbi) * blksize - chksum_ofs? > > Self answer - it'd be fine to get 4KB only, since payload will be covered > by entire checkpoint pack. Yup, ;) Thanks, > >> >>> + } >>> + return chksum; >>> +} >>> + >>> static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr, >>> struct f2fs_checkpoint **cp_block, struct page **cp_page, >>> unsigned long long *version) >>> { >>> - unsigned long blk_size = sbi->blocksize; >>> size_t crc_offset = 0; >>> - __u32 crc = 0; >>> + __u32 crc; >>> >>> *cp_page = f2fs_get_meta_page(sbi, cp_addr); >>> if (IS_ERR(*cp_page)) >>> @@ -809,15 +823,16 @@ static int get_checkpoint_version(struct f2fs_sb_info *sbi, block_t cp_addr, >>> *cp_block = (struct f2fs_checkpoint *)page_address(*cp_page); >>> >>> crc_offset = le32_to_cpu((*cp_block)->checksum_offset); >>> - if (crc_offset > (blk_size - sizeof(__le32))) { >>> + if (crc_offset < CP_MIN_CHKSUM_OFFSET || >>> + crc_offset > CP_CHKSUM_OFFSET) { >>> f2fs_put_page(*cp_page, 1); >>> f2fs_msg(sbi->sb, KERN_WARNING, >>> "invalid crc_offset: %zu", crc_offset); >>> return -EINVAL; >>> } >>> >>> - crc = cur_cp_crc(*cp_block); >>> - if (!f2fs_crc_valid(sbi, crc, *cp_block, crc_offset)) { >>> + crc = f2fs_checkpoint_chksum(sbi, *cp_block); >>> + if (crc != cur_cp_crc(*cp_block)) { >>> f2fs_put_page(*cp_page, 1); >>> f2fs_msg(sbi->sb, KERN_WARNING, "invalid crc value"); >>> return -EINVAL; >>> @@ -1425,7 +1440,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) >>> get_sit_bitmap(sbi, __bitmap_ptr(sbi, SIT_BITMAP)); >>> get_nat_bitmap(sbi, __bitmap_ptr(sbi, NAT_BITMAP)); >>> >>> - crc32 = f2fs_crc32(sbi, ckpt, le32_to_cpu(ckpt->checksum_offset)); >>> + crc32 = f2fs_checkpoint_chksum(sbi, ckpt); >>> *((__le32 *)((unsigned char *)ckpt + >>> le32_to_cpu(ckpt->checksum_offset))) >>> = cpu_to_le32(crc32); >>> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h >>> index 55da9abed023..65559900d4d7 100644 >>> --- a/include/linux/f2fs_fs.h >>> +++ b/include/linux/f2fs_fs.h >>> @@ -164,6 +164,10 @@ struct f2fs_checkpoint { >>> unsigned char sit_nat_version_bitmap[1]; >>> } __packed; >>> >>> +#define CP_CHKSUM_OFFSET 4092 /* default chksum offset in checkpoint */ >>> +#define CP_MIN_CHKSUM_OFFSET \ >>> + (offsetof(struct f2fs_checkpoint, sit_nat_version_bitmap)) >>> + >>> /* >>> * For orphan inode management >>> */ >>> -- >>> 2.18.0.rc1 >> >> >> _______________________________________________ >> Linux-f2fs-devel mailing list >> Linux-f2fs-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > . >
next prev parent reply other threads:[~2019-04-24 7:14 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-22 9:33 [PATCH 1/2] f2fs: allow unfixed f2fs_checkpoint.checksum_offset Chao Yu 2019-04-22 9:33 ` Chao Yu 2019-04-22 9:33 ` [PATCH 2/2] f2fs: relocate chksum_offset for large_nat_bitmap feature Chao Yu 2019-04-22 9:33 ` Chao Yu 2019-04-24 11:43 ` [f2fs-dev] " Ju Hyung Park 2019-04-25 1:36 ` Chao Yu 2019-04-25 1:36 ` Chao Yu 2019-04-23 20:43 ` [PATCH 1/2] f2fs: allow unfixed f2fs_checkpoint.checksum_offset Jaegeuk Kim 2019-04-23 20:56 ` [f2fs-dev] " Jaegeuk Kim 2019-04-24 7:14 ` Chao Yu [this message] 2019-04-24 7:14 ` Chao Yu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=a2bf9e24-2c33-b9cd-c214-2887f7f099ba@huawei.com \ --to=yuchao0@huawei.com \ --cc=jaegeuk@kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-kernel@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.