Re: [f2fs-dev] [PATCH] fsck.f2fs: handle find_next_free_block failure gracefully

[Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>]

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



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel