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

2018-04-13 Thread Chao Yu
On 2018/4/13 11:59, Jaegeuk Kim wrote:
> 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 
>>>>>> ---
>>>>>>  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.

We need a more clear flag to indicate that filesystem is corrupted to decide
blocking discard, instead of using current flag which includes a state indicate
losing nat_bits.

So what's your opinion? keep as it is?

Thanks,

> 
> 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


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

2018-04-12 Thread Jaegeuk Kim
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 
> >>>> ---
> >>>>  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


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

2018-04-12 Thread Chao Yu
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 
>>>> ---
>>>>  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
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


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

2018-04-12 Thread Jaegeuk Kim
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 
> >> ---
> >>  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.

> 
> >
> >> +  }
> >>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


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

2018-04-10 Thread heyunlei


>-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 
>> ---
>>  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
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


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

2018-04-09 Thread heyunlei


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

>
>> +}
>>  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


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

2018-04-09 Thread Jaegeuk Kim
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?

> + }
>   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


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

2018-04-09 Thread Chao Yu
On 2018/4/9 11:34, 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 

Reviewed-by: Chao Yu 

Thanks,


--
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