From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chao Yu Subject: Re: [PATCH] fsck.f2fs: recover nat bits feature default by fsck Date: Fri, 13 Apr 2018 09:20:16 +0800 Message-ID: References: <1523244881-13125-1-git-send-email-heyunlei@huawei.com> <20180410040047.GD47598@jaegeuk-macbookpro.roam.corp.google.com> <42B685BFA705F94C860C6DD0752F056548363846@DGGEMA503-MBS.china.huawei.com> <20180413003706.GA51348@jaegeuk-macbookpro.roam.corp.google.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1f6nOR-0002R5-Q3 for linux-f2fs-devel@lists.sourceforge.net; Fri, 13 Apr 2018 01:20:47 +0000 Received: from szxga04-in.huawei.com ([45.249.212.190] helo=huawei.com) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) id 1f6nON-004HX0-TB for linux-f2fs-devel@lists.sourceforge.net; Fri, 13 Apr 2018 01:20:47 +0000 In-Reply-To: <20180413003706.GA51348@jaegeuk-macbookpro.roam.corp.google.com> Content-Language: en-US List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net To: Jaegeuk Kim , heyunlei Cc: "Zhangdianfang (Euler)" , "linux-f2fs-devel@lists.sourceforge.net" On 2018/4/13 8:37, Jaegeuk Kim wrote: > On 04/10, heyunlei wrote: >> >> >>> -----Original Message----- >>> From: Jaegeuk Kim [mailto:jaegeuk@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 >>>> --- >>>> 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