* [PATCH] fsck.f2fs: write checkpoint with OPU mode @ 2019-05-24 7:56 Chao Yu 2019-06-22 21:46 ` [f2fs-dev] " Jaegeuk Kim 0 siblings, 1 reply; 6+ messages in thread From: Chao Yu @ 2019-05-24 7:56 UTC (permalink / raw) To: linux-f2fs-devel; +Cc: jaegeuk This original patch was from Weichao Guo. We may encounter both checkpoints invalid in such a case: 1. kernel writes CP A; 2. power-cut when kernel writes CP B, then CP B is corrupted; 3. fsck: load CP A, fix meta/data; 4. power-cut when fsck writes CP A in-place, then CP A is corrupted too; To avoid both checkpoints being invalid, this patch changes to enables fsck to write checkpoint with out-place-update method first, and then write checkpoint in original place. This can make sure during fsck repairing, even there is sudden power-cut, filesystem will still have at least one valid checkpoint. Signed-off-by: Weichao Guo <guoweichao@huawei.com> Signed-off-by: Chao Yu <yuchao0@huawei.com> --- v2: - clean up codes - cover flush_journal_entries() case - update commet message fsck/fsck.c | 17 +++++++++++++++-- fsck/fsck.h | 1 + fsck/mount.c | 15 ++++++++++++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/fsck/fsck.c b/fsck/fsck.c index 6f0f262..6aed51d 100644 --- a/fsck/fsck.c +++ b/fsck/fsck.c @@ -2121,6 +2121,19 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi) write_nat_bits(sbi, sb, cp, sbi->cur_cp); } +static void fix_checkpoints(struct f2fs_sb_info *sbi) +{ + int i, ret; + + for (i = 0; i < 2; i++) { + /* write checkpoint out of place first */ + sbi->cur_cp = sbi->cur_cp % 2 + 1; + fix_checkpoint(sbi); + ret = f2fs_fsync_device(); + ASSERT(ret >= 0); + } +} + int check_curseg_offset(struct f2fs_sb_info *sbi, int type) { struct curseg_info *curseg = CURSEG_I(sbi, type); @@ -2771,10 +2784,10 @@ int fsck_verify(struct f2fs_sb_info *sbi) rewrite_sit_area_bitmap(sbi); fix_curseg_info(sbi); fix_checksum(sbi); - fix_checkpoint(sbi); + fix_checkpoints(sbi); } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) || is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) { - write_checkpoint(sbi); + write_checkpoints(sbi); } } return ret; diff --git a/fsck/fsck.h b/fsck/fsck.h index d38e8de..8fe5db1 100644 --- a/fsck/fsck.h +++ b/fsck/fsck.h @@ -192,6 +192,7 @@ extern void move_curseg_info(struct f2fs_sb_info *, u64, int); extern void write_curseg_info(struct f2fs_sb_info *); extern int find_next_free_block(struct f2fs_sb_info *, u64 *, int, int); extern void write_checkpoint(struct f2fs_sb_info *); +extern void write_checkpoints(struct f2fs_sb_info *); extern void update_superblock(struct f2fs_super_block *, int); extern void update_data_blkaddr(struct f2fs_sb_info *, nid_t, u16, block_t); extern void update_nat_blkaddr(struct f2fs_sb_info *, nid_t, nid_t, block_t); diff --git a/fsck/mount.c b/fsck/mount.c index 1c5cd93..bbb1af7 100644 --- a/fsck/mount.c +++ b/fsck/mount.c @@ -2127,7 +2127,7 @@ void flush_journal_entries(struct f2fs_sb_info *sbi) int n_sits = flush_sit_journal_entries(sbi); if (n_nats || n_sits) - write_checkpoint(sbi); + write_checkpoints(sbi); } void flush_sit_entries(struct f2fs_sb_info *sbi) @@ -2452,6 +2452,19 @@ void write_checkpoint(struct f2fs_sb_info *sbi) ASSERT(ret >= 0); } +void write_checkpoints(struct f2fs_sb_info *sbi) +{ + int i, ret; + + for (i = 0; i < 2; i++) { + /* write checkpoint out of place first */ + sbi->cur_cp = sbi->cur_cp % 2 + 1; + write_checkpoint(sbi); + ret = f2fs_fsync_device(); + ASSERT(ret >= 0); + } +} + void build_nat_area_bitmap(struct f2fs_sb_info *sbi) { struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA); -- 2.18.0.rc1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [f2fs-dev] [PATCH] fsck.f2fs: write checkpoint with OPU mode 2019-05-24 7:56 [PATCH] fsck.f2fs: write checkpoint with OPU mode Chao Yu @ 2019-06-22 21:46 ` Jaegeuk Kim [not found] ` <MWHPR02MB26710762B08C9EAB74BB2FABC6E00@MWHPR02MB2671.namprd02.prod.outlook.com> 0 siblings, 1 reply; 6+ messages in thread From: Jaegeuk Kim @ 2019-06-22 21:46 UTC (permalink / raw) To: Chao Yu; +Cc: linux-f2fs-devel Hi Weichao, This patch breaks the image found by my local power-cut tests. On 05/24, Chao Yu wrote: > This original patch was from Weichao Guo. > > We may encounter both checkpoints invalid in such a case: > 1. kernel writes CP A; > 2. power-cut when kernel writes CP B, then CP B is corrupted; > 3. fsck: load CP A, fix meta/data; Would it be better to copy CP A to CP B position first? Thanks, > 4. power-cut when fsck writes CP A in-place, then CP A is corrupted too; > > To avoid both checkpoints being invalid, this patch changes to enables > fsck to write checkpoint with out-place-update method first, and then > write checkpoint in original place. > > This can make sure during fsck repairing, even there is sudden power-cut, > filesystem will still have at least one valid checkpoint. > > Signed-off-by: Weichao Guo <guoweichao@huawei.com> > Signed-off-by: Chao Yu <yuchao0@huawei.com> > --- > v2: > - clean up codes > - cover flush_journal_entries() case > - update commet message > fsck/fsck.c | 17 +++++++++++++++-- > fsck/fsck.h | 1 + > fsck/mount.c | 15 ++++++++++++++- > 3 files changed, 30 insertions(+), 3 deletions(-) > > diff --git a/fsck/fsck.c b/fsck/fsck.c > index 6f0f262..6aed51d 100644 > --- a/fsck/fsck.c > +++ b/fsck/fsck.c > @@ -2121,6 +2121,19 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi) > write_nat_bits(sbi, sb, cp, sbi->cur_cp); > } > > +static void fix_checkpoints(struct f2fs_sb_info *sbi) > +{ > + int i, ret; > + > + for (i = 0; i < 2; i++) { > + /* write checkpoint out of place first */ > + sbi->cur_cp = sbi->cur_cp % 2 + 1; > + fix_checkpoint(sbi); > + ret = f2fs_fsync_device(); > + ASSERT(ret >= 0); > + } > +} > + > int check_curseg_offset(struct f2fs_sb_info *sbi, int type) > { > struct curseg_info *curseg = CURSEG_I(sbi, type); > @@ -2771,10 +2784,10 @@ int fsck_verify(struct f2fs_sb_info *sbi) > rewrite_sit_area_bitmap(sbi); > fix_curseg_info(sbi); > fix_checksum(sbi); > - fix_checkpoint(sbi); > + fix_checkpoints(sbi); > } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) || > is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) { > - write_checkpoint(sbi); > + write_checkpoints(sbi); > } > } > return ret; > diff --git a/fsck/fsck.h b/fsck/fsck.h > index d38e8de..8fe5db1 100644 > --- a/fsck/fsck.h > +++ b/fsck/fsck.h > @@ -192,6 +192,7 @@ extern void move_curseg_info(struct f2fs_sb_info *, u64, int); > extern void write_curseg_info(struct f2fs_sb_info *); > extern int find_next_free_block(struct f2fs_sb_info *, u64 *, int, int); > extern void write_checkpoint(struct f2fs_sb_info *); > +extern void write_checkpoints(struct f2fs_sb_info *); > extern void update_superblock(struct f2fs_super_block *, int); > extern void update_data_blkaddr(struct f2fs_sb_info *, nid_t, u16, block_t); > extern void update_nat_blkaddr(struct f2fs_sb_info *, nid_t, nid_t, block_t); > diff --git a/fsck/mount.c b/fsck/mount.c > index 1c5cd93..bbb1af7 100644 > --- a/fsck/mount.c > +++ b/fsck/mount.c > @@ -2127,7 +2127,7 @@ void flush_journal_entries(struct f2fs_sb_info *sbi) > int n_sits = flush_sit_journal_entries(sbi); > > if (n_nats || n_sits) > - write_checkpoint(sbi); > + write_checkpoints(sbi); > } > > void flush_sit_entries(struct f2fs_sb_info *sbi) > @@ -2452,6 +2452,19 @@ void write_checkpoint(struct f2fs_sb_info *sbi) > ASSERT(ret >= 0); > } > > +void write_checkpoints(struct f2fs_sb_info *sbi) > +{ > + int i, ret; > + > + for (i = 0; i < 2; i++) { > + /* write checkpoint out of place first */ > + sbi->cur_cp = sbi->cur_cp % 2 + 1; > + write_checkpoint(sbi); > + ret = f2fs_fsync_device(); > + ASSERT(ret >= 0); > + } > +} > + > void build_nat_area_bitmap(struct f2fs_sb_info *sbi) > { > struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA); > -- > 2.18.0.rc1 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <MWHPR02MB26710762B08C9EAB74BB2FABC6E00@MWHPR02MB2671.namprd02.prod.outlook.com>]
* Re: [f2fs-dev] 回复: [PATCH] fsck.f2fs: write checkpoint with OPU mode [not found] ` <MWHPR02MB26710762B08C9EAB74BB2FABC6E00@MWHPR02MB2671.namprd02.prod.outlook.com> @ 2019-06-24 2:24 ` Chao Yu 2019-06-24 14:36 ` Chao Yu 0 siblings, 1 reply; 6+ messages in thread From: Chao Yu @ 2019-06-24 2:24 UTC (permalink / raw) To: guo weichao, Jaegeuk Kim; +Cc: linux-f2fs-devel Hi Jaegeuk, I picked up Weichao's patch since I'm not sure whether Weichao still has time working on it. On 2019/6/24 9:23, guo weichao wrote: > Hi Jaegeuk, > > I think it's better to copy CP A to CP B position first, which can make sure we > have a fsck-not-touched correct checkpoint. Jaegeuk, Weichao, I think it's okay, let me update the patch. :) > > P.S: did you want to discuss it with Chao Yu? :）HAHA Weichao, it's glad to see your activity again. ;) Thanks, > > BR, > Weichao > -------------------------------------------------------------------------------- > *发件人:* Jaegeuk Kim <jaegeuk@kernel.org> > *发送时间:* 2019年6月23日 5:46 > *收件人:* Chao Yu > *抄送:* linux-f2fs-devel@lists.sourceforge.net > *主题:* Re: [f2fs-dev] [PATCH] fsck.f2fs: write checkpoint with OPU mode > > Hi Weichao, > > This patch breaks the image found by my local power-cut tests. > > On 05/24, Chao Yu wrote: >> This original patch was from Weichao Guo. >> >> We may encounter both checkpoints invalid in such a case: >> 1. kernel writes CP A; >> 2. power-cut when kernel writes CP B, then CP B is corrupted; >> 3. fsck: load CP A, fix meta/data; > > Would it be better to copy CP A to CP B position first? > > Thanks, > >> 4. power-cut when fsck writes CP A in-place, then CP A is corrupted too; >> >> To avoid both checkpoints being invalid, this patch changes to enables >> fsck to write checkpoint with out-place-update method first, and then >> write checkpoint in original place. >> >> This can make sure during fsck repairing, even there is sudden power-cut, >> filesystem will still have at least one valid checkpoint. >> >> Signed-off-by: Weichao Guo <guoweichao@huawei.com> >> Signed-off-by: Chao Yu <yuchao0@huawei.com> >> --- >> v2: >> - clean up codes >> - cover flush_journal_entries() case >> - update commet message >> fsck/fsck.c | 17 +++++++++++++++-- >> fsck/fsck.h | 1 + >> fsck/mount.c | 15 ++++++++++++++- >> 3 files changed, 30 insertions(+), 3 deletions(-) >> >> diff --git a/fsck/fsck.c b/fsck/fsck.c >> index 6f0f262..6aed51d 100644 >> --- a/fsck/fsck.c >> +++ b/fsck/fsck.c >> @@ -2121,6 +2121,19 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi) >> write_nat_bits(sbi, sb, cp, sbi->cur_cp); >> } >> >> +static void fix_checkpoints(struct f2fs_sb_info *sbi) >> +{ >> + int i, ret; >> + >> + for (i = 0; i < 2; i++) { >> + /* write checkpoint out of place first */ >> + sbi->cur_cp = sbi->cur_cp % 2 + 1; >> + fix_checkpoint(sbi); >> + ret = f2fs_fsync_device(); >> + ASSERT(ret >= 0); >> + } >> +} >> + >> int check_curseg_offset(struct f2fs_sb_info *sbi, int type) >> { >> struct curseg_info *curseg = CURSEG_I(sbi, type); >> @@ -2771,10 +2784,10 @@ int fsck_verify(struct f2fs_sb_info *sbi) >> rewrite_sit_area_bitmap(sbi); >> fix_curseg_info(sbi); >> fix_checksum(sbi); >> - fix_checkpoint(sbi); >> + fix_checkpoints(sbi); >> } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) || >> is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) { >> - write_checkpoint(sbi); >> + write_checkpoints(sbi); >> } >> } >> return ret; >> diff --git a/fsck/fsck.h b/fsck/fsck.h >> index d38e8de..8fe5db1 100644 >> --- a/fsck/fsck.h >> +++ b/fsck/fsck.h >> @@ -192,6 +192,7 @@ extern void move_curseg_info(struct f2fs_sb_info *, u64, int); >> extern void write_curseg_info(struct f2fs_sb_info *); >> extern int find_next_free_block(struct f2fs_sb_info *, u64 *, int, int); >> extern void write_checkpoint(struct f2fs_sb_info *); >> +extern void write_checkpoints(struct f2fs_sb_info *); >> extern void update_superblock(struct f2fs_super_block *, int); >> extern void update_data_blkaddr(struct f2fs_sb_info *, nid_t, u16, block_t); >> extern void update_nat_blkaddr(struct f2fs_sb_info *, nid_t, nid_t, block_t); >> diff --git a/fsck/mount.c b/fsck/mount.c >> index 1c5cd93..bbb1af7 100644 >> --- a/fsck/mount.c >> +++ b/fsck/mount.c >> @@ -2127,7 +2127,7 @@ void flush_journal_entries(struct f2fs_sb_info *sbi) >> int n_sits = flush_sit_journal_entries(sbi); >> >> if (n_nats || n_sits) >> - write_checkpoint(sbi); >> + write_checkpoints(sbi); >> } >> >> void flush_sit_entries(struct f2fs_sb_info *sbi) >> @@ -2452,6 +2452,19 @@ void write_checkpoint(struct f2fs_sb_info *sbi) >> ASSERT(ret >= 0); >> } >> >> +void write_checkpoints(struct f2fs_sb_info *sbi) >> +{ >> + int i, ret; >> + >> + for (i = 0; i < 2; i++) { >> + /* write checkpoint out of place first */ >> + sbi->cur_cp = sbi->cur_cp % 2 + 1; >> + write_checkpoint(sbi); >> + ret = f2fs_fsync_device(); >> + ASSERT(ret >= 0); >> + } >> +} >> + >> void build_nat_area_bitmap(struct f2fs_sb_info *sbi) >> { >> struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA); >> -- >> 2.18.0.rc1 > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [f2fs-dev] 回复: [PATCH] fsck.f2fs: write checkpoint with OPU mode 2019-06-24 2:24 ` [f2fs-dev] 回复: " Chao Yu @ 2019-06-24 14:36 ` Chao Yu 2019-06-24 16:02 ` Jaegeuk Kim 0 siblings, 1 reply; 6+ messages in thread From: Chao Yu @ 2019-06-24 14:36 UTC (permalink / raw) To: Chao Yu, guo weichao, Jaegeuk Kim; +Cc: linux-f2fs-devel Hi all, One more concern is that, if checkpoint A is corrupted, and checkpoint B is valid, we may copy CP B to CP A, and then writeback fixed CP B with the same cp_ver, then kernel will load CP A if two CP has the same cp_ver, result in loading wrong CP, right? Thanks, On 2019-6-24 10:24, Chao Yu wrote: > Hi Jaegeuk, > > I picked up Weichao's patch since I'm not sure whether Weichao still has time > working on it. > > On 2019/6/24 9:23, guo weichao wrote: >> Hi Jaegeuk, >> >> I think it's better to copy CP A to CP B position first, which can make sure we >> have a fsck-not-touched correct checkpoint. > > Jaegeuk, Weichao, > > I think it's okay, let me update the patch. :) > >> >> P.S: did you want to discuss it with Chao Yu? :）HAHA > > Weichao, it's glad to see your activity again. ;) > > Thanks, > >> >> BR, >> Weichao >> -------------------------------------------------------------------------------- >> *发件人:* Jaegeuk Kim <jaegeuk@kernel.org> >> *发送时间:* 2019年6月23日 5:46 >> *收件人:* Chao Yu >> *抄送:* linux-f2fs-devel@lists.sourceforge.net >> *主题:* Re: [f2fs-dev] [PATCH] fsck.f2fs: write checkpoint with OPU mode >> >> Hi Weichao, >> >> This patch breaks the image found by my local power-cut tests. >> >> On 05/24, Chao Yu wrote: >>> This original patch was from Weichao Guo. >>> >>> We may encounter both checkpoints invalid in such a case: >>> 1. kernel writes CP A; >>> 2. power-cut when kernel writes CP B, then CP B is corrupted; >>> 3. fsck: load CP A, fix meta/data; >> >> Would it be better to copy CP A to CP B position first? >> >> Thanks, >> >>> 4. power-cut when fsck writes CP A in-place, then CP A is corrupted too; >>> >>> To avoid both checkpoints being invalid, this patch changes to enables >>> fsck to write checkpoint with out-place-update method first, and then >>> write checkpoint in original place. >>> >>> This can make sure during fsck repairing, even there is sudden power-cut, >>> filesystem will still have at least one valid checkpoint. >>> >>> Signed-off-by: Weichao Guo <guoweichao@huawei.com> >>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>> --- >>> v2: >>> - clean up codes >>> - cover flush_journal_entries() case >>> - update commet message >>> fsck/fsck.c | 17 +++++++++++++++-- >>> fsck/fsck.h | 1 + >>> fsck/mount.c | 15 ++++++++++++++- >>> 3 files changed, 30 insertions(+), 3 deletions(-) >>> >>> diff --git a/fsck/fsck.c b/fsck/fsck.c >>> index 6f0f262..6aed51d 100644 >>> --- a/fsck/fsck.c >>> +++ b/fsck/fsck.c >>> @@ -2121,6 +2121,19 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi) >>> write_nat_bits(sbi, sb, cp, sbi->cur_cp); >>> } >>> >>> +static void fix_checkpoints(struct f2fs_sb_info *sbi) >>> +{ >>> + int i, ret; >>> + >>> + for (i = 0; i < 2; i++) { >>> + /* write checkpoint out of place first */ >>> + sbi->cur_cp = sbi->cur_cp % 2 + 1; >>> + fix_checkpoint(sbi); >>> + ret = f2fs_fsync_device(); >>> + ASSERT(ret >= 0); >>> + } >>> +} >>> + >>> int check_curseg_offset(struct f2fs_sb_info *sbi, int type) >>> { >>> struct curseg_info *curseg = CURSEG_I(sbi, type); >>> @@ -2771,10 +2784,10 @@ int fsck_verify(struct f2fs_sb_info *sbi) >>> rewrite_sit_area_bitmap(sbi); >>> fix_curseg_info(sbi); >>> fix_checksum(sbi); >>> - fix_checkpoint(sbi); >>> + fix_checkpoints(sbi); >>> } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) || >>> is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) { >>> - write_checkpoint(sbi); >>> + write_checkpoints(sbi); >>> } >>> } >>> return ret; >>> diff --git a/fsck/fsck.h b/fsck/fsck.h >>> index d38e8de..8fe5db1 100644 >>> --- a/fsck/fsck.h >>> +++ b/fsck/fsck.h >>> @@ -192,6 +192,7 @@ extern void move_curseg_info(struct f2fs_sb_info *, u64, int); >>> extern void write_curseg_info(struct f2fs_sb_info *); >>> extern int find_next_free_block(struct f2fs_sb_info *, u64 *, int, int); >>> extern void write_checkpoint(struct f2fs_sb_info *); >>> +extern void write_checkpoints(struct f2fs_sb_info *); >>> extern void update_superblock(struct f2fs_super_block *, int); >>> extern void update_data_blkaddr(struct f2fs_sb_info *, nid_t, u16, block_t); >>> extern void update_nat_blkaddr(struct f2fs_sb_info *, nid_t, nid_t, block_t); >>> diff --git a/fsck/mount.c b/fsck/mount.c >>> index 1c5cd93..bbb1af7 100644 >>> --- a/fsck/mount.c >>> +++ b/fsck/mount.c >>> @@ -2127,7 +2127,7 @@ void flush_journal_entries(struct f2fs_sb_info *sbi) >>> int n_sits = flush_sit_journal_entries(sbi); >>> >>> if (n_nats || n_sits) >>> - write_checkpoint(sbi); >>> + write_checkpoints(sbi); >>> } >>> >>> void flush_sit_entries(struct f2fs_sb_info *sbi) >>> @@ -2452,6 +2452,19 @@ void write_checkpoint(struct f2fs_sb_info *sbi) >>> ASSERT(ret >= 0); >>> } >>> >>> +void write_checkpoints(struct f2fs_sb_info *sbi) >>> +{ >>> + int i, ret; >>> + >>> + for (i = 0; i < 2; i++) { >>> + /* write checkpoint out of place first */ >>> + sbi->cur_cp = sbi->cur_cp % 2 + 1; >>> + write_checkpoint(sbi); >>> + ret = f2fs_fsync_device(); >>> + ASSERT(ret >= 0); >>> + } >>> +} >>> + >>> void build_nat_area_bitmap(struct f2fs_sb_info *sbi) >>> { >>> struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA); >>> -- >>> 2.18.0.rc1 >> >> >> _______________________________________________ >> Linux-f2fs-devel mailing list >> Linux-f2fs-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [f2fs-dev] 回复: [PATCH] fsck.f2fs: write checkpoint with OPU mode 2019-06-24 14:36 ` Chao Yu @ 2019-06-24 16:02 ` Jaegeuk Kim 2019-06-25 1:59 ` Chao Yu 0 siblings, 1 reply; 6+ messages in thread From: Jaegeuk Kim @ 2019-06-24 16:02 UTC (permalink / raw) To: Chao Yu; +Cc: linux-f2fs-devel On 06/24, Chao Yu wrote: > Hi all, > > One more concern is that, if checkpoint A is corrupted, and checkpoint B is > valid, we may copy CP B to CP A, and then writeback fixed CP B with the same > cp_ver, then kernel will load CP A if two CP has the same cp_ver, result in > loading wrong CP, right? Yup, we need to handle that. When copying the checkpoint, we may need to copy whole segment w/ version - 1. > > Thanks, > > On 2019-6-24 10:24, Chao Yu wrote: > > Hi Jaegeuk, > > > > I picked up Weichao's patch since I'm not sure whether Weichao still has time > > working on it. > > > > On 2019/6/24 9:23, guo weichao wrote: > >> Hi Jaegeuk, > >> > >> I think it's better to copy CP A to CP B position first, which can make sure we > >> have a fsck-not-touched correct checkpoint. > > > > Jaegeuk, Weichao, > > > > I think it's okay, let me update the patch. :) > > > >> > >> P.S: did you want to discuss it with Chao Yu? :）HAHA > > > > Weichao, it's glad to see your activity again. ;) > > > > Thanks, > > > >> > >> BR, > >> Weichao > >> -------------------------------------------------------------------------------- > >> *发件人:* Jaegeuk Kim <jaegeuk@kernel.org> > >> *发送时间:* 2019年6月23日 5:46 > >> *收件人:* Chao Yu > >> *抄送:* linux-f2fs-devel@lists.sourceforge.net > >> *主题:* Re: [f2fs-dev] [PATCH] fsck.f2fs: write checkpoint with OPU mode > >> > >> Hi Weichao, > >> > >> This patch breaks the image found by my local power-cut tests. > >> > >> On 05/24, Chao Yu wrote: > >>> This original patch was from Weichao Guo. > >>> > >>> We may encounter both checkpoints invalid in such a case: > >>> 1. kernel writes CP A; > >>> 2. power-cut when kernel writes CP B, then CP B is corrupted; > >>> 3. fsck: load CP A, fix meta/data; > >> > >> Would it be better to copy CP A to CP B position first? > >> > >> Thanks, > >> > >>> 4. power-cut when fsck writes CP A in-place, then CP A is corrupted too; > >>> > >>> To avoid both checkpoints being invalid, this patch changes to enables > >>> fsck to write checkpoint with out-place-update method first, and then > >>> write checkpoint in original place. > >>> > >>> This can make sure during fsck repairing, even there is sudden power-cut, > >>> filesystem will still have at least one valid checkpoint. > >>> > >>> Signed-off-by: Weichao Guo <guoweichao@huawei.com> > >>> Signed-off-by: Chao Yu <yuchao0@huawei.com> > >>> --- > >>> v2: > >>> - clean up codes > >>> - cover flush_journal_entries() case > >>> - update commet message > >>> fsck/fsck.c | 17 +++++++++++++++-- > >>> fsck/fsck.h | 1 + > >>> fsck/mount.c | 15 ++++++++++++++- > >>> 3 files changed, 30 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/fsck/fsck.c b/fsck/fsck.c > >>> index 6f0f262..6aed51d 100644 > >>> --- a/fsck/fsck.c > >>> +++ b/fsck/fsck.c > >>> @@ -2121,6 +2121,19 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi) > >>> write_nat_bits(sbi, sb, cp, sbi->cur_cp); > >>> } > >>> > >>> +static void fix_checkpoints(struct f2fs_sb_info *sbi) > >>> +{ > >>> + int i, ret; > >>> + > >>> + for (i = 0; i < 2; i++) { > >>> + /* write checkpoint out of place first */ > >>> + sbi->cur_cp = sbi->cur_cp % 2 + 1; > >>> + fix_checkpoint(sbi); > >>> + ret = f2fs_fsync_device(); > >>> + ASSERT(ret >= 0); > >>> + } > >>> +} > >>> + > >>> int check_curseg_offset(struct f2fs_sb_info *sbi, int type) > >>> { > >>> struct curseg_info *curseg = CURSEG_I(sbi, type); > >>> @@ -2771,10 +2784,10 @@ int fsck_verify(struct f2fs_sb_info *sbi) > >>> rewrite_sit_area_bitmap(sbi); > >>> fix_curseg_info(sbi); > >>> fix_checksum(sbi); > >>> - fix_checkpoint(sbi); > >>> + fix_checkpoints(sbi); > >>> } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) || > >>> is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) { > >>> - write_checkpoint(sbi); > >>> + write_checkpoints(sbi); > >>> } > >>> } > >>> return ret; > >>> diff --git a/fsck/fsck.h b/fsck/fsck.h > >>> index d38e8de..8fe5db1 100644 > >>> --- a/fsck/fsck.h > >>> +++ b/fsck/fsck.h > >>> @@ -192,6 +192,7 @@ extern void move_curseg_info(struct f2fs_sb_info *, u64, int); > >>> extern void write_curseg_info(struct f2fs_sb_info *); > >>> extern int find_next_free_block(struct f2fs_sb_info *, u64 *, int, int); > >>> extern void write_checkpoint(struct f2fs_sb_info *); > >>> +extern void write_checkpoints(struct f2fs_sb_info *); > >>> extern void update_superblock(struct f2fs_super_block *, int); > >>> extern void update_data_blkaddr(struct f2fs_sb_info *, nid_t, u16, block_t); > >>> extern void update_nat_blkaddr(struct f2fs_sb_info *, nid_t, nid_t, block_t); > >>> diff --git a/fsck/mount.c b/fsck/mount.c > >>> index 1c5cd93..bbb1af7 100644 > >>> --- a/fsck/mount.c > >>> +++ b/fsck/mount.c > >>> @@ -2127,7 +2127,7 @@ void flush_journal_entries(struct f2fs_sb_info *sbi) > >>> int n_sits = flush_sit_journal_entries(sbi); > >>> > >>> if (n_nats || n_sits) > >>> - write_checkpoint(sbi); > >>> + write_checkpoints(sbi); > >>> } > >>> > >>> void flush_sit_entries(struct f2fs_sb_info *sbi) > >>> @@ -2452,6 +2452,19 @@ void write_checkpoint(struct f2fs_sb_info *sbi) > >>> ASSERT(ret >= 0); > >>> } > >>> > >>> +void write_checkpoints(struct f2fs_sb_info *sbi) > >>> +{ > >>> + int i, ret; > >>> + > >>> + for (i = 0; i < 2; i++) { > >>> + /* write checkpoint out of place first */ > >>> + sbi->cur_cp = sbi->cur_cp % 2 + 1; > >>> + write_checkpoint(sbi); > >>> + ret = f2fs_fsync_device(); > >>> + ASSERT(ret >= 0); > >>> + } > >>> +} > >>> + > >>> void build_nat_area_bitmap(struct f2fs_sb_info *sbi) > >>> { > >>> struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA); > >>> -- > >>> 2.18.0.rc1 > >> > >> > >> _______________________________________________ > >> Linux-f2fs-devel mailing list > >> Linux-f2fs-devel@lists.sourceforge.net > >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > > > > > _______________________________________________ > > Linux-f2fs-devel mailing list > > Linux-f2fs-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [f2fs-dev] 回复: [PATCH] fsck.f2fs: write checkpoint with OPU mode 2019-06-24 16:02 ` Jaegeuk Kim @ 2019-06-25 1:59 ` Chao Yu 0 siblings, 0 replies; 6+ messages in thread From: Chao Yu @ 2019-06-25 1:59 UTC (permalink / raw) To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel On 2019/6/25 0:02, Jaegeuk Kim wrote: > On 06/24, Chao Yu wrote: >> Hi all, >> >> One more concern is that, if checkpoint A is corrupted, and checkpoint B is >> valid, we may copy CP B to CP A, and then writeback fixed CP B with the same >> cp_ver, then kernel will load CP A if two CP has the same cp_ver, result in >> loading wrong CP, right? > > Yup, we need to handle that. When copying the checkpoint, we may need to copy > whole segment w/ version - 1. Yes, but sadly if CP B becomes corrupted during fsck, CP A with version - 1 will be loaded, but the cp_ver in CP is not matching with cp_ver of node in dnode list, so we may fail to recovery fsynced file later. How about this: 1. copy valid CP to mirror position 2. repair current CP and writeback it to CP #0 position Thanks, > >> >> Thanks, >> >> On 2019-6-24 10:24, Chao Yu wrote: >>> Hi Jaegeuk, >>> >>> I picked up Weichao's patch since I'm not sure whether Weichao still has time >>> working on it. >>> >>> On 2019/6/24 9:23, guo weichao wrote: >>>> Hi Jaegeuk, >>>> >>>> I think it's better to copy CP A to CP B position first, which can make sure we >>>> have a fsck-not-touched correct checkpoint. >>> >>> Jaegeuk, Weichao, >>> >>> I think it's okay, let me update the patch. :) >>> >>>> >>>> P.S: did you want to discuss it with Chao Yu? :）HAHA >>> >>> Weichao, it's glad to see your activity again. ;) >>> >>> Thanks, >>> >>>> >>>> BR, >>>> Weichao >>>> -------------------------------------------------------------------------------- >>>> *发件人:* Jaegeuk Kim <jaegeuk@kernel.org> >>>> *发送时间:* 2019年6月23日 5:46 >>>> *收件人:* Chao Yu >>>> *抄送:* linux-f2fs-devel@lists.sourceforge.net >>>> *主题:* Re: [f2fs-dev] [PATCH] fsck.f2fs: write checkpoint with OPU mode >>>> >>>> Hi Weichao, >>>> >>>> This patch breaks the image found by my local power-cut tests. >>>> >>>> On 05/24, Chao Yu wrote: >>>>> This original patch was from Weichao Guo. >>>>> >>>>> We may encounter both checkpoints invalid in such a case: >>>>> 1. kernel writes CP A; >>>>> 2. power-cut when kernel writes CP B, then CP B is corrupted; >>>>> 3. fsck: load CP A, fix meta/data; >>>> >>>> Would it be better to copy CP A to CP B position first? >>>> >>>> Thanks, >>>> >>>>> 4. power-cut when fsck writes CP A in-place, then CP A is corrupted too; >>>>> >>>>> To avoid both checkpoints being invalid, this patch changes to enables >>>>> fsck to write checkpoint with out-place-update method first, and then >>>>> write checkpoint in original place. >>>>> >>>>> This can make sure during fsck repairing, even there is sudden power-cut, >>>>> filesystem will still have at least one valid checkpoint. >>>>> >>>>> Signed-off-by: Weichao Guo <guoweichao@huawei.com> >>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>>>> --- >>>>> v2: >>>>> - clean up codes >>>>> - cover flush_journal_entries() case >>>>> - update commet message >>>>> fsck/fsck.c | 17 +++++++++++++++-- >>>>> fsck/fsck.h | 1 + >>>>> fsck/mount.c | 15 ++++++++++++++- >>>>> 3 files changed, 30 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/fsck/fsck.c b/fsck/fsck.c >>>>> index 6f0f262..6aed51d 100644 >>>>> --- a/fsck/fsck.c >>>>> +++ b/fsck/fsck.c >>>>> @@ -2121,6 +2121,19 @@ static void fix_checkpoint(struct f2fs_sb_info *sbi) >>>>> write_nat_bits(sbi, sb, cp, sbi->cur_cp); >>>>> } >>>>> >>>>> +static void fix_checkpoints(struct f2fs_sb_info *sbi) >>>>> +{ >>>>> + int i, ret; >>>>> + >>>>> + for (i = 0; i < 2; i++) { >>>>> + /* write checkpoint out of place first */ >>>>> + sbi->cur_cp = sbi->cur_cp % 2 + 1; >>>>> + fix_checkpoint(sbi); >>>>> + ret = f2fs_fsync_device(); >>>>> + ASSERT(ret >= 0); >>>>> + } >>>>> +} >>>>> + >>>>> int check_curseg_offset(struct f2fs_sb_info *sbi, int type) >>>>> { >>>>> struct curseg_info *curseg = CURSEG_I(sbi, type); >>>>> @@ -2771,10 +2784,10 @@ int fsck_verify(struct f2fs_sb_info *sbi) >>>>> rewrite_sit_area_bitmap(sbi); >>>>> fix_curseg_info(sbi); >>>>> fix_checksum(sbi); >>>>> - fix_checkpoint(sbi); >>>>> + fix_checkpoints(sbi); >>>>> } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) || >>>>> is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) { >>>>> - write_checkpoint(sbi); >>>>> + write_checkpoints(sbi); >>>>> } >>>>> } >>>>> return ret; >>>>> diff --git a/fsck/fsck.h b/fsck/fsck.h >>>>> index d38e8de..8fe5db1 100644 >>>>> --- a/fsck/fsck.h >>>>> +++ b/fsck/fsck.h >>>>> @@ -192,6 +192,7 @@ extern void move_curseg_info(struct f2fs_sb_info *, u64, int); >>>>> extern void write_curseg_info(struct f2fs_sb_info *); >>>>> extern int find_next_free_block(struct f2fs_sb_info *, u64 *, int, int); >>>>> extern void write_checkpoint(struct f2fs_sb_info *); >>>>> +extern void write_checkpoints(struct f2fs_sb_info *); >>>>> extern void update_superblock(struct f2fs_super_block *, int); >>>>> extern void update_data_blkaddr(struct f2fs_sb_info *, nid_t, u16, block_t); >>>>> extern void update_nat_blkaddr(struct f2fs_sb_info *, nid_t, nid_t, block_t); >>>>> diff --git a/fsck/mount.c b/fsck/mount.c >>>>> index 1c5cd93..bbb1af7 100644 >>>>> --- a/fsck/mount.c >>>>> +++ b/fsck/mount.c >>>>> @@ -2127,7 +2127,7 @@ void flush_journal_entries(struct f2fs_sb_info *sbi) >>>>> int n_sits = flush_sit_journal_entries(sbi); >>>>> >>>>> if (n_nats || n_sits) >>>>> - write_checkpoint(sbi); >>>>> + write_checkpoints(sbi); >>>>> } >>>>> >>>>> void flush_sit_entries(struct f2fs_sb_info *sbi) >>>>> @@ -2452,6 +2452,19 @@ void write_checkpoint(struct f2fs_sb_info *sbi) >>>>> ASSERT(ret >= 0); >>>>> } >>>>> >>>>> +void write_checkpoints(struct f2fs_sb_info *sbi) >>>>> +{ >>>>> + int i, ret; >>>>> + >>>>> + for (i = 0; i < 2; i++) { >>>>> + /* write checkpoint out of place first */ >>>>> + sbi->cur_cp = sbi->cur_cp % 2 + 1; >>>>> + write_checkpoint(sbi); >>>>> + ret = f2fs_fsync_device(); >>>>> + ASSERT(ret >= 0); >>>>> + } >>>>> +} >>>>> + >>>>> void build_nat_area_bitmap(struct f2fs_sb_info *sbi) >>>>> { >>>>> struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA); >>>>> -- >>>>> 2.18.0.rc1 >>>> >>>> >>>> _______________________________________________ >>>> Linux-f2fs-devel mailing list >>>> Linux-f2fs-devel@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel >>> >>> >>> _______________________________________________ >>> Linux-f2fs-devel mailing list >>> Linux-f2fs-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel >>> > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-25 1:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-24 7:56 [PATCH] fsck.f2fs: write checkpoint with OPU mode Chao Yu
2019-06-22 21:46 ` [f2fs-dev] " Jaegeuk Kim
[not found] ` <MWHPR02MB26710762B08C9EAB74BB2FABC6E00@MWHPR02MB2671.namprd02.prod.outlook.com>
2019-06-24 2:24 ` [f2fs-dev] 回复: " Chao Yu
2019-06-24 14:36 ` Chao Yu
2019-06-24 16:02 ` Jaegeuk Kim
2019-06-25 1:59 ` Chao Yu
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.