Re: [f2fs-dev] [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN
Chao Yu via Linux-f2fs-devel <[email protected]>
| Newsgroups | net.sourceforge.lists.linux-f2fs-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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!"); >>> + } >>> >>> 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