linux-kernel - Re: [f2fs-dev] [PATCH 2/7] f2fs: read summary blocks with the correct amount for migration

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 for Android: free password hash cracker in your pocket

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

Message-ID: <501416af-b08c-448b-881d-0915575e22f5@kernel.org>
Date: Fri, 6 Sep 2024 10:56:06 +0800
From: Chao Yu <chao@...nel.org>
To: Daeho Jeong <daeho43@...il.com>, linux-kernel@...r.kernel.org,
 linux-f2fs-devel@...ts.sourceforge.net, kernel-team@...roid.com
Cc: Daeho Jeong <daehojeong@...gle.com>
Subject: Re: [f2fs-dev] [PATCH 2/7] f2fs: read summary blocks with the correct
 amount for migration_granularity

On 2024/8/30 5:52, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@...gle.com>
> 
> Now we do readahead for a full section by not considering
> migration_granularity and it triggers unnecessary read. So, make it read
> with the correct amount.
> 
> Signed-off-by: Daeho Jeong <daehojeong@...gle.com>
> ---
>   fs/f2fs/gc.c | 33 ++++++++++++++++++++-------------
>   1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 46e3bc26b78a..b5d3fd40b17a 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -1708,24 +1708,33 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>   	struct blk_plug plug;
>   	unsigned int segno = start_segno;
>   	unsigned int end_segno = start_segno + SEGS_PER_SEC(sbi);
> +	unsigned int sec_end_segno;
>   	int seg_freed = 0, migrated = 0;
>   	unsigned char type = IS_DATASEG(get_seg_entry(sbi, segno)->type) ?
>   						SUM_TYPE_DATA : SUM_TYPE_NODE;
>   	unsigned char data_type = (type == SUM_TYPE_DATA) ? DATA : NODE;
>   	int submitted = 0;
>   
> -	if (__is_large_section(sbi))
> -		end_segno = rounddown(end_segno, SEGS_PER_SEC(sbi));
> +	if (__is_large_section(sbi)) {
> +		sec_end_segno = rounddown(end_segno, SEGS_PER_SEC(sbi));
>   
> -	/*
> -	 * zone-capacity can be less than zone-size in zoned devices,
> -	 * resulting in less than expected usable segments in the zone,
> -	 * calculate the end segno in the zone which can be garbage collected
> -	 */
> -	if (f2fs_sb_has_blkzoned(sbi))
> -		end_segno -= SEGS_PER_SEC(sbi) -
> +		/*
> +		 * zone-capacity can be less than zone-size in zoned devices,
> +		 * resulting in less than expected usable segments in the zone,
> +		 * calculate the end segno in the zone which can be garbage
> +		 * collected
> +		 */
> +		if (f2fs_sb_has_blkzoned(sbi))
> +			sec_end_segno -= SEGS_PER_SEC(sbi) -
>   					f2fs_usable_segs_in_sec(sbi, segno);
>   
> +		if (gc_type == BG_GC)
> +			end_segno = start_segno + sbi->migration_granularity;
> +
> +		if (end_segno > sec_end_segno)
> +			end_segno = sec_end_segno;
> +	}
> +
>   	sanity_check_seg_type(sbi, get_seg_entry(sbi, segno)->type);
>   
>   	/* readahead multi ssa blocks those have contiguous address */
> @@ -1762,9 +1771,6 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>   
>   		if (get_valid_blocks(sbi, segno, false) == 0)
>   			goto freed;
> -		if (gc_type == BG_GC && __is_large_section(sbi) &&
> -				migrated >= sbi->migration_granularity)

It seems we change the logic from migrating "migration_granularity" segments which
has valid blocks to scanning "migration_granularity" segments and try migrating
valid blocks in those segments.

IIUC, when background GC recycle sparse zone, it will take gc thread more round,
it seems low efficient. How do you think of keeping previous implementation?

Thanks,

> -			goto skip;
>   		if (!PageUptodate(sum_page) || unlikely(f2fs_cp_error(sbi)))
>   			goto skip;
>   
> @@ -1803,7 +1809,8 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>   
>   		if (__is_large_section(sbi))
>   			sbi->next_victim_seg[gc_type] =
> -				(segno + 1 < end_segno) ? segno + 1 : NULL_SEGNO;
> +				(segno + 1 < sec_end_segno) ?
> +					segno + 1 : NULL_SEGNO;
>   skip:
>   		f2fs_put_page(sum_page, 0);
>   	}