Re: [f2fs-dev] [PATCH] fsck.f2fs: detect and recover corrupted quota file

2018-09-19 Thread Chao Yu
Hi Sheng,

On 2018/9/19 9:48, Sheng Yong wrote:
> Hi, Chao
> 
> On 2018/9/19 9:28, Chao Yu wrote:
>> Once quota file is corrupted, kernel will set CP_QUOTA_NEED_FSCK_FLAG
>> into checkpoint pack, this patch makes fsck supporting to detect the flag
>> and try to rebuild corrupted quota file.
>>
>> Signed-off-by: Chao Yu 
>> ---
>>   fsck/fsck.c   |  3 ++-
>>   fsck/main.c   |  1 +
>>   fsck/mount.c  | 11 +--
>>   include/f2fs_fs.h |  2 ++
>>   4 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>> index 40b95f7054a2..28c913b83355 100644
>> --- a/fsck/fsck.c
>> +++ b/fsck/fsck.c
>> @@ -2670,7 +2670,8 @@ int fsck_verify(struct f2fs_sb_info *sbi)
>>  flush_curseg_sit_entries(sbi);
>>  }
>>  fix_checkpoint(sbi);
>> -} else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG)) {
>> +} else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) ||
>> +is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) {
>>  write_checkpoint(sbi);
>>  }
>>  }
>> diff --git a/fsck/main.c b/fsck/main.c
>> index 714e28a509b9..e20f31ca09e4 100644
>> --- a/fsck/main.c
>> +++ b/fsck/main.c
>> @@ -162,6 +162,7 @@ static void add_default_options(void)
>>  case CONF_ANDROID:
>>  __add_fsck_options();
>>  }
>> +c.quota_fix = 1;
>>   }
>>   
>>   void f2fs_parse_options(int argc, char *argv[])
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index 6a3382dbd449..21a39a7222c6 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -405,6 +405,8 @@ void print_ckpt_info(struct f2fs_sb_info *sbi)
>>   void print_cp_state(u32 flag)
>>   {
>>  MSG(0, "Info: checkpoint state = %x : ", flag);
>> +if (flag & CP_QUOTA_NEED_FSCK_FLAG)
>> +MSG(0, "%s", " quota_need_fsck");
>>  if (flag & CP_LARGE_NAT_BITMAP_FLAG)
>>  MSG(0, "%s", " large_nat_bitmap");
>>  if (flag & CP_NOCRC_RECOVERY_FLAG)
>> @@ -2541,12 +2543,17 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
>>   
>>  print_ckpt_info(sbi);
>>   
>> +if (c.quota_fix) {
>> +if (get_cp(ckpt_flags) & CP_QUOTA_NEED_FSCK_FLAG)
>> +c.fix_on = 1;
>> +}
>> +
> 
> I think we don't need the quota_fix, if -f is set, fsck will always check
> quota files. If anything wrong, fix_on will be set.

Yeah, since kernel will be aware of quota file corruption under journalled
mode, so I prefer to let fsck to check the CP_QUOTA_NEED_FSCK_FLAG by
default and once that flag is set, then do repairing automatically, like we
did for CP_FSCK_FLAG. By this way we can avoid traversing all data to check
quota info.

So how about just turning o c.fix_on under -a or -p 0 mode? Since in
android environment, auto_fix is enabled by default, we can expect above
design can work.

Thanks,

> 
> thanks,
> 
>>  if (c.auto_fix || c.preen_mode) {
>>  u32 flag = get_cp(ckpt_flags);
>>   
>>  if (flag & CP_FSCK_FLAG ||
>> -(exist_qf_ino(sb) && (!(flag & CP_UMOUNT_FLAG) ||
>> -flag & CP_ERROR_FLAG))) {
>> +flag & CP_QUOTA_NEED_FSCK_FLAG ||
>> +(exist_qf_ino(sb) && (flag & CP_ERROR_FLAG))) {
>>  c.fix_on = 1;
>>  } else if (!c.preen_mode) {
>>  print_cp_state(flag);
>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>> index 9396c785a254..160eaf72f0b6 100644
>> --- a/include/f2fs_fs.h
>> +++ b/include/f2fs_fs.h
>> @@ -372,6 +372,7 @@ struct f2fs_configuration {
>>  int defset;
>>  int bug_on;
>>  int auto_fix;
>> +int quota_fix;
>>  int preen_mode;
>>  int ro;
>>  int preserve_limits;/* preserve quota limits */
>> @@ -641,6 +642,7 @@ struct f2fs_super_block {
>>   /*
>>* For checkpoint
>>*/
>> +#define CP_QUOTA_NEED_FSCK_FLAG 0x0800
>>   #define CP_LARGE_NAT_BITMAP_FLAG   0x0400
>>   #define CP_NOCRC_RECOVERY_FLAG 0x0200
>>   #define CP_TRIMMED_FLAG0x0100
>>
> 
> 
> 
> ___
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> 



___
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: detect and recover corrupted quota file

2018-09-18 Thread Sheng Yong

Hi, Chao

On 2018/9/19 9:28, Chao Yu wrote:

Once quota file is corrupted, kernel will set CP_QUOTA_NEED_FSCK_FLAG
into checkpoint pack, this patch makes fsck supporting to detect the flag
and try to rebuild corrupted quota file.

Signed-off-by: Chao Yu 
---
  fsck/fsck.c   |  3 ++-
  fsck/main.c   |  1 +
  fsck/mount.c  | 11 +--
  include/f2fs_fs.h |  2 ++
  4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index 40b95f7054a2..28c913b83355 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -2670,7 +2670,8 @@ int fsck_verify(struct f2fs_sb_info *sbi)
flush_curseg_sit_entries(sbi);
}
fix_checkpoint(sbi);
-   } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG)) {
+   } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) ||
+   is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) {
write_checkpoint(sbi);
}
}
diff --git a/fsck/main.c b/fsck/main.c
index 714e28a509b9..e20f31ca09e4 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -162,6 +162,7 @@ static void add_default_options(void)
case CONF_ANDROID:
__add_fsck_options();
}
+   c.quota_fix = 1;
  }
  
  void f2fs_parse_options(int argc, char *argv[])

diff --git a/fsck/mount.c b/fsck/mount.c
index 6a3382dbd449..21a39a7222c6 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -405,6 +405,8 @@ void print_ckpt_info(struct f2fs_sb_info *sbi)
  void print_cp_state(u32 flag)
  {
MSG(0, "Info: checkpoint state = %x : ", flag);
+   if (flag & CP_QUOTA_NEED_FSCK_FLAG)
+   MSG(0, "%s", " quota_need_fsck");
if (flag & CP_LARGE_NAT_BITMAP_FLAG)
MSG(0, "%s", " large_nat_bitmap");
if (flag & CP_NOCRC_RECOVERY_FLAG)
@@ -2541,12 +2543,17 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
  
  	print_ckpt_info(sbi);
  
+	if (c.quota_fix) {

+   if (get_cp(ckpt_flags) & CP_QUOTA_NEED_FSCK_FLAG)
+   c.fix_on = 1;
+   }
+


I think we don't need the quota_fix, if -f is set, fsck will always check
quota files. If anything wrong, fix_on will be set.

thanks,


if (c.auto_fix || c.preen_mode) {
u32 flag = get_cp(ckpt_flags);
  
  		if (flag & CP_FSCK_FLAG ||

-   (exist_qf_ino(sb) && (!(flag & CP_UMOUNT_FLAG) ||
-   flag & CP_ERROR_FLAG))) {
+   flag & CP_QUOTA_NEED_FSCK_FLAG ||
+   (exist_qf_ino(sb) && (flag & CP_ERROR_FLAG))) {
c.fix_on = 1;
} else if (!c.preen_mode) {
print_cp_state(flag);
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 9396c785a254..160eaf72f0b6 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -372,6 +372,7 @@ struct f2fs_configuration {
int defset;
int bug_on;
int auto_fix;
+   int quota_fix;
int preen_mode;
int ro;
int preserve_limits;/* preserve quota limits */
@@ -641,6 +642,7 @@ struct f2fs_super_block {
  /*
   * For checkpoint
   */
+#define CP_QUOTA_NEED_FSCK_FLAG0x0800
  #define CP_LARGE_NAT_BITMAP_FLAG  0x0400
  #define CP_NOCRC_RECOVERY_FLAG0x0200
  #define CP_TRIMMED_FLAG   0x0100





___
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: detect and recover corrupted quota file

2018-08-29 Thread Chao Yu
Hi Sheng,

On 2018/8/29 23:06, Sheng Yong wrote:
> Hi, Chao
> 
> On 2018/8/29 20:09, Chao Yu wrote:
>> Once quota file is corrupted, kernel will set CP_QUOTA_NEED_FSCK_FLAG
>> into checkpoint pack, this patch makes fsck supporting to detect the flag
>> and try to rebuild corrupted quota file.
>>
> Do we need to drop recovery data? Recovery data is never checked by fsck,
> if fsck tries to rebuild quota, it needs to allocate blocks from cursegs,
> this may overwrite data blocks which may be recovered latter.

IMO, we'd better not to drop those data, so I prefer to support recovering
fsynced data in fsck, and do recovery in the condition of:
- needs to allocate block from log header.
- through defined option.

How do you think?

Thanks,

> 
> thanks
>> Signed-off-by: Chao Yu 
>> ---
>>   fsck/fsck.c   | 3 ++-
>>   fsck/mount.c  | 6 --
>>   include/f2fs_fs.h | 2 ++
>>   lib/libf2fs.c | 2 ++
>>   4 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>> index f080d3c8741c..10b69ef403ef 100644
>> --- a/fsck/fsck.c
>> +++ b/fsck/fsck.c
>> @@ -2668,7 +2668,8 @@ int fsck_verify(struct f2fs_sb_info *sbi)
>>   flush_curseg_sit_entries(sbi);
>>   }
>>   fix_checkpoint(sbi);
>> -    } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG)) {
>> +    } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) ||
>> +    is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) {
>>   write_checkpoint(sbi);
>>   }
>>   }
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index a2448e370c0b..168cf387991f 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -405,6 +405,8 @@ void print_ckpt_info(struct f2fs_sb_info *sbi)
>>   void print_cp_state(u32 flag)
>>   {
>>   MSG(0, "Info: checkpoint state = %x : ", flag);
>> +    if (flag & CP_QUOTA_NEED_FSCK_FLAG)
>> +    MSG(0, "%s", " quota_need_fsck");
>>   if (flag & CP_LARGE_NAT_BITMAP_FLAG)
>>   MSG(0, "%s", " large_nat_bitmap");
>>   if (flag & CP_NOCRC_RECOVERY_FLAG)
>> @@ -2532,8 +2534,8 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
>>   u32 flag = get_cp(ckpt_flags);
>>     if (flag & CP_FSCK_FLAG ||
>> -    (exist_qf_ino(sb) && (!(flag & CP_UMOUNT_FLAG) ||
>> -    flag & CP_ERROR_FLAG))) {
>> +    flag & CP_QUOTA_NEED_FSCK_FLAG ||
>> +    (exist_qf_ino(sb) && (flag & CP_ERROR_FLAG))) {
>>   c.fix_on = 1;
>>   } else if (!c.preen_mode) {
>>   print_cp_state(flag);
>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>> index 9396c785a254..160eaf72f0b6 100644
>> --- a/include/f2fs_fs.h
>> +++ b/include/f2fs_fs.h
>> @@ -372,6 +372,7 @@ struct f2fs_configuration {
>>   int defset;
>>   int bug_on;
>>   int auto_fix;
>> +    int quota_fix;
>>   int preen_mode;
>>   int ro;
>>   int preserve_limits;    /* preserve quota limits */
>> @@ -641,6 +642,7 @@ struct f2fs_super_block {
>>   /*
>>    * For checkpoint
>>    */
>> +#define CP_QUOTA_NEED_FSCK_FLAG    0x0800
>>   #define CP_LARGE_NAT_BITMAP_FLAG    0x0400
>>   #define CP_NOCRC_RECOVERY_FLAG    0x0200
>>   #define CP_TRIMMED_FLAG    0x0100
>> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
>> index a1f8beb1f78d..192b8125de36 100644
>> --- a/lib/libf2fs.c
>> +++ b/lib/libf2fs.c
>> @@ -619,6 +619,8 @@ void f2fs_init_configuration(void)
>>   /* default root owner */
>>   c.root_uid = getuid();
>>   c.root_gid = getgid();
>> +
>> +    c.quota_fix = 1;
>>   }
>>     #ifdef HAVE_SETMNTENT
>>
> 
> 
> --
> 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

--
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: detect and recover corrupted quota file

2018-08-29 Thread Sheng Yong

Hi, Chao

On 2018/8/29 20:09, Chao Yu wrote:

Once quota file is corrupted, kernel will set CP_QUOTA_NEED_FSCK_FLAG
into checkpoint pack, this patch makes fsck supporting to detect the flag
and try to rebuild corrupted quota file.


Do we need to drop recovery data? Recovery data is never checked by fsck,
if fsck tries to rebuild quota, it needs to allocate blocks from cursegs,
this may overwrite data blocks which may be recovered latter.

thanks

Signed-off-by: Chao Yu 
---
  fsck/fsck.c   | 3 ++-
  fsck/mount.c  | 6 --
  include/f2fs_fs.h | 2 ++
  lib/libf2fs.c | 2 ++
  4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index f080d3c8741c..10b69ef403ef 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -2668,7 +2668,8 @@ int fsck_verify(struct f2fs_sb_info *sbi)
flush_curseg_sit_entries(sbi);
}
fix_checkpoint(sbi);
-   } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG)) {
+   } else if (is_set_ckpt_flags(cp, CP_FSCK_FLAG) ||
+   is_set_ckpt_flags(cp, CP_QUOTA_NEED_FSCK_FLAG)) {
write_checkpoint(sbi);
}
}
diff --git a/fsck/mount.c b/fsck/mount.c
index a2448e370c0b..168cf387991f 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -405,6 +405,8 @@ void print_ckpt_info(struct f2fs_sb_info *sbi)
  void print_cp_state(u32 flag)
  {
MSG(0, "Info: checkpoint state = %x : ", flag);
+   if (flag & CP_QUOTA_NEED_FSCK_FLAG)
+   MSG(0, "%s", " quota_need_fsck");
if (flag & CP_LARGE_NAT_BITMAP_FLAG)
MSG(0, "%s", " large_nat_bitmap");
if (flag & CP_NOCRC_RECOVERY_FLAG)
@@ -2532,8 +2534,8 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
u32 flag = get_cp(ckpt_flags);
  
  		if (flag & CP_FSCK_FLAG ||

-   (exist_qf_ino(sb) && (!(flag & CP_UMOUNT_FLAG) ||
-   flag & CP_ERROR_FLAG))) {
+   flag & CP_QUOTA_NEED_FSCK_FLAG ||
+   (exist_qf_ino(sb) && (flag & CP_ERROR_FLAG))) {
c.fix_on = 1;
} else if (!c.preen_mode) {
print_cp_state(flag);
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 9396c785a254..160eaf72f0b6 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -372,6 +372,7 @@ struct f2fs_configuration {
int defset;
int bug_on;
int auto_fix;
+   int quota_fix;
int preen_mode;
int ro;
int preserve_limits;/* preserve quota limits */
@@ -641,6 +642,7 @@ struct f2fs_super_block {
  /*
   * For checkpoint
   */
+#define CP_QUOTA_NEED_FSCK_FLAG0x0800
  #define CP_LARGE_NAT_BITMAP_FLAG  0x0400
  #define CP_NOCRC_RECOVERY_FLAG0x0200
  #define CP_TRIMMED_FLAG   0x0100
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index a1f8beb1f78d..192b8125de36 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -619,6 +619,8 @@ void f2fs_init_configuration(void)
/* default root owner */
c.root_uid = getuid();
c.root_gid = getgid();
+
+   c.quota_fix = 1;
  }
  
  #ifdef HAVE_SETMNTENT





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