Re: [PATCH] f2fs: fix deadloop in foreground GC

From: Jaegeuk Kim
Date: Tue May 03 2022 - 18:25:08 EST


On 04/30, Chao Yu wrote:
> As Yanming reported in bugzilla:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=215914
>
> The root cause is: in a very small sized image, it's very easy to
> exceed threshold of foreground GC, if we calculate free space and
> dirty data based on section granularity, in corner case,
> has_not_enough_free_secs() will always return true, result in
> deadloop in f2fs_gc().

Performance regression was reported. Can we check this for very small sized
image only?

>
> So this patch refactors has_not_enough_free_secs() as below to fix
> this issue:
> 1. calculate needed space based on block granularity, and separate
> all blocks to two parts, section part, and block part, comparing
> section part to free section, and comparing block part to free space
> in openned log.
> 2. account F2FS_DIRTY_NODES, F2FS_DIRTY_IMETA and F2FS_DIRTY_DENTS
> as node block consumer;
> 3. account F2FS_DIRTY_DENTS as data block consumer;
>
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Ming Yan <yanming@xxxxxxxxxx>
> Signed-off-by: Chao Yu <chao.yu@xxxxxxxx>
> ---
> fs/f2fs/segment.h | 30 +++++++++++++++++-------------
> 1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index 8a591455d796..28f7aa9b40bf 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -575,11 +575,10 @@ static inline int reserved_sections(struct f2fs_sb_info *sbi)
> return GET_SEC_FROM_SEG(sbi, reserved_segments(sbi));
> }
>
> -static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
> +static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
> + unsigned int node_blocks, unsigned int dent_blocks)
> {
> - unsigned int node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
> - get_pages(sbi, F2FS_DIRTY_DENTS);
> - unsigned int dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
> +
> unsigned int segno, left_blocks;
> int i;
>
> @@ -605,19 +604,24 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
> static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
> int freed, int needed)
> {
> - int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
> - int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
> - int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
> + unsigned int total_node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
> + get_pages(sbi, F2FS_DIRTY_DENTS) +
> + get_pages(sbi, F2FS_DIRTY_IMETA);
> + unsigned int total_dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
> + unsigned int node_secs = total_node_blocks / BLKS_PER_SEC(sbi);
> + unsigned int dent_secs = total_dent_blocks / BLKS_PER_SEC(sbi);
> + unsigned int node_blocks = total_node_blocks % BLKS_PER_SEC(sbi);
> + unsigned int dent_blocks = total_dent_blocks % BLKS_PER_SEC(sbi);
>
> if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
> return false;
>
> - if (free_sections(sbi) + freed == reserved_sections(sbi) + needed &&
> - has_curseg_enough_space(sbi))
> - return false;
> - return (free_sections(sbi) + freed) <=
> - (node_secs + 2 * dent_secs + imeta_secs +
> - reserved_sections(sbi) + needed);
> + if (free_sections(sbi) + freed <=
> + node_secs + dent_secs + reserved_sections(sbi) + needed)
> + return true;
> + if (!has_curseg_enough_space(sbi, node_blocks, dent_blocks))
> + return true;
> + return false;
> }
>
> static inline bool f2fs_is_checkpoint_ready(struct f2fs_sb_info *sbi)
> --
> 2.32.0