Re: [f2fs-dev] [PATCH 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack
On 09/19, Junling Zheng wrote: > From: Chao Yu > > If sb checksum is not enabled, and cp pack is valid due to no > crc inconsistence, let's try to recover cp_payload based on > cp_pack_start_sum in cp pack. > > Signed-off-by: Chao Yu > --- > fsck/f2fs.h | 5 + > fsck/mount.c | 10 +++--- > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/fsck/f2fs.h b/fsck/f2fs.h > index d216444..0d0d5e2 100644 > --- a/fsck/f2fs.h > +++ b/fsck/f2fs.h > @@ -259,6 +259,11 @@ static inline unsigned long __bitmap_size(struct > f2fs_sb_info *sbi, int flag) > return 0; > } > > +static inline block_t __cp_payload(struct f2fs_sb_info *sbi) > +{ > + return le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload); > +} > + > static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag) > { > struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi); > diff --git a/fsck/mount.c b/fsck/mount.c > index 9019921..0e8fa41 100644 > --- a/fsck/mount.c > +++ b/fsck/mount.c > @@ -975,12 +975,16 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi) > } > > cp_pack_start_sum = __start_sum_addr(sbi); > - cp_payload = get_sb(cp_payload); > + cp_payload = __cp_payload(sbi); > if (cp_pack_start_sum < cp_payload + 1 || > cp_pack_start_sum > blocks_per_seg - 1 - > NR_CURSEG_TYPE) { > - MSG(0, "\tWrong cp_pack_start_sum(%u)\n", cp_pack_start_sum); > - return 1; > + MSG(0, "\tWrong cp_pack_start_sum(%u) or cp_payload(%u)\n", > + cp_pack_start_sum, cp_payload); > + if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM)) > + return 1; Is this trying to fix superblock? Why is it related to SB_CHKSUM? > + set_sb(cp_payload, cp_pack_start_sum - 1); > + update_superblock(sb, SB_ALL); > } > > return 0; > -- > 2.19.0 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack
On 2018/9/20 7:38, Jaegeuk Kim wrote: > On 09/19, Junling Zheng wrote: >> From: Chao Yu >> >> If sb checksum is not enabled, and cp pack is valid due to no >> crc inconsistence, let's try to recover cp_payload based on >> cp_pack_start_sum in cp pack. >> >> Signed-off-by: Chao Yu >> --- >> fsck/f2fs.h | 5 + >> fsck/mount.c | 10 +++--- >> 2 files changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/fsck/f2fs.h b/fsck/f2fs.h >> index d216444..0d0d5e2 100644 >> --- a/fsck/f2fs.h >> +++ b/fsck/f2fs.h >> @@ -259,6 +259,11 @@ static inline unsigned long __bitmap_size(struct >> f2fs_sb_info *sbi, int flag) >> return 0; >> } >> >> +static inline block_t __cp_payload(struct f2fs_sb_info *sbi) >> +{ >> +return le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload); >> +} >> + >> static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag) >> { >> struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi); >> diff --git a/fsck/mount.c b/fsck/mount.c >> index 9019921..0e8fa41 100644 >> --- a/fsck/mount.c >> +++ b/fsck/mount.c >> @@ -975,12 +975,16 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi) >> } >> >> cp_pack_start_sum = __start_sum_addr(sbi); >> -cp_payload = get_sb(cp_payload); >> +cp_payload = __cp_payload(sbi); >> if (cp_pack_start_sum < cp_payload + 1 || >> cp_pack_start_sum > blocks_per_seg - 1 - >> NR_CURSEG_TYPE) { >> -MSG(0, "\tWrong cp_pack_start_sum(%u)\n", cp_pack_start_sum); >> -return 1; >> +MSG(0, "\tWrong cp_pack_start_sum(%u) or cp_payload(%u)\n", >> +cp_pack_start_sum, cp_payload); >> +if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM)) >> +return 1; > > Is this trying to fix superblock? Why is it related to SB_CHKSUM? > If sb_checksum is enabled, we should not try to fix superblock. In this situation, both sb and cp are verified with crc, so we don't know which one, cp_payload or cp_pack_start_sum, is the exact one. By contraries, if sb_checksum is disabled, we could try to fix superblock as below :) >> +set_sb(cp_payload, cp_pack_start_sum - 1); >> +update_superblock(sb, SB_ALL); >> } >> >> return 0; >> -- >> 2.19.0 > > . > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 5/5] fsck.f2fs: try to recover cp_payload from valid cp pack
On 2018/9/20 10:38, Junling Zheng wrote: > On 2018/9/20 7:38, Jaegeuk Kim wrote: >> On 09/19, Junling Zheng wrote: >>> From: Chao Yu >>> >>> If sb checksum is not enabled, and cp pack is valid due to no >>> crc inconsistence, let's try to recover cp_payload based on >>> cp_pack_start_sum in cp pack. >>> >>> Signed-off-by: Chao Yu >>> --- >>> fsck/f2fs.h | 5 + >>> fsck/mount.c | 10 +++--- >>> 2 files changed, 12 insertions(+), 3 deletions(-) >>> >>> diff --git a/fsck/f2fs.h b/fsck/f2fs.h >>> index d216444..0d0d5e2 100644 >>> --- a/fsck/f2fs.h >>> +++ b/fsck/f2fs.h >>> @@ -259,6 +259,11 @@ static inline unsigned long __bitmap_size(struct >>> f2fs_sb_info *sbi, int flag) >>> return 0; >>> } >>> >>> +static inline block_t __cp_payload(struct f2fs_sb_info *sbi) >>> +{ >>> + return le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload); >>> +} >>> + >>> static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag) >>> { >>> struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi); >>> diff --git a/fsck/mount.c b/fsck/mount.c >>> index 9019921..0e8fa41 100644 >>> --- a/fsck/mount.c >>> +++ b/fsck/mount.c >>> @@ -975,12 +975,16 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi) >>> } >>> >>> cp_pack_start_sum = __start_sum_addr(sbi); >>> - cp_payload = get_sb(cp_payload); >>> + cp_payload = __cp_payload(sbi); >>> if (cp_pack_start_sum < cp_payload + 1 || >>> cp_pack_start_sum > blocks_per_seg - 1 - >>> NR_CURSEG_TYPE) { >>> - MSG(0, "\tWrong cp_pack_start_sum(%u)\n", cp_pack_start_sum); >>> - return 1; >>> + MSG(0, "\tWrong cp_pack_start_sum(%u) or cp_payload(%u)\n", >>> + cp_pack_start_sum, cp_payload); >>> + if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM)) >>> + return 1; >> >> Is this trying to fix superblock? Why is it related to SB_CHKSUM? >> > > If sb_checksum is enabled, we should not try to fix superblock. > In this situation, both sb and cp are verified with crc, so we > don't know which one, cp_payload or cp_pack_start_sum, is the > exact one. > > By contraries, if sb_checksum is disabled, we could try to fix > superblock as below :) Thanks for helping explanation, Junling. That's true, w/o sb checksum feature, sb data is run out-of-protection, instead, checkpoint pack data is always protected by checksum, so I expect cp pack data is more trusty, that's why I just do recovery based on cp pack's data. If sb checksum feature is on, and checksum in SB and CP are all correct, we still do not know which data is more trusty, so I just leave such case for now. Thanks, > >>> + set_sb(cp_payload, cp_pack_start_sum - 1); >>> + update_superblock(sb, SB_ALL); >>> } >>> >> return 0; >>> -- >>> 2.19.0 >> >> . >> > > > > . > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel