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