Re: [PATCH] fsck.f2fs: recover nat bits feature default by fsck

[heyunlei <heyunlei@huawei.com>]

>-----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 <heyunlei@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?

How about patch as below:

@@ -1055,6 +1055,7 @@ enum {
        SBI_POR_DOING,                          /* recovery is doing or not */
        SBI_NEED_SB_WRITE,                      /* need to recover superblock */
        SBI_NEED_CP,                            /* need to checkpoint */
+       SBI_DISABLE_NAT_BITS,                   /* disable nat bits temporay */
 };

 enum {
@@ -1517,11 +1518,10 @@ static inline void disable_nat_bits(struct f2fs_sb_info *sbi, bool lock)
 {
        unsigned long flags;

-       set_sbi_flag(sbi, SBI_NEED_FSCK);
+       set_sbi_flag(sbi, SBI_DISABLE_NAT_BITS);

        if (lock)
                spin_lock_irqsave(&sbi->cp_lock, flags);
-       __clear_ckpt_flags(F2FS_CKPT(sbi), CP_NAT_BITS_FLAG);
        kfree(NM_I(sbi)->nat_bits);
        NM_I(sbi)->nat_bits = NULL;
        if (lock)
@@ -1531,7 +1531,8 @@ static inline void disable_nat_bits(struct f2fs_sb_info *sbi, bool lock)
 static inline bool enabled_nat_bits(struct f2fs_sb_info *sbi,
                                        struct cp_control *cpc)
 {
-       bool set = is_set_ckpt_flags(sbi, CP_NAT_BITS_FLAG);
+       bool set = is_set_ckpt_flags(sbi, CP_NAT_BITS_FLAG) &&
+                       !is_sbi_flag_set(sbi, SBI_DISABLE_NAT_BITS);

        return (cpc) ? (cpc->reason & CP_UMOUNT) && set : set;
 }

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