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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.