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 <[email protected]> > >> *发送时间:* 2019年6月23日 5:46 > >> *收件人:* Chao Yu > >> *抄送:* [email protected] > >> *主题:* 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 <[email protected]> > >>> Signed-off-by: Chao Yu <[email protected]> > >>> --- > >>> 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 > >> [email protected] > >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > > > > > _______________________________________________ > > Linux-f2fs-devel mailing list > > [email protected] > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > > _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
