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. On Fri, Apr 11, 2025 at 1:57 AM Chao Yu <c...@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+f3d6ipobxegkzxxtsxcfwrkb9ph68jtuk3h9cn8ovl...@mail.gmail.com/ > [2] > https://lore.kernel.org/linux-f2fs-devel/20190514093340.40217-2-yuch...@huawei.com/ > > Fixes: daa0f8b ("resize.f2fs: add option for large_nat_bitmap feature") > Reported-by: Ju Hyung Park <qkrwngud...@gmail.com> > Signed-off-by: Chao Yu <c...@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
