* [PATCH] f2fs: fix to avoid migrating empty section
@ 2025-09-01 2:04 Chao Yu
2025-09-03 0:04 ` [f2fs-dev] " Zhiguo Niu
2025-09-29 22:30 ` patchwork-bot+f2fs
0 siblings, 2 replies; 5+ messages in thread
From: Chao Yu @ 2025-09-01 2:04 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, Daeho Jeong
It reports a bug from device w/ zufs:
F2FS-fs (dm-64): Inconsistent segment (173822) type [1, 0] in SSA and SIT
F2FS-fs (dm-64): Stopped filesystem due to reason: 4
Thread A Thread B
- f2fs_expand_inode_data
- f2fs_allocate_pinning_section
- f2fs_gc_range
- do_garbage_collect w/ segno #x
- writepage
- f2fs_allocate_data_block
- new_curseg
- allocate segno #x
The root cause is: fallocate on pinning file may race w/ block allocation
as above, result in do_garbage_collect() from fallocate() may migrate
segment which is just allocated by a log, the log will update segment type
in its in-memory structure, however GC will get segment type from on-disk
SSA block, once segment type changes by log, we can detect such
inconsistency, then shutdown filesystem.
In this case, on-disk SSA shows type of segno #173822 is 1 (SUM_TYPE_NODE),
however segno #173822 was just allocated as data type segment, so in-memory
SIT shows type of segno #173822 is 0 (SUM_TYPE_DATA).
Change as below to fix this issue:
- check whether current section is empty before gc
- add sanity checks on do_garbage_collect() to avoid any race case, result
in migrating segment used by log.
- btw, it fixes misc issue in printed logs: "SSA and SIT" -> "SIT and SSA".
Fixes: 9703d69d9d15 ("f2fs: support file pinning for zoned devices")
Cc: Daeho Jeong <daehojeong@google.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/gc.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index ed3acbfc83ca..a7708cf80c04 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1794,6 +1794,13 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
struct folio *sum_folio = filemap_get_folio(META_MAPPING(sbi),
GET_SUM_BLOCK(sbi, segno));
+ if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno))) {
+ f2fs_err(sbi, "%s: segment %u is used by log",
+ __func__, segno);
+ f2fs_bug_on(sbi, 1);
+ goto skip;
+ }
+
if (get_valid_blocks(sbi, segno, false) == 0)
goto freed;
if (gc_type == BG_GC && __is_large_section(sbi) &&
@@ -1805,7 +1812,7 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
sum = folio_address(sum_folio);
if (type != GET_SUM_TYPE((&sum->footer))) {
- f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SSA and SIT",
+ f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SIT and SSA",
segno, type, GET_SUM_TYPE((&sum->footer)));
f2fs_stop_checkpoint(sbi, false,
STOP_CP_REASON_CORRUPTED_SUMMARY);
@@ -2068,6 +2075,13 @@ int f2fs_gc_range(struct f2fs_sb_info *sbi,
.iroot = RADIX_TREE_INIT(gc_list.iroot, GFP_NOFS),
};
+ /*
+ * avoid migrating empty section, as it can be allocated by
+ * log in parallel.
+ */
+ if (!get_valid_blocks(sbi, segno, true))
+ continue;
+
if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno)))
continue;
--
2.49.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: fix to avoid migrating empty section
2025-09-01 2:04 [PATCH] f2fs: fix to avoid migrating empty section Chao Yu
@ 2025-09-03 0:04 ` Zhiguo Niu
2025-09-03 1:33 ` Chao Yu
2025-09-29 22:30 ` patchwork-bot+f2fs
1 sibling, 1 reply; 5+ messages in thread
From: Zhiguo Niu @ 2025-09-03 0:04 UTC (permalink / raw)
To: Chao Yu; +Cc: jaegeuk, Daeho Jeong, linux-kernel, linux-f2fs-devel
Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
于2025年9月1日周一 10:08写道：
>
> It reports a bug from device w/ zufs:
>
> F2FS-fs (dm-64): Inconsistent segment (173822) type [1, 0] in SSA and SIT
> F2FS-fs (dm-64): Stopped filesystem due to reason: 4
>
> Thread A Thread B
> - f2fs_expand_inode_data
> - f2fs_allocate_pinning_section
> - f2fs_gc_range
> - do_garbage_collect w/ segno #x
> - writepage
> - f2fs_allocate_data_block
> - new_curseg
> - allocate segno #x
>
> The root cause is: fallocate on pinning file may race w/ block allocation
> as above, result in do_garbage_collect() from fallocate() may migrate
> segment which is just allocated by a log, the log will update segment type
> in its in-memory structure, however GC will get segment type from on-disk
> SSA block, once segment type changes by log, we can detect such
> inconsistency, then shutdown filesystem.
>
> In this case, on-disk SSA shows type of segno #173822 is 1 (SUM_TYPE_NODE),
> however segno #173822 was just allocated as data type segment, so in-memory
> SIT shows type of segno #173822 is 0 (SUM_TYPE_DATA).
>
> Change as below to fix this issue:
> - check whether current section is empty before gc
> - add sanity checks on do_garbage_collect() to avoid any race case, result
> in migrating segment used by log.
> - btw, it fixes misc issue in printed logs: "SSA and SIT" -> "SIT and SSA".
>
> Fixes: 9703d69d9d15 ("f2fs: support file pinning for zoned devices")
> Cc: Daeho Jeong <daehojeong@google.com>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> fs/f2fs/gc.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index ed3acbfc83ca..a7708cf80c04 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -1794,6 +1794,13 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> struct folio *sum_folio = filemap_get_folio(META_MAPPING(sbi),
> GET_SUM_BLOCK(sbi, segno));
>
> + if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno))) {
> + f2fs_err(sbi, "%s: segment %u is used by log",
> + __func__, segno);
> + f2fs_bug_on(sbi, 1);
Hi Chao,
Do we need f2fs_bug_on here? or it is enough to skip current segno
and let gc continue?
Thanks!
> + goto skip;
> + }
> +
> if (get_valid_blocks(sbi, segno, false) == 0)
> goto freed;
> if (gc_type == BG_GC && __is_large_section(sbi) &&
> @@ -1805,7 +1812,7 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>
> sum = folio_address(sum_folio);
> if (type != GET_SUM_TYPE((&sum->footer))) {
> - f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SSA and SIT",
> + f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SIT and SSA",
> segno, type, GET_SUM_TYPE((&sum->footer)));
> f2fs_stop_checkpoint(sbi, false,
> STOP_CP_REASON_CORRUPTED_SUMMARY);
> @@ -2068,6 +2075,13 @@ int f2fs_gc_range(struct f2fs_sb_info *sbi,
> .iroot = RADIX_TREE_INIT(gc_list.iroot, GFP_NOFS),
> };
>
> + /*
> + * avoid migrating empty section, as it can be allocated by
> + * log in parallel.
> + */
> + if (!get_valid_blocks(sbi, segno, true))
> + continue;
> +
> if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno)))
> continue;
>
> --
> 2.49.0
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: fix to avoid migrating empty section
2025-09-03 0:04 ` [f2fs-dev] " Zhiguo Niu
@ 2025-09-03 1:33 ` Chao Yu
2025-09-03 1:56 ` Zhiguo Niu
0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2025-09-03 1:33 UTC (permalink / raw)
To: Zhiguo Niu; +Cc: chao, jaegeuk, Daeho Jeong, linux-kernel, linux-f2fs-devel
On 9/3/25 08:04, Zhiguo Niu wrote:
> Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
> 于2025年9月1日周一 10:08写道：
>>
>> It reports a bug from device w/ zufs:
>>
>> F2FS-fs (dm-64): Inconsistent segment (173822) type [1, 0] in SSA and SIT
>> F2FS-fs (dm-64): Stopped filesystem due to reason: 4
>>
>> Thread A Thread B
>> - f2fs_expand_inode_data
>> - f2fs_allocate_pinning_section
>> - f2fs_gc_range
>> - do_garbage_collect w/ segno #x
>> - writepage
>> - f2fs_allocate_data_block
>> - new_curseg
>> - allocate segno #x
>>
>> The root cause is: fallocate on pinning file may race w/ block allocation
>> as above, result in do_garbage_collect() from fallocate() may migrate
>> segment which is just allocated by a log, the log will update segment type
>> in its in-memory structure, however GC will get segment type from on-disk
>> SSA block, once segment type changes by log, we can detect such
>> inconsistency, then shutdown filesystem.
>>
>> In this case, on-disk SSA shows type of segno #173822 is 1 (SUM_TYPE_NODE),
>> however segno #173822 was just allocated as data type segment, so in-memory
>> SIT shows type of segno #173822 is 0 (SUM_TYPE_DATA).
>>
>> Change as below to fix this issue:
>> - check whether current section is empty before gc
>> - add sanity checks on do_garbage_collect() to avoid any race case, result
>> in migrating segment used by log.
>> - btw, it fixes misc issue in printed logs: "SSA and SIT" -> "SIT and SSA".
>>
>> Fixes: 9703d69d9d15 ("f2fs: support file pinning for zoned devices")
>> Cc: Daeho Jeong <daehojeong@google.com>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>> fs/f2fs/gc.c | 16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index ed3acbfc83ca..a7708cf80c04 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -1794,6 +1794,13 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>> struct folio *sum_folio = filemap_get_folio(META_MAPPING(sbi),
>> GET_SUM_BLOCK(sbi, segno));
>>
>> + if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno))) {
>> + f2fs_err(sbi, "%s: segment %u is used by log",
>> + __func__, segno);
>> + f2fs_bug_on(sbi, 1);
> Hi Chao,
> Do we need f2fs_bug_on here? or it is enough to skip current segno
> and let gc continue?
Zhiguo,
I think we should never select in-use segment as GC victim, so I add a
f2fs_bug_on() here to detect any potential bugs. Let's see what will we
find w/ this.
Thanks,
> Thanks!
>> + goto skip;
>> + }
>> +
>> if (get_valid_blocks(sbi, segno, false) == 0)
>> goto freed;
>> if (gc_type == BG_GC && __is_large_section(sbi) &&
>> @@ -1805,7 +1812,7 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>>
>> sum = folio_address(sum_folio);
>> if (type != GET_SUM_TYPE((&sum->footer))) {
>> - f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SSA and SIT",
>> + f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SIT and SSA",
>> segno, type, GET_SUM_TYPE((&sum->footer)));
>> f2fs_stop_checkpoint(sbi, false,
>> STOP_CP_REASON_CORRUPTED_SUMMARY);
>> @@ -2068,6 +2075,13 @@ int f2fs_gc_range(struct f2fs_sb_info *sbi,
>> .iroot = RADIX_TREE_INIT(gc_list.iroot, GFP_NOFS),
>> };
>>
>> + /*
>> + * avoid migrating empty section, as it can be allocated by
>> + * log in parallel.
>> + */
>> + if (!get_valid_blocks(sbi, segno, true))
>> + continue;
>> +
>> if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno)))
>> continue;
>>
>> --
>> 2.49.0
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: fix to avoid migrating empty section
2025-09-03 1:33 ` Chao Yu
@ 2025-09-03 1:56 ` Zhiguo Niu
0 siblings, 0 replies; 5+ messages in thread
From: Zhiguo Niu @ 2025-09-03 1:56 UTC (permalink / raw)
To: Chao Yu; +Cc: jaegeuk, Daeho Jeong, linux-kernel, linux-f2fs-devel
Chao Yu <chao@kernel.org> 于2025年9月3日周三 09:33写道：
>
> On 9/3/25 08:04, Zhiguo Niu wrote:
> > Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
> > 于2025年9月1日周一 10:08写道：
> >>
> >> It reports a bug from device w/ zufs:
> >>
> >> F2FS-fs (dm-64): Inconsistent segment (173822) type [1, 0] in SSA and SIT
> >> F2FS-fs (dm-64): Stopped filesystem due to reason: 4
> >>
> >> Thread A Thread B
> >> - f2fs_expand_inode_data
> >> - f2fs_allocate_pinning_section
> >> - f2fs_gc_range
> >> - do_garbage_collect w/ segno #x
> >> - writepage
> >> - f2fs_allocate_data_block
> >> - new_curseg
> >> - allocate segno #x
> >>
> >> The root cause is: fallocate on pinning file may race w/ block allocation
> >> as above, result in do_garbage_collect() from fallocate() may migrate
> >> segment which is just allocated by a log, the log will update segment type
> >> in its in-memory structure, however GC will get segment type from on-disk
> >> SSA block, once segment type changes by log, we can detect such
> >> inconsistency, then shutdown filesystem.
> >>
> >> In this case, on-disk SSA shows type of segno #173822 is 1 (SUM_TYPE_NODE),
> >> however segno #173822 was just allocated as data type segment, so in-memory
> >> SIT shows type of segno #173822 is 0 (SUM_TYPE_DATA).
> >>
> >> Change as below to fix this issue:
> >> - check whether current section is empty before gc
> >> - add sanity checks on do_garbage_collect() to avoid any race case, result
> >> in migrating segment used by log.
> >> - btw, it fixes misc issue in printed logs: "SSA and SIT" -> "SIT and SSA".
> >>
> >> Fixes: 9703d69d9d15 ("f2fs: support file pinning for zoned devices")
> >> Cc: Daeho Jeong <daehojeong@google.com>
> >> Signed-off-by: Chao Yu <chao@kernel.org>
> >> ---
> >> fs/f2fs/gc.c | 16 +++++++++++++++-
> >> 1 file changed, 15 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >> index ed3acbfc83ca..a7708cf80c04 100644
> >> --- a/fs/f2fs/gc.c
> >> +++ b/fs/f2fs/gc.c
> >> @@ -1794,6 +1794,13 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> >> struct folio *sum_folio = filemap_get_folio(META_MAPPING(sbi),
> >> GET_SUM_BLOCK(sbi, segno));
> >>
> >> + if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno))) {
> >> + f2fs_err(sbi, "%s: segment %u is used by log",
> >> + __func__, segno);
> >> + f2fs_bug_on(sbi, 1);
> > Hi Chao,
> > Do we need f2fs_bug_on here? or it is enough to skip current segno
> > and let gc continue?
>
> Zhiguo,
>
> I think we should never select in-use segment as GC victim, so I add a
> f2fs_bug_on() here to detect any potential bugs. Let's see what will we
> find w/ this.
Hi Chao,
Got it and thanks for this.
>
> Thanks,
>
> > Thanks!
> >> + goto skip;
> >> + }
> >> +
> >> if (get_valid_blocks(sbi, segno, false) == 0)
> >> goto freed;
> >> if (gc_type == BG_GC && __is_large_section(sbi) &&
> >> @@ -1805,7 +1812,7 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
> >>
> >> sum = folio_address(sum_folio);
> >> if (type != GET_SUM_TYPE((&sum->footer))) {
> >> - f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SSA and SIT",
> >> + f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SIT and SSA",
> >> segno, type, GET_SUM_TYPE((&sum->footer)));
> >> f2fs_stop_checkpoint(sbi, false,
> >> STOP_CP_REASON_CORRUPTED_SUMMARY);
> >> @@ -2068,6 +2075,13 @@ int f2fs_gc_range(struct f2fs_sb_info *sbi,
> >> .iroot = RADIX_TREE_INIT(gc_list.iroot, GFP_NOFS),
> >> };
> >>
> >> + /*
> >> + * avoid migrating empty section, as it can be allocated by
> >> + * log in parallel.
> >> + */
> >> + if (!get_valid_blocks(sbi, segno, true))
> >> + continue;
> >> +
> >> if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno)))
> >> continue;
> >>
> >> --
> >> 2.49.0
> >>
> >>
> >>
> >> _______________________________________________
> >> Linux-f2fs-devel mailing list
> >> Linux-f2fs-devel@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: fix to avoid migrating empty section
2025-09-01 2:04 [PATCH] f2fs: fix to avoid migrating empty section Chao Yu
2025-09-03 0:04 ` [f2fs-dev] " Zhiguo Niu
@ 2025-09-29 22:30 ` patchwork-bot+f2fs
1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+f2fs @ 2025-09-29 22:30 UTC (permalink / raw)
To: Chao Yu; +Cc: jaegeuk, daehojeong, linux-kernel, linux-f2fs-devel
Hello:
This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:
On Mon, 1 Sep 2025 10:04:15 +0800 you wrote:
> It reports a bug from device w/ zufs:
>
> F2FS-fs (dm-64): Inconsistent segment (173822) type [1, 0] in SSA and SIT
> F2FS-fs (dm-64): Stopped filesystem due to reason: 4
>
> Thread A Thread B
> - f2fs_expand_inode_data
> - f2fs_allocate_pinning_section
> - f2fs_gc_range
> - do_garbage_collect w/ segno #x
> - writepage
> - f2fs_allocate_data_block
> - new_curseg
> - allocate segno #x
>
> [...]
Here is the summary with links:
- [f2fs-dev] f2fs: fix to avoid migrating empty section
https://git.kernel.org/jaegeuk/f2fs/c/d625a2b08c08
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-09-29 22:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-01 2:04 [PATCH] f2fs: fix to avoid migrating empty section Chao Yu
2025-09-03 0:04 ` [f2fs-dev] " Zhiguo Niu
2025-09-03 1:33 ` Chao Yu
2025-09-03 1:56 ` Zhiguo Niu
2025-09-29 22:30 ` patchwork-bot+f2fs
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome