On 04/13, Chao Yu wrote: > On 2018/4/13 8:37, Jaegeuk Kim wrote: > > On 04/10, heyunlei wrote: > >> > >> > >>> -----Original Message----- > >>> From: Jaegeuk Kim [mailto:jaeg...@kernel.org] > >>> Sent: Tuesday, April 10, 2018 12:01 PM > >>> To: heyunlei > >>> Cc: Yuchao (T); linux-f2fs-devel@lists.sourceforge.net; Wangbintian; > >>> Zhangdianfang (Euler) > >>> Subject: Re: [f2fs-dev][PATCH] fsck.f2fs: recover nat bits feature > >>> default by fsck > >>> > >>> On 04/09, Yunlei He wrote: > >>>> Now, nat bits feature is enabled by default, we will > >>>> meet with the following scenarios: > >>>> > >>>> i. disabled, without CP_NAT_BITS_FLAG, if fsck find some > >>>> fs errors, fix or write new checkpoint will then enable it. > >>>> ii. enabled, with CP_NAT_BITS_FLAG, in the case of sudden > >>>> power off, bitmap will get lost but CP_NAT_BITS_FLAG > >>>> still exist, fsck will recover bitmap in f2fs_do_mount. > >>>> iii. enabled, with CP_NAT_BITS_FLAG, both of bitmap and > >>>> CP_NAT_BITS_FLAG will get lost if not enough space for > >>>> nat bits or nat bits check failed during mounting. > >>>> SBI_NEED_FSCK is set, fsck will recover flag and bitmap > >>>> before next mount. > >>>> > >>>> SBI_NEED_FSCK means fs is corrupted, is not suitable for > >>>> nat bits disabled. This patch try to recover nat bits all > >>>> by fsck, no need set SBI_NEED_FSCK flag in kernel. > >>>> > >>>> Signed-off-by: Yunlei He <heyun...@huawei.com> > >>>> --- > >>>> fsck/mount.c | 15 ++++++++++----- > >>>> 1 file changed, 10 insertions(+), 5 deletions(-) > >>>> > >>>> diff --git a/fsck/mount.c b/fsck/mount.c > >>>> index e5574c5..2361ee0 100644 > >>>> --- a/fsck/mount.c > >>>> +++ b/fsck/mount.c > >>>> @@ -2389,7 +2389,7 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi) > >>>> } > >>>> > >>>> /* Check nat_bits */ > >>>> - if (c.func != DUMP && is_set_ckpt_flags(cp, CP_NAT_BITS_FLAG)) { > >>>> + if (c.func != DUMP) { > >>>> u_int32_t nat_bits_bytes, nat_bits_blocks; > >>>> __le64 *kaddr; > >>>> u_int32_t blk; > >>>> @@ -2406,10 +2406,15 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi) > >>>> kaddr = malloc(PAGE_SIZE); > >>>> ret = dev_read_block(kaddr, blk); > >>>> ASSERT(ret >= 0); > >>>> - if (*kaddr != get_cp_crc(cp)) > >>>> - write_nat_bits(sbi, sb, cp, sbi->cur_cp); > >>>> - else > >>>> - MSG(0, "Info: Found valid nat_bits in > >>>> checkpoint\n"); > >>>> + if(is_set_ckpt_flags(cp, CP_NAT_BITS_FLAG)) { > >>>> + if (*kaddr != get_cp_crc(cp)) > >>>> + write_nat_bits(sbi, sb, cp, > >>>> sbi->cur_cp); > >>>> + else > >>>> + MSG(0, "Info: Found valid nat_bits in > >>>> checkpoint\n"); > >>>> + } else if (c.func == FSCK){ > >>>> + ASSERT_MSG("Need to recover nat_bits."); > >>>> + c.fix_on = 1; > >>> > >>> What if kernel doesn't support this? > >> > >> Fix or write checkpoint now will enable nat bits by default if cp space is > >> enough, > >> So maybe it will not affect kernel not supporting nat bits? > > > > I don't think we really need this, since it mixes up whole thing. > > IIUC, Yunlei just want use a flag to detect *real* data corruption on-line, > then > it can be a condition to end up issuing discard from background/umount/fstrim > to > prevent further data losing. > > For that, as I suggested before, we can split in-memory SBI_NEED_FSCK to > SBI_LOSE_NAT_BIT and SBI_NEED_FSCK, for backward compatibility, on-disk flag > can > still be old one as below: > > update_ckpt_flags() > > if (SBI_NEED_FSCK || SBI_LOSE_NAT_BIT) > set_cp_flag(CP_FSCK_FLAG) > > How about that?
I don't think we need to add more complexity here which will give another bug later. Blocking discards would make sense tho, I don't think nat_bits should be together with it. Thanks, > > Thanks, > > > > >> > >>> > >>>> + } > >>>> free(kaddr); > >>>> } > >>>> return 0; > >>>> -- > >>>> 1.9.1 > > > > . > > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
