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

Reply via email to