All of lore.kernel.org
 help / color / mirror / Atom feed
* [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
* 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.