On 2018/4/13 8:37, Jaegeuk Kim wrote:
> On 04/10, heyunlei wrote:
>>
>>
>>> -----Original Message-----
>>> From: Jaegeuk Kim [mailto:[email protected]]
>>> Sent: Tuesday, April 10, 2018 12:01 PM
>>> To: heyunlei
>>> Cc: Yuchao (T); [email protected]; 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 <[email protected]>
>>>> ---
>>>> 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?
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
