Re: [f2fs-dev] 回复: [PATCH] fsck.f2fs: write checkpoint with OPU mode

[Chao Yu <yuchao0@huawei.com>]

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