* Re: [f2fs-dev] [PATCH] fsck.f2fs: handle find_next_free_block failure gracefully
2026-07-23 19:35 [PATCH] fsck.f2fs: handle find_next_free_block failure gracefully Daeho Jeong
@ 2026-08-03 7:39 ` Chao Yu
0 siblings, 0 replies; 2+ messages in thread
From: Chao Yu @ 2026-08-03 7:39 UTC (permalink / raw)
To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team
Cc: chao, Daeho Jeong
On 7/24/26 03:35, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
>
> When find_next_free_block() fails to allocate a free block, callers like
> move_one_curseg_info(), reserve_new_block(), and update_block() previously
> crashed via ASSERT().
Seems there are a lot of ASSERT() in path of above functions? e.g.
f2fs_create
/* write child */
set_summary(&sum, de->ino, 0, ni.version);
ret = reserve_new_block(sbi, &blkaddr, &sum, CURSEG_HOT_NODE, 1);
nodeblk_alloced = true;
ASSERT(!ret);
convert_inline_dentry
ret = update_block(sbi, node, p_blkaddr, NULL);
ASSERT(ret >= 0);
Can you please take a look?
>
> Replace these ASSERT() calls with proper error propagation to gracefully
> abort FSCK execution instead of crashing.
>
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> ---
> fsck/fsck.c | 22 +++++++++++++++-------
> fsck/fsck.h | 8 ++++----
> fsck/main.c | 4 +++-
> fsck/mount.c | 28 ++++++++++++++++++----------
> fsck/segment.c | 8 ++++----
> 5 files changed, 44 insertions(+), 26 deletions(-)
>
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index db44f9d..b128617 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -2972,13 +2972,15 @@ int check_curseg_offsets(struct f2fs_sb_info *sbi, bool check_wp)
> return 0;
> }
>
> -static void fix_curseg_info(struct f2fs_sb_info *sbi, bool check_wp)
> +static int fix_curseg_info(struct f2fs_sb_info *sbi, bool check_wp)
> {
> - int i, need_update = 0;
> + int i, need_update = 0, ret;
>
> for (i = 0; i < NO_CHECK_TYPE; i++) {
> if (check_curseg_offset(sbi, i, check_wp)) {
> - update_curseg_info(sbi, i);
> + ret = update_curseg_info(sbi, i);
Another caller of fix_curseg_info(), fsck_verify() will accept the failure silently,
I think it needs to propagate the error to do_fsck().
Thanks,
> + if (ret)
> + return ret;
> need_update = 1;
> }
> }
> @@ -2987,6 +2989,7 @@ static void fix_curseg_info(struct f2fs_sb_info *sbi, bool check_wp)
> write_curseg_info(sbi);
> flush_curseg_sit_entries(sbi);
> }
> + return 0;
> }
>
> int check_sit_types(struct f2fs_sb_info *sbi)
> @@ -3595,23 +3598,28 @@ static void fix_wp_sit_alignment(struct f2fs_sb_info *UNUSED(sbi))
> * Check and fix consistency with write pointers at the beginning of
> * fsck so that following writes by fsck do not fail.
> */
> -void fsck_chk_and_fix_write_pointers(struct f2fs_sb_info *sbi)
> +int fsck_chk_and_fix_write_pointers(struct f2fs_sb_info *sbi)
> {
> struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
> + int ret = 0;
>
> if (c.zoned_model != F2FS_ZONED_HM)
> - return;
> + return 0;
>
> if (c.fix_on) {
> flush_nat_journal_entries(sbi);
> flush_sit_journal_entries(sbi);
>
> - if (check_curseg_offsets(sbi, true))
> - fix_curseg_info(sbi, true);
> + if (check_curseg_offsets(sbi, true)) {
> + ret = fix_curseg_info(sbi, true);
> + if (ret)
> + return ret;
> + }
>
> fix_wp_sit_alignment(sbi);
> fsck->chk.wp_fixed = 1;
> }
> + return 0;
> }
>
> int fsck_chk_curseg_info(struct f2fs_sb_info *sbi)
> diff --git a/fsck/fsck.h b/fsck/fsck.h
> index 05daa2d..ec37e08 100644
> --- a/fsck/fsck.h
> +++ b/fsck/fsck.h
> @@ -190,7 +190,7 @@ int fsck_chk_inline_dentries(struct f2fs_sb_info *, struct f2fs_node *,
> void fsck_chk_checkpoint(struct f2fs_sb_info *sbi);
> void fsck_update_sb_flags(struct f2fs_sb_info *sbi);
> int fsck_chk_meta(struct f2fs_sb_info *sbi);
> -void fsck_chk_and_fix_write_pointers(struct f2fs_sb_info *);
> +int fsck_chk_and_fix_write_pointers(struct f2fs_sb_info *sbi);
> int fsck_chk_curseg_info(struct f2fs_sb_info *);
> void pretty_print_filename(const u8 *raw_name, u32 len,
> char out[F2FS_PRINT_NAMELEN], int enc_name);
> @@ -227,11 +227,11 @@ extern int f2fs_find_fsync_inode(struct f2fs_sb_info *, struct list_head *);
> extern void f2fs_destroy_fsync_dnodes(struct list_head *);
>
> extern void flush_journal_entries(struct f2fs_sb_info *);
> -extern void update_curseg_info(struct f2fs_sb_info *, int);
> +extern int update_curseg_info(struct f2fs_sb_info *sbi, int type);
> extern void zero_journal_entries(struct f2fs_sb_info *);
> extern void flush_sit_entries(struct f2fs_sb_info *);
> -extern void move_curseg_info(struct f2fs_sb_info *, u64, int);
> -extern void move_one_curseg_info(struct f2fs_sb_info *sbi, u64 from, int left,
> +extern int move_curseg_info(struct f2fs_sb_info *sbi, u64 from, int left);
> +extern int move_one_curseg_info(struct f2fs_sb_info *sbi, u64 from, int left,
> int i);
> extern void write_curseg_info(struct f2fs_sb_info *);
> extern void save_curseg_warm_node_info(struct f2fs_sb_info *);
> diff --git a/fsck/main.c b/fsck/main.c
> index 08d38d8..460e4db 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -985,7 +985,9 @@ static int do_fsck(struct f2fs_sb_info *sbi)
> if (c.roll_forward && c.zoned_model == F2FS_ZONED_HM)
> save_curseg_warm_node_info(sbi);
>
> - fsck_chk_and_fix_write_pointers(sbi);
> + ret = fsck_chk_and_fix_write_pointers(sbi);
> + if (ret)
> + return FSCK_OPERATIONAL_ERROR;
>
> fsck_chk_curseg_info(sbi);
>
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 6f640a0..85ed404 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -3159,7 +3159,7 @@ next_segment:
> return -1;
> }
>
> -void move_one_curseg_info(struct f2fs_sb_info *sbi, u64 from, int left,
> +int move_one_curseg_info(struct f2fs_sb_info *sbi, u64 from, int left,
> int i)
> {
> struct f2fs_super_block *sb = F2FS_RAW_SUPER(sbi);
> @@ -3171,7 +3171,7 @@ void move_one_curseg_info(struct f2fs_sb_info *sbi, u64 from, int left,
>
> if ((get_sb(feature) & F2FS_FEATURE_RO)) {
> if (i != CURSEG_HOT_DATA && i != CURSEG_HOT_NODE)
> - return;
> + return 0;
>
> if (i == CURSEG_HOT_DATA) {
> left = 0;
> @@ -3191,7 +3191,10 @@ bypass_ssa:
> to = from;
> ret = find_next_free_block(sbi, &to, left, i,
> c.zoned_model == F2FS_ZONED_HM);
> - ASSERT(ret == 0);
> + if (ret) {
> + ERR_MSG("Failed to find next free block for curseg[%d]\n", i);
> + return ret;
> + }
>
> old_segno = curseg->segno;
> curseg->segno = GET_SEGNO(sbi, to);
> @@ -3211,22 +3214,27 @@ bypass_ssa:
>
> FIX_MSG("Move curseg[%d] %x -> %x after %"PRIx64"\n",
> i, old_segno, curseg->segno, from);
> + return 0;
> }
>
> -void move_curseg_info(struct f2fs_sb_info *sbi, u64 from, int left)
> +int move_curseg_info(struct f2fs_sb_info *sbi, u64 from, int left)
> {
> - int i;
> + int i, ret;
>
> /* update summary blocks having nullified journal entries */
> - for (i = 0; i < NO_CHECK_TYPE; i++)
> - move_one_curseg_info(sbi, from, left, i);
> + for (i = 0; i < NO_CHECK_TYPE; i++) {
> + ret = move_one_curseg_info(sbi, from, left, i);
> + if (ret)
> + return ret;
> + }
> + return 0;
> }
>
> -void update_curseg_info(struct f2fs_sb_info *sbi, int type)
> +int update_curseg_info(struct f2fs_sb_info *sbi, int type)
> {
> if (!relocate_curseg_offset(sbi, type))
> - return;
> - move_one_curseg_info(sbi, SM_I(sbi)->main_blkaddr, 0, type);
> + return 0;
> + return move_one_curseg_info(sbi, SM_I(sbi)->main_blkaddr, 0, type);
> }
>
> void zero_journal_entries(struct f2fs_sb_info *sbi)
> diff --git a/fsck/segment.c b/fsck/segment.c
> index 96de22a..46dc747 100644
> --- a/fsck/segment.c
> +++ b/fsck/segment.c
> @@ -70,8 +70,8 @@ int reserve_new_block(struct f2fs_sb_info *sbi, block_t *to,
> }
>
> if (find_next_free_block(sbi, &blkaddr, left, type, false)) {
> - ERR_MSG("Can't find free block");
> - ASSERT(0);
> + ERR_MSG("Can't find free block\n");
> + return -ENOSPC;
> }
>
> se = get_seg_entry(sbi, GET_SEGNO(sbi, blkaddr));
> @@ -789,8 +789,8 @@ int update_block(struct f2fs_sb_info *sbi, void *buf, u32 *blkaddr,
>
> new_blkaddr = SM_I(sbi)->main_blkaddr;
> if (find_next_free_block(sbi, &new_blkaddr, 0, type, false)) {
> - ERR_MSG("Can't find free block for the update");
> - ASSERT(0);
> + ERR_MSG("Can't find free block for the update\n");
> + return -ENOSPC;
> }
>
> ret = dev_write_block(buf, new_blkaddr, f2fs_io_type_to_rw_hint(type));
^ permalink raw reply [flat|nested] 2+ messages in thread