| Message ID | 20241111085058.4136077-1-shengyong@oppo.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [f2fs-dev,1/2] f2fs: fix changing cursegs if recovery fails on zoned device | expand |
On 2024/11/11 16:50, Sheng Yong wrote: > Fsync data recovery attempts to check and fix write pointer consistency > of cursegs and all other zones. If the write pointers of cursegs are > unaligned, cursegs are changed to new sections. > > If recovery fails, zone write pointers are still checked and fixed, > but the latest checkpoint cannot be written back. Additionally, retry- > mount skips recovery and rolls back to reuse the old cursegs whose > zones are already finished. This can lead to unaligned write later. > > This patch addresses the issue by leaving writer pointers untouched if > recovery fails. When retry-mount is performed, cursegs and other zones > are checked and fixed after skipping recovery. > > Signed-off-by: Song Feng <songfeng@oppo.com> > Signed-off-by: Yongpeng Yang <yangyongpeng1@oppo.com> > Signed-off-by: Sheng Yong <shengyong@oppo.com> > --- > fs/f2fs/recovery.c | 2 +- > fs/f2fs/super.c | 11 ++++++++--- > 2 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c > index e4d81b8705d1..324f948247ae 100644 > --- a/fs/f2fs/recovery.c > +++ b/fs/f2fs/recovery.c > @@ -899,7 +899,7 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only) > * and the f2fs is not read only, check and fix zoned block devices' > * write pointer consistency. > */ > - if (f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sbi->sb)) { > + if (!err && f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sbi->sb)) { > int err2 = f2fs_fix_curseg_write_pointer(sbi); > > if (!err2) > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 87ab5696bd48..42224c71ae20 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -4738,13 +4738,18 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) > reset_checkpoint: > /* > * If the f2fs is not readonly and fsync data recovery succeeds, > - * check zoned block devices' write pointer consistency. > + * write pointer consistency of cursegs and other zones are already > + * checked and fixed during recovery. However, if recovery fails, > + * write pointers are left untouched, and retry-mount should check > + * them here. > */ > - if (f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sb)) { > + if (skip_recovery && f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sb)) { > int err2; > > f2fs_notice(sbi, "Checking entire write pointers"); > - err2 = f2fs_check_write_pointer(sbi); > + err2 = f2fs_fix_curseg_write_pointer(sbi); > + if (!err2) > + err2 = f2fs_check_write_pointer(sbi); > if (err2) > err = err2; What about wrapping above logic into f2fs_fix_and_check_write_pointer() and reuse in f2fs_fill_super() and f2fs_recover_fsync_data()? int f2fs_fix_and_check_write_pointer() { int err; if (!f2fs_sb_has_blkzoned(sbi) || f2fs_readonly(sbi->sb)) return 0; err = f2fs_fix_curseg_write_pointer(sbi); if (!err) err = f2fs_check_write_pointer(sbi); return err; } Thanks, > }
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c index e4d81b8705d1..324f948247ae 100644 --- a/fs/f2fs/recovery.c +++ b/fs/f2fs/recovery.c @@ -899,7 +899,7 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only) * and the f2fs is not read only, check and fix zoned block devices' * write pointer consistency. */ - if (f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sbi->sb)) { + if (!err && f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sbi->sb)) { int err2 = f2fs_fix_curseg_write_pointer(sbi); if (!err2) diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 87ab5696bd48..42224c71ae20 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -4738,13 +4738,18 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) reset_checkpoint: /* * If the f2fs is not readonly and fsync data recovery succeeds, - * check zoned block devices' write pointer consistency. + * write pointer consistency of cursegs and other zones are already + * checked and fixed during recovery. However, if recovery fails, + * write pointers are left untouched, and retry-mount should check + * them here. */ - if (f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sb)) { + if (skip_recovery && f2fs_sb_has_blkzoned(sbi) && !f2fs_readonly(sb)) { int err2; f2fs_notice(sbi, "Checking entire write pointers"); - err2 = f2fs_check_write_pointer(sbi); + err2 = f2fs_fix_curseg_write_pointer(sbi); + if (!err2) + err2 = f2fs_check_write_pointer(sbi); if (err2) err = err2; }