* [f2fs-dev] [PATCH] f2fs-tools: fix the wrong sbi->cur_cp setting @ 2021-10-25 12:09 Wang Xiaojun 2021-10-26 0:29 ` Chao Yu 0 siblings, 1 reply; 5+ messages in thread From: Wang Xiaojun @ 2021-10-25 12:09 UTC (permalink / raw) To: chao, jaegeuk; +Cc: linux-f2fs-devel If sbi->cur_cp is 2 and the duplicate_checkpoint function returns in advance because sbi->cp_backuped is set to true, we cannot set sbi->cur_cp to 1. Signed-off-by: Wang Xiaojun <wangxiaojun11@huawei.com> --- fsck/fsck.c | 3 --- fsck/mount.c | 5 ++--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/fsck/fsck.c b/fsck/fsck.c index 110c1ec..aa77a34 100644 --- a/fsck/fsck.c +++ b/fsck/fsck.c @@ -2383,9 +2383,6 @@ static void fix_checkpoints(struct f2fs_sb_info *sbi) { /* copy valid checkpoint to its mirror position */ duplicate_checkpoint(sbi); - - /* repair checkpoint at CP #0 position */ - sbi->cur_cp = 1; fix_checkpoint(sbi); } diff --git a/fsck/mount.c b/fsck/mount.c index c928a15..295170e 100644 --- a/fsck/mount.c +++ b/fsck/mount.c @@ -2998,6 +2998,8 @@ void duplicate_checkpoint(struct f2fs_sb_info *sbi) ASSERT(ret >= 0); sbi->cp_backuped = 1; + /* repair checkpoint at CP #0 position */ + sbi->cur_cp = 1; MSG(0, "Info: Duplicate valid checkpoint to mirror position " "%llu -> %llu\n", src, dst); @@ -3098,9 +3100,6 @@ void write_checkpoints(struct f2fs_sb_info *sbi) { /* copy valid checkpoint to its mirror position */ duplicate_checkpoint(sbi); - - /* repair checkpoint at CP #0 position */ - sbi->cur_cp = 1; write_checkpoint(sbi); } -- 2.31.1 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs-tools: fix the wrong sbi->cur_cp setting 2021-10-25 12:09 [f2fs-dev] [PATCH] f2fs-tools: fix the wrong sbi->cur_cp setting Wang Xiaojun @ 2021-10-26 0:29 ` Chao Yu 2021-10-26 1:38 ` Wang Xiaojun 0 siblings, 1 reply; 5+ messages in thread From: Chao Yu @ 2021-10-26 0:29 UTC (permalink / raw) To: Wang Xiaojun, jaegeuk; +Cc: linux-f2fs-devel On 2021/10/25 20:09, Wang Xiaojun wrote: > If sbi->cur_cp is 2 and the duplicate_checkpoint function returns > in advance because sbi->cp_backuped is set to true, we cannot set > sbi->cur_cp to 1. Hmmm, in previous implementation, what problem we will encounter, and what's the root cause? Thanks, > > Signed-off-by: Wang Xiaojun <wangxiaojun11@huawei.com> > --- > fsck/fsck.c | 3 --- > fsck/mount.c | 5 ++--- > 2 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/fsck/fsck.c b/fsck/fsck.c > index 110c1ec..aa77a34 100644 > --- a/fsck/fsck.c > +++ b/fsck/fsck.c > @@ -2383,9 +2383,6 @@ static void fix_checkpoints(struct f2fs_sb_info *sbi) > { > /* copy valid checkpoint to its mirror position */ > duplicate_checkpoint(sbi); > - > - /* repair checkpoint at CP #0 position */ > - sbi->cur_cp = 1; > fix_checkpoint(sbi); > } > > diff --git a/fsck/mount.c b/fsck/mount.c > index c928a15..295170e 100644 > --- a/fsck/mount.c > +++ b/fsck/mount.c > @@ -2998,6 +2998,8 @@ void duplicate_checkpoint(struct f2fs_sb_info *sbi) > ASSERT(ret >= 0); > > sbi->cp_backuped = 1; > + /* repair checkpoint at CP #0 position */ > + sbi->cur_cp = 1; > > MSG(0, "Info: Duplicate valid checkpoint to mirror position " > "%llu -> %llu\n", src, dst); > @@ -3098,9 +3100,6 @@ void write_checkpoints(struct f2fs_sb_info *sbi) > { > /* copy valid checkpoint to its mirror position */ > duplicate_checkpoint(sbi); > - > - /* repair checkpoint at CP #0 position */ > - sbi->cur_cp = 1; > write_checkpoint(sbi); > } > > _______________________________________________ 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] 5+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs-tools: fix the wrong sbi->cur_cp setting 2021-10-26 0:29 ` Chao Yu @ 2021-10-26 1:38 ` Wang Xiaojun 2021-10-26 1:47 ` Chao Yu 0 siblings, 1 reply; 5+ messages in thread From: Wang Xiaojun @ 2021-10-26 1:38 UTC (permalink / raw) To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel 在 2021/10/26 8:29, Chao Yu 写道: > On 2021/10/25 20:09, Wang Xiaojun wrote: >> If sbi->cur_cp is 2 and the duplicate_checkpoint function returns >> in advance because sbi->cp_backuped is set to true, we cannot set >> sbi->cur_cp to 1. > > Hmmm, in previous implementation, what problem we will encounter, and > what's the root cause? > > Thanks, In fact, it's not causing problems at this time. During the code review, I found that this was not logically reasonable. This parameter(sbi->cur_cp) can be set to 1 only after successful replication. Thanks, > >> >> Signed-off-by: Wang Xiaojun <wangxiaojun11@huawei.com> >> --- >> fsck/fsck.c | 3 --- >> fsck/mount.c | 5 ++--- >> 2 files changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/fsck/fsck.c b/fsck/fsck.c >> index 110c1ec..aa77a34 100644 >> --- a/fsck/fsck.c >> +++ b/fsck/fsck.c >> @@ -2383,9 +2383,6 @@ static void fix_checkpoints(struct f2fs_sb_info >> *sbi) >> { >> /* copy valid checkpoint to its mirror position */ >> duplicate_checkpoint(sbi); >> - >> - /* repair checkpoint at CP #0 position */ >> - sbi->cur_cp = 1; >> fix_checkpoint(sbi); >> } >> diff --git a/fsck/mount.c b/fsck/mount.c >> index c928a15..295170e 100644 >> --- a/fsck/mount.c >> +++ b/fsck/mount.c >> @@ -2998,6 +2998,8 @@ void duplicate_checkpoint(struct f2fs_sb_info >> *sbi) >> ASSERT(ret >= 0); >> sbi->cp_backuped = 1; >> + /* repair checkpoint at CP #0 position */ >> + sbi->cur_cp = 1; >> MSG(0, "Info: Duplicate valid checkpoint to mirror position " >> "%llu -> %llu\n", src, dst); >> @@ -3098,9 +3100,6 @@ void write_checkpoints(struct f2fs_sb_info *sbi) >> { >> /* copy valid checkpoint to its mirror position */ >> duplicate_checkpoint(sbi); >> - >> - /* repair checkpoint at CP #0 position */ >> - sbi->cur_cp = 1; >> write_checkpoint(sbi); >> } >> > . _______________________________________________ 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] 5+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs-tools: fix the wrong sbi->cur_cp setting 2021-10-26 1:38 ` Wang Xiaojun @ 2021-10-26 1:47 ` Chao Yu 2021-10-26 2:01 ` Wang Xiaojun 0 siblings, 1 reply; 5+ messages in thread From: Chao Yu @ 2021-10-26 1:47 UTC (permalink / raw) To: Wang Xiaojun, jaegeuk; +Cc: linux-f2fs-devel On 2021/10/26 9:38, Wang Xiaojun wrote: > > 在 2021/10/26 8:29, Chao Yu 写道: >> On 2021/10/25 20:09, Wang Xiaojun wrote: >>> If sbi->cur_cp is 2 and the duplicate_checkpoint function returns >>> in advance because sbi->cp_backuped is set to true, we cannot set >>> sbi->cur_cp to 1. >> >> Hmmm, in previous implementation, what problem we will encounter, and >> what's the root cause? >> >> Thanks, > > In fact, it's not causing problems at this time. > > During the code review, I found that this was not logically reasonable. > > This parameter(sbi->cur_cp) can be set to 1 only after successful replication. IIRC, after mirroring checkpoint (cp_backuped == 1), we choose to always update CP #0 area, so once SPO occurs, at least there is one valid checkpoint can be kept in CP #1 area. [1] [1] 81bad9d11ea5 ("fsck.f2fs: write checkpoint with OPU mode") Thanks, > > Thanks, > >> >>> >>> Signed-off-by: Wang Xiaojun <wangxiaojun11@huawei.com> >>> --- >>> fsck/fsck.c | 3 --- >>> fsck/mount.c | 5 ++--- >>> 2 files changed, 2 insertions(+), 6 deletions(-) >>> >>> diff --git a/fsck/fsck.c b/fsck/fsck.c >>> index 110c1ec..aa77a34 100644 >>> --- a/fsck/fsck.c >>> +++ b/fsck/fsck.c >>> @@ -2383,9 +2383,6 @@ static void fix_checkpoints(struct f2fs_sb_info *sbi) >>> { >>> /* copy valid checkpoint to its mirror position */ >>> duplicate_checkpoint(sbi); >>> - >>> - /* repair checkpoint at CP #0 position */ >>> - sbi->cur_cp = 1; >>> fix_checkpoint(sbi); >>> } >>> diff --git a/fsck/mount.c b/fsck/mount.c >>> index c928a15..295170e 100644 >>> --- a/fsck/mount.c >>> +++ b/fsck/mount.c >>> @@ -2998,6 +2998,8 @@ void duplicate_checkpoint(struct f2fs_sb_info *sbi) >>> ASSERT(ret >= 0); >>> sbi->cp_backuped = 1; >>> + /* repair checkpoint at CP #0 position */ >>> + sbi->cur_cp = 1; >>> MSG(0, "Info: Duplicate valid checkpoint to mirror position " >>> "%llu -> %llu\n", src, dst); >>> @@ -3098,9 +3100,6 @@ void write_checkpoints(struct f2fs_sb_info *sbi) >>> { >>> /* copy valid checkpoint to its mirror position */ >>> duplicate_checkpoint(sbi); >>> - >>> - /* repair checkpoint at CP #0 position */ >>> - sbi->cur_cp = 1; >>> write_checkpoint(sbi); >>> } >>> >> . _______________________________________________ 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] 5+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs-tools: fix the wrong sbi->cur_cp setting 2021-10-26 1:47 ` Chao Yu @ 2021-10-26 2:01 ` Wang Xiaojun 0 siblings, 0 replies; 5+ messages in thread From: Wang Xiaojun @ 2021-10-26 2:01 UTC (permalink / raw) To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel 在 2021/10/26 9:47, Chao Yu 写道: > On 2021/10/26 9:38, Wang Xiaojun wrote: >> >> 在 2021/10/26 8:29, Chao Yu 写道: >>> On 2021/10/25 20:09, Wang Xiaojun wrote: >>>> If sbi->cur_cp is 2 and the duplicate_checkpoint function returns >>>> in advance because sbi->cp_backuped is set to true, we cannot set >>>> sbi->cur_cp to 1. >>> >>> Hmmm, in previous implementation, what problem we will encounter, and >>> what's the root cause? >>> >>> Thanks, >> >> In fact, it's not causing problems at this time. >> >> During the code review, I found that this was not logically reasonable. >> >> This parameter(sbi->cur_cp) can be set to 1 only after successful >> replication. > > IIRC, after mirroring checkpoint (cp_backuped == 1), we choose to > always update > CP #0 area, so once SPO occurs, at least there is one valid checkpoint > can be kept > in CP #1 area. [1] > > [1] 81bad9d11ea5 ("fsck.f2fs: write checkpoint with OPU mode") > > Thanks, Got it. thanks > >> >> Thanks, >> >>> >>>> >>>> Signed-off-by: Wang Xiaojun <wangxiaojun11@huawei.com> >>>> --- >>>> fsck/fsck.c | 3 --- >>>> fsck/mount.c | 5 ++--- >>>> 2 files changed, 2 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/fsck/fsck.c b/fsck/fsck.c >>>> index 110c1ec..aa77a34 100644 >>>> --- a/fsck/fsck.c >>>> +++ b/fsck/fsck.c >>>> @@ -2383,9 +2383,6 @@ static void fix_checkpoints(struct >>>> f2fs_sb_info *sbi) >>>> { >>>> /* copy valid checkpoint to its mirror position */ >>>> duplicate_checkpoint(sbi); >>>> - >>>> - /* repair checkpoint at CP #0 position */ >>>> - sbi->cur_cp = 1; >>>> fix_checkpoint(sbi); >>>> } >>>> diff --git a/fsck/mount.c b/fsck/mount.c >>>> index c928a15..295170e 100644 >>>> --- a/fsck/mount.c >>>> +++ b/fsck/mount.c >>>> @@ -2998,6 +2998,8 @@ void duplicate_checkpoint(struct f2fs_sb_info >>>> *sbi) >>>> ASSERT(ret >= 0); >>>> sbi->cp_backuped = 1; >>>> + /* repair checkpoint at CP #0 position */ >>>> + sbi->cur_cp = 1; >>>> MSG(0, "Info: Duplicate valid checkpoint to mirror position " >>>> "%llu -> %llu\n", src, dst); >>>> @@ -3098,9 +3100,6 @@ void write_checkpoints(struct f2fs_sb_info *sbi) >>>> { >>>> /* copy valid checkpoint to its mirror position */ >>>> duplicate_checkpoint(sbi); >>>> - >>>> - /* repair checkpoint at CP #0 position */ >>>> - sbi->cur_cp = 1; >>>> write_checkpoint(sbi); >>>> } >>>> >>> . > . _______________________________________________ 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] 5+ messages in thread
end of thread, other threads:[~2021-10-26 2:02 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-25 12:09 [f2fs-dev] [PATCH] f2fs-tools: fix the wrong sbi->cur_cp setting Wang Xiaojun 2021-10-26 0:29 ` Chao Yu 2021-10-26 1:38 ` Wang Xiaojun 2021-10-26 1:47 ` Chao Yu 2021-10-26 2:01 ` Wang Xiaojun
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.