Received-SPF: pass (google.com: domain of linux-kernel+bounces-80417-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <afa444c5-7ac1-457e-9559-6d7c6623c426@kernel.org>
Date: Mon, 26 Feb 2024 10:40:41 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [f2fs-dev] [PATCH 1/5] f2fs: check number of blocks in a current
 section
Content-Language: en-US
To: Jaegeuk Kim <jaegeuk@kernel.org>, linux-kernel@vger.kernel.org,
 linux-f2fs-devel@lists.sourceforge.net
References: <20240223205535.307307-1-jaegeuk@kernel.org>
From: Chao Yu <chao@kernel.org>
In-Reply-To: <20240223205535.307307-1-jaegeuk@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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