mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN
@ 2026-08-18 17:05 Daeho Jeong
  2026-08-19  2:34 ` [f2fs-dev] " Chao Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Daeho Jeong @ 2026-08-18 17:05 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong, Sunmin Jeong

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.

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;
-- 
2.55.0.691.gc56d675ccc-goog


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN
  2026-08-18 17:05 [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN Daeho Jeong
@ 2026-08-19  2:34 ` Chao Yu
  2026-08-19 18:21   ` Daeho Jeong
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2026-08-19  2:34 UTC (permalink / raw)
  To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team
  Cc: chao, Daeho Jeong

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.

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;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN
  2026-08-19  2:34 ` [f2fs-dev] " Chao Yu
@ 2026-08-19 18:21   ` Daeho Jeong
  2026-08-20  7:20     ` Chao Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Daeho Jeong @ 2026-08-19 18:21 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong

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().

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:
  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;
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN
  2026-08-19 18:21   ` Daeho Jeong
@ 2026-08-20  7:20     ` Chao Yu
       [not found]       ` <CABdZyexzNgVLcdSFRkt=AZnkgW5Q40m3Hf8JFk63WuTGhQ0S7w@mail.gmail.com>
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2026-08-20  7:20 UTC (permalink / raw)
  To: Daeho Jeong
  Cc: chao, linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong

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;
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN
       [not found]       ` <CABdZyexzNgVLcdSFRkt=AZnkgW5Q40m3Hf8JFk63WuTGhQ0S7w@mail.gmail.com>
@ 2026-08-20 16:14         ` Daeho Jeong
  0 siblings, 0 replies; 5+ messages in thread
From: Daeho Jeong @ 2026-08-20 16:14 UTC (permalink / raw)
  To: Daeho Jeong; +Cc: Chao Yu, linux-kernel, linux-f2fs-devel, kernel-team

On Thu, Aug 20, 2026 at 8:47 AM Daeho Jeong <daehojeong@google.com> wrote:
>
>
>
> On Thu, Aug 20, 2026 at 12:20 AM Chao Yu <chao@kernel.org> wrote:
>>
>> 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!");
>> >>> +             }


Got it. Thanks~


>>
>> >>>
>> >>>               spin_lock(&sbi->stat_lock);
>> >>>               sbi->user_block_count += shrunk_blocks;
>> >>
>>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-20 16:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-18 17:05 [PATCH] f2fs: avoid setting SBI_NEED_FSCK on transient resize failure with -EAGAIN 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
     [not found]       ` <CABdZyexzNgVLcdSFRkt=AZnkgW5Q40m3Hf8JFk63WuTGhQ0S7w@mail.gmail.com>
2026-08-20 16:14         ` Daeho Jeong

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®