Re: [PATCH v2 1/2] f2fs: use per-log target_bitmap to improve lookup performace of ssr allocation

[Jaegeuk Kim]

Hi Chao,

This introduces another bitmap increasing memory footprint. Do we know how
much performance benefit with this?

On 04/11, Chao Yu wrote:
> After commit 899fee36fac0 ("f2fs: fix to avoid data corruption by
> forbidding SSR overwrite"), valid block bitmap of current openned
> segment is fixed, let's introduce a per-log bitmap instead of temp
> bitmap to avoid unnecessary calculation overhead whenever allocating
> free slot w/ SSR allocator.
> 
> Signed-off-by: Chao Yu <chao@xxxxxxxxxx>
> ---
> v2:
> - rebase to last dev-test branch.
>  fs/f2fs/segment.c | 30 ++++++++++++++++++++++--------
>  fs/f2fs/segment.h |  1 +
>  2 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 6474b7338e81..af716925db19 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2840,31 +2840,39 @@ static int new_curseg(struct f2fs_sb_info *sbi, int type, bool new_sec)
>  	return 0;
>  }
>  
> -static int __next_free_blkoff(struct f2fs_sb_info *sbi,
> -					int segno, block_t start)
> +static void __get_segment_bitmap(struct f2fs_sb_info *sbi,
> +					unsigned long *target_map,
> +					int segno)
>  {
>  	struct seg_entry *se = get_seg_entry(sbi, segno);
>  	int entries = SIT_VBLOCK_MAP_SIZE / sizeof(unsigned long);
> -	unsigned long *target_map = SIT_I(sbi)->tmp_map;
>  	unsigned long *ckpt_map = (unsigned long *)se->ckpt_valid_map;
>  	unsigned long *cur_map = (unsigned long *)se->cur_valid_map;
>  	int i;
>  
>  	for (i = 0; i < entries; i++)
>  		target_map[i] = ckpt_map[i] | cur_map[i];
> +}
> +
> +static int __next_free_blkoff(struct f2fs_sb_info *sbi, unsigned long *bitmap,
> +					int segno, block_t start)
> +{
> +	__get_segment_bitmap(sbi, bitmap, segno);
>  
> -	return __find_rev_next_zero_bit(target_map, BLKS_PER_SEG(sbi), start);
> +	return __find_rev_next_zero_bit(bitmap, BLKS_PER_SEG(sbi), start);
>  }
>  
>  static int f2fs_find_next_ssr_block(struct f2fs_sb_info *sbi,
> -		struct curseg_info *seg)
> +					struct curseg_info *seg)
>  {
> -	return __next_free_blkoff(sbi, seg->segno, seg->next_blkoff + 1);
> +	return __find_rev_next_zero_bit(seg->target_map,
> +				BLKS_PER_SEG(sbi), seg->next_blkoff + 1);
>  }
>  
>  bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno)
>  {
> -	return __next_free_blkoff(sbi, segno, 0) < BLKS_PER_SEG(sbi);
> +	return __next_free_blkoff(sbi, SIT_I(sbi)->tmp_map, segno, 0) <
> +							BLKS_PER_SEG(sbi);
>  }
>  
>  /*
> @@ -2890,7 +2898,8 @@ static int change_curseg(struct f2fs_sb_info *sbi, int type)
>  
>  	reset_curseg(sbi, type, 1);
>  	curseg->alloc_type = SSR;
> -	curseg->next_blkoff = __next_free_blkoff(sbi, curseg->segno, 0);
> +	curseg->next_blkoff = __next_free_blkoff(sbi, curseg->target_map,
> +							curseg->segno, 0);
>  
>  	sum_page = f2fs_get_sum_page(sbi, new_segno);
>  	if (IS_ERR(sum_page)) {
> @@ -4635,6 +4644,10 @@ static int build_curseg(struct f2fs_sb_info *sbi)
>  				sizeof(struct f2fs_journal), GFP_KERNEL);
>  		if (!array[i].journal)
>  			return -ENOMEM;
> +		array[i].target_map = f2fs_kzalloc(sbi, SIT_VBLOCK_MAP_SIZE,
> +								GFP_KERNEL);
> +		if (!array[i].target_map)
> +			return -ENOMEM;
>  		if (i < NR_PERSISTENT_LOG)
>  			array[i].seg_type = CURSEG_HOT_DATA + i;
>  		else if (i == CURSEG_COLD_DATA_PINNED)
> @@ -5453,6 +5466,7 @@ static void destroy_curseg(struct f2fs_sb_info *sbi)
>  	for (i = 0; i < NR_CURSEG_TYPE; i++) {
>  		kfree(array[i].sum_blk);
>  		kfree(array[i].journal);
> +		kfree(array[i].target_map);
>  	}
>  	kfree(array);
>  }
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index e1c0f418aa11..10f3e44f036f 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -292,6 +292,7 @@ struct curseg_info {
>  	struct f2fs_summary_block *sum_blk;	/* cached summary block */
>  	struct rw_semaphore journal_rwsem;	/* protect journal area */
>  	struct f2fs_journal *journal;		/* cached journal info */
> +	unsigned long *target_map;		/* bitmap for SSR allocator */
>  	unsigned char alloc_type;		/* current allocation type */
>  	unsigned short seg_type;		/* segment type like CURSEG_XXX_TYPE */
>  	unsigned int segno;			/* current segment number */
> -- 
> 2.40.1