Re: [f2fs-dev] [PATCH] Revert "resize.f2fs: add option for large_nat_bitmap feature"

[Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>]

On 2025/4/11 17:04, Juhyung Park wrote:
> Doesn't it simply remove the flag and proceed?
> 
> What happens if large nat bitmap is required (e.g., a lot of inodes)?
> 
> Sounds like we should actively detect it and stop it from proceeding,
> not just discarding the flag.

If the flag existed in old image, it won't discard it during resize, this patch
only avoid enabling large nat bitmap on image during resize via -i option.

Thanks,

> 
> On Fri, Apr 11, 2025 at 1:57 AM Chao Yu <chao@kernel.org> wrote:
>>
>> This reverts commit daa0f8b9e9b1eea6c85e5c169b809254d9d7074e.
>>
>> As Juhyung reported in [1]:
>>
>> Here's what I did:
>> 1. Remounted to checkpoint=disable
>> 2. Create a dm-snapshot of the current root device
>> 3. Mount snapshot to replay the log
>> 4. Unmount
>> 5. Resize sector 487248896 to 486539264
>> // ./resize.f2fs -d 3 -s -i /dev/mapper/snap -t 486539264
>>
>> Latest f2fs-tools was used:
>> 33c5b9539af2 f2fs_io: add fragread command to evaluate fragmented
>> buffer for reads
>>
>> This reproduced the mount and fsck failure.
>>
>> Mount log:
>> [442431.020594] F2FS-fs (dm-2): invalid crc_offset: 0
>> [442431.130055] F2FS-fs (dm-2): SIT is corrupted node# 13615201 vs 13616290
>> [442431.139684] F2FS-fs (dm-2): Failed to initialize F2FS segment manager (-117)
>>
>> I checked below testcases:
>>
>> truncate -s $((512*1024*1024*1024)) img
>> mkfs.f2fs -f img $((256*1024*1024))
>>
>> Description             Test command                                    FSCK output
>> a) shrink w/ -s         resize.f2fs -s -i img -t $((128*1024*1024))     Fine
>> b) expand w/ -s         resize.f2fs -s -i img -t $((1024*1024*1024))    Corrupted
>> c) shrink w/o -s        resize.f2fs -i img -t $((128*1024*1024))        No run
>> d) expand w/o -s        resize.f2fs -i img -t $((1024*1024*1024))       Fine
>>
>> Output from b):
>> [ASSERT] (check_block_count:2299)  --> Wrong SIT valid blocks: segno=0x29400, 0 vs. 13
>>
>> The root cause is: safe mode (-s option) is conflict w/ large nat bitmap feature
>> (-i option), since once we enable large nat bitmap, layout of checkpoint will be
>> changed [2], we must relocate nat/sit_bitmap to the end of new location of
>> cp_checksum, however in safe mode, we won't change metadata of checkpoint, so we
>> need to keep nat/sit_bitmap as it is, which includes checksum data in its old
>> location.
>>
>> Let's revert -i support for resize.f2fs first, and then reenable it after
>> fix and well tested.
>>
>> Thanks a lot for the help from Juhyung, including providing reproducer and
>> hints.
>>
>> [1] https://lore.kernel.org/linux-f2fs-devel/CAD14+f3D6iPOBxEgkZxxTsXCfwRKb9ph68JtUk3H9cn8ovLt9w@mail.gmail.com/
>> [2] https://lore.kernel.org/linux-f2fs-devel/20190514093340.40217-2-yuchao0@huawei.com/
>>
>> Fixes: daa0f8b ("resize.f2fs: add option for large_nat_bitmap feature")
>> Reported-by: Ju Hyung Park <qkrwngud825@gmail.com>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   fsck/main.c   | 6 +-----
>>   fsck/resize.c | 3 ---
>>   2 files changed, 1 insertion(+), 8 deletions(-)
>>
>> diff --git a/fsck/main.c b/fsck/main.c
>> index 524dbb1..af40613 100644
>> --- a/fsck/main.c
>> +++ b/fsck/main.c
>> @@ -141,7 +141,6 @@ void resize_usage()
>>          MSG(0, "[options]:\n");
>>          MSG(0, "  -d debug level [default:0]\n");
>>          MSG(0, "  -H support write hint\n");
>> -       MSG(0, "  -i extended node bitmap, node ratio is 20%% by default\n");
>>          MSG(0, "  -o overprovision percentage [default:auto]\n");
>>          MSG(0, "  -s safe resize (Does not resize metadata)\n");
>>          MSG(0, "  -t target sectors [default: device size]\n");
>> @@ -602,7 +601,7 @@ void f2fs_parse_options(int argc, char *argv[])
>>   #endif
>>          } else if (!strcmp("resize.f2fs", prog)) {
>>   #ifdef WITH_RESIZE
>> -               const char *option_string = "d:fHst:io:V";
>> +               const char *option_string = "d:fHst:o:V";
>>
>>                  c.func = RESIZE;
>>                  while ((option = getopt(argc, argv, option_string)) != EOF) {
>> @@ -637,9 +636,6 @@ void f2fs_parse_options(int argc, char *argv[])
>>                                          ret = sscanf(optarg, "%"PRIx64"",
>>                                                          &c.target_sectors);
>>                                  break;
>> -                       case 'i':
>> -                               c.large_nat_bitmap = 1;
>> -                               break;
>>                          case 'o':
>>                                  c.new_overprovision = atof(optarg);
>>                                  break;
>> diff --git a/fsck/resize.c b/fsck/resize.c
>> index 9b3b071..2681b9f 100644
>> --- a/fsck/resize.c
>> +++ b/fsck/resize.c
>> @@ -531,9 +531,6 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
>>
>>          /* update nat_bits flag */
>>          flags = update_nat_bits_flags(new_sb, cp, get_cp(ckpt_flags));
>> -       if (c.large_nat_bitmap)
>> -               flags |= CP_LARGE_NAT_BITMAP_FLAG;
>> -
>>          if (flags & CP_COMPACT_SUM_FLAG)
>>                  flags &= ~CP_COMPACT_SUM_FLAG;
>>          if (flags & CP_LARGE_NAT_BITMAP_FLAG)
>> --
>> 2.40.1
>>



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel