linux-kernel - Re: [PATCH v2] f2fs: fix missing discard for active segments

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4270b213-e4f9-46b2-958a-df3dbaaed969@kernel.org>
Date: Mon, 20 Jan 2025 20:25:08 +0800
From: Chao Yu <chao@...nel.org>
To: Chunhai Guo <guochunhai@...o.com>, jaegeuk@...nel.org
Cc: chao@...nel.org, linux-f2fs-devel@...ts.sourceforge.net,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] f2fs: fix missing discard for active segments

On 1/9/25 20:27, Chunhai Guo wrote:
> During a checkpoint, the current active segment X may not be handled
> properly. This occurs when segment X has 0 valid blocks and a non-zero

How does this happen? Allocator selects a dirty segment w/ SSR? and the
left valid data blocks were deleted later before following checkpoint?

If so, pending discard count in that segment should be in range of (0, 512)?

Thanks,

> number of discard blocks, for the following reasons:
> 
> locate_dirty_segment() does not mark any active segment as a prefree
> segment. As a result, segment X is not included in dirty_segmap[PRE], and
> f2fs_clear_prefree_segments() skips it when handling prefree segments.
> 
> add_discard_addrs() skips any segment with 0 valid blocks, so segment X is
> also skipped.
> 
> Consequently, no `struct discard_cmd` is actually created for segment X.
> However, the ckpt_valid_map and cur_valid_map of segment X are synced by
> seg_info_to_raw_sit() during the current checkpoint process. As a result,
> it cannot find the missing discard bits even in subsequent checkpoints.
> Consequently, the value of sbi->discard_blks remains non-zero. Thus, when
> f2fs is umounted, CP_TRIMMED_FLAG will not be set due to the non-zero
> sbi->discard_blks.
> 
> Relevant code process:
> 
> f2fs_write_checkpoint()
>      f2fs_flush_sit_entries()
>           list_for_each_entry_safe(ses, tmp, head, set_list) {
>               for_each_set_bit_from(segno, bitmap, end) {
>                   ...
>                   add_discard_addrs(sbi, cpc, false); // skip segment X due to its 0 valid blocks
>                   ...
>                   seg_info_to_raw_sit(); // sync ckpt_valid_map with cur_valid_map for segment X
>                   ...
>               }
>           }
>      f2fs_clear_prefree_segments(); // segment X is not included in dirty_segmap[PRE] and is skipped
> 
> Since add_discard_addrs() can handle active segments with non-zero valid
> blocks, it is reasonable to fix this issue by allowing it to also handle
> active segments with 0 valid blocks.
> 
> Fixes: b29555505d81 ("f2fs: add key functions for small discards")
> Signed-off-by: Chunhai Guo <guochunhai@...o.com>
> ---
> v1: https://lore.kernel.org/linux-f2fs-devel/20241203065108.2763436-1-guochunhai@vivo.com/
> v1->v2:
>   - Modify the commit message to make it easier to understand.
>   - Add fixes to the commit.
> ---
>   fs/f2fs/segment.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 86e547f008f9..13ee73a3c481 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2090,7 +2090,9 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>   		return false;
>   
>   	if (!force) {
> -		if (!f2fs_realtime_discard_enable(sbi) || !se->valid_blocks ||
> +		if (!f2fs_realtime_discard_enable(sbi) ||
> +			(!se->valid_blocks &&
> +				!IS_CURSEG(sbi, cpc->trim_start)) ||
>   			SM_I(sbi)->dcc_info->nr_discards >=
>   				SM_I(sbi)->dcc_info->max_discards)
>   			return false;