Re: [f2fs-dev] [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN
Daeho Jeong <[email protected]>
| Newsgroups | net.sourceforge.lists.linux-f2fs-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CACOAw_wq3UDos43X49hcufaKRaGsMYcQmYujuy1x256+L2a--w@mail.gmail.com> |
On Thu, Aug 20, 2026 at 8:47 AM Daeho Jeong <[email protected]> wrote: > > > > On Thu, Aug 20, 2026 at 12:20 AM Chao Yu <[email protected]> wrote: >> >> On 8/20/26 02:21, Daeho Jeong wrote: >> > On Tue, Aug 18, 2026 at 7:34 PM Chao Yu <[email protected]> wrote: >> >> >> >> On 8/19/26 01:05, Daeho Jeong wrote: >> >>> From: Daeho Jeong <[email protected]> >> >>> >> >>> When f2fs_resize_fs() fails due to transient lock contention or retryable >> >>> GC failure in free_segment_range() returning -EAGAIN, no filesystem >> >>> metadata has been modified on-disk yet. The filesystem remains completely >> >>> consistent and clean. >> >> >> >> Any way to make sure EAGAIN is from free_segment_range(), in case we return >> >> EAGAIN from 1) any other places that we may miss to check now or 2) we changed >> >> the code to return EAGAIN in future. >> > >> > Makes sense. >> > >> > In fact, if free_segment_range() fails with ANY error (e.g., -EAGAIN, >> > -ENOMEM, -ERESTARTSYS, etc.), no on-disk superblock or filesystem metadata >> > has been modified yet, and free_segment_range() safely restores all in-memory >> > counters (MAIN_SECS, free_sections) before returning. Therefore, setting >> > SBI_NEED_FSCK is not needed for all errors occurring in free_segment_range(). >> >> Yeah, better. >> >> > >> > To make this explicit and defensive against any future changes, we can >> > separate the error recovery path with dedicated labels as follows: >> > >> > err = free_segment_range(sbi, secs, false); >> > if (err) >> > - goto recover_out; >> > + goto recover_user_blocks; >> > >> > update_sb_metadata(sbi, -secs); >> > >> > ... >> > >> > recover_out: >> > + set_sbi_flag(sbi, SBI_NEED_FSCK); >> > + f2fs_err(sbi, "resize_fs failed, should run fsck to repair!"); >> > +recover_user_blocks: >> >> How about this? >> >> recover_out: >> if (err) { >> f2fs_bug_on(sbi, err == -EAGAIN); >> set_sbi_flag(sbi, SBI_NEED_FSCK); >> f2fs_err(sbi, "resize_fs failed, should run fsck to repair!"); >> } >> >> recover_user_blocks: >> >> Thanks, >> >> > clear_sbi_flag(sbi, SBI_IS_RESIZEFS); >> > if (err) { >> > - set_sbi_flag(sbi, SBI_NEED_FSCK); >> > - f2fs_err(sbi, "resize_fs failed, should run fsck to repair!"); >> > - >> > spin_lock(&sbi->stat_lock); >> > sbi->user_block_count += shrunk_blocks; >> > spin_unlock(&sbi->stat_lock); >> > >> > Thanks, >> > >> >> >> >> Thanks, >> >> >> >>> >> >>> However, the current error recovery path unconditionally sets the >> >>> SBI_NEED_FSCK flag on any error, forcing an unnecessary and time-consuming >> >>> fsck.f2fs repair on the subsequent mount/reboot. >> >>> >> >>> Fix this by guarding set_sbi_flag(sbi, SBI_NEED_FSCK) with >> >>> `if (err != -EAGAIN)`, avoiding false-positive filesystem corruption >> >>> flags on transient resize retries. >> >>> >> >>> Signed-off-by: Daeho Jeong <[email protected]> >> >>> Signed-off-by: Sunmin Jeong <[email protected]> >> >>> --- >> >>> fs/f2fs/gc.c | 6 ++++-- >> >>> 1 file changed, 4 insertions(+), 2 deletions(-) >> >>> >> >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >> >>> index 192b16ac02f8..787133ee2eb2 100644 >> >>> --- a/fs/f2fs/gc.c >> >>> +++ b/fs/f2fs/gc.c >> >>> @@ -2446,8 +2446,10 @@ int f2fs_resize_fs(struct file *filp, __u64 block_count) >> >>> recover_out: >> >>> clear_sbi_flag(sbi, SBI_IS_RESIZEFS); >> >>> if (err) { >> >>> - set_sbi_flag(sbi, SBI_NEED_FSCK); >> >>> - f2fs_err(sbi, "resize_fs failed, should run fsck to repair!"); >> >>> + if (err != -EAGAIN) { >> >>> + set_sbi_flag(sbi, SBI_NEED_FSCK); >> >>> + f2fs_err(sbi, "resize_fs failed, should run fsck to repair!"); >> >>> + } Got it. Thanks~ >> >> >>> >> >>> spin_lock(&sbi->stat_lock); >> >>> sbi->user_block_count += shrunk_blocks; >> >> >> _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel