Re: [f2fs-dev] [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN

[Daeho Jeong]

On Thu, Aug 20, 2026 at 8:47 AM Daeho Jeong <daehojeong@xxxxxxxxxx> wrote:
>
>
>
> On Thu, Aug 20, 2026 at 12:20 AM Chao Yu <chao@xxxxxxxxxx> wrote:
>>
>> On 8/20/26 02:21, Daeho Jeong wrote:
>> > On Tue, Aug 18, 2026 at 7:34 PM Chao Yu <chao@xxxxxxxxxx> wrote:
>> >>
>> >> On 8/19/26 01:05, Daeho Jeong wrote:
>> >>> From: Daeho Jeong <daehojeong@xxxxxxxxxx>
>> >>>
>> >>> 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 <daehojeong@xxxxxxxxxx>
>> >>> Signed-off-by: Sunmin Jeong <s_min.jeong@xxxxxxxxxxx>
>> >>> ---
>> >>>  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;
>> >>
>>