From: Chao Yu <chao@kernel.org>
To: Daeho Jeong <daeho43@gmail.com>
Cc: chao@kernel.org, linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net, kernel-team@android.com,
Daeho Jeong <daehojeong@google.com>
Subject: Re: [f2fs-dev] [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN
Date: Thu, 20 Aug 2026 15:20:54 +0800 [thread overview]
Message-ID: <746c8f52-cf7c-4f63-b2e1-36e0cad940a1@kernel.org> (raw)
In-Reply-To: <CACOAw_xSS78P_c8ZWXGDadxa+xFAVWWN0kAx1QMrujVuruBYsQ@mail.gmail.com>
On 8/20/26 02:21, Daeho Jeong wrote:
> On Tue, Aug 18, 2026 at 7:34 PM Chao Yu <chao@kernel.org> wrote:
>>
>> On 8/19/26 01:05, Daeho Jeong wrote:
>>> From: Daeho Jeong <daehojeong@google.com>
>>>
>>> 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@google.com>
>>> Signed-off-by: Sunmin Jeong <s_min.jeong@samsung.com>
>>> ---
>>> 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;
>>
next prev parent reply other threads:[~2026-08-20 7:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-18 17:05 Daeho Jeong
2026-08-19 2:34 ` [f2fs-dev] " Chao Yu
2026-08-19 18:21 ` Daeho Jeong
2026-08-20 7:20 ` Chao Yu [this message]
[not found] ` <CABdZyexzNgVLcdSFRkt=AZnkgW5Q40m3Hf8JFk63WuTGhQ0S7w@mail.gmail.com>
2026-08-20 16:14 ` Daeho Jeong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=746c8f52-cf7c-4f63-b2e1-36e0cad940a1@kernel.org \
--to=chao@kernel.org \
--cc=daeho43@gmail.com \
--cc=daehojeong@google.com \
--cc=kernel-team@android.com \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®