Re: [f2fs-dev] [PATCH 1/5] f2fs: check number of blocks in a current section

From: Chao Yu <chao@kernel.org>
To: Jaegeuk Kim <jaegeuk@kernel.org>,
	linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH 1/5] f2fs: check number of blocks in a current section
Date: Mon, 26 Feb 2024 10:40:41 +0800	[thread overview]
Message-ID: <afa444c5-7ac1-457e-9559-6d7c6623c426@kernel.org> (raw)
In-Reply-To: <20240223205535.307307-1-jaegeuk@kernel.org>

On 2024/2/24 4:55, Jaegeuk Kim wrote:
> In cfd66bb715fd ("f2fs: fix deadloop in foreground GC"), we needed to check
> the number of blocks in a section instead of the segment.
> 
> In addtion, let's check the entire node sections when checking the avaiable
> node block space. It does not match one to one per temperature, but currently

I tested this patch w/ testcase in [1], it doesn't complain.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=215914

> we don't have exact dirty page count per temperature. Hence, use a rough
> estimation.

Do we need more accurate per-temperature dirty node count for extreme
corner case?

e.g. node_blocks is counted from hot dirty nodes, and current hot_node
log has no enough free space for it.

Thanks,

> 
> Fixes: cfd66bb715fd ("f2fs: fix deadloop in foreground GC")
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>   fs/f2fs/segment.h | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index 3edd3809b6b5..15bf5edd9b3c 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -561,23 +561,23 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
>   			unsigned int node_blocks, unsigned int dent_blocks)
>   {
>   
> -	unsigned int segno, left_blocks;
> +	unsigned segno, left_blocks;
>   	int i;
>   
> -	/* check current node segment */
> +	/* check current node sections, which counts very roughly */
> +	left_blocks = 0;
>   	for (i = CURSEG_HOT_NODE; i <= CURSEG_COLD_NODE; i++) {
>   		segno = CURSEG_I(sbi, i)->segno;
> -		left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
> -				get_seg_entry(sbi, segno)->ckpt_valid_blocks;
> -
> -		if (node_blocks > left_blocks)
> -			return false;
> +		left_blocks += CAP_BLKS_PER_SEC(sbi) -
> +				get_ckpt_valid_blocks(sbi, segno, true);
>   	}
> +	if (node_blocks > left_blocks)
> +		return false;
>   
> -	/* check current data segment */
> +	/* check current data section for dentry blocks. */
>   	segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno;
> -	left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
> -			get_seg_entry(sbi, segno)->ckpt_valid_blocks;
> +	left_blocks = CAP_BLKS_PER_SEC(sbi) -
> +			get_ckpt_valid_blocks(sbi, segno, true);
>   	if (dent_blocks > left_blocks)
>   		return false;
>   	return true;
> @@ -626,7 +626,7 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>   
>   	if (free_secs > upper_secs)
>   		return false;
> -	else if (free_secs <= lower_secs)
> +	if (free_secs <= lower_secs)
>   		return true;
>   	return !curseg_space;
>   }