Re: [PATCH v1 2/2] f2fs: introduce get_section_mtime

[Chao Yu]

On 2024/9/11 10:40, liuderong@xxxxxxxx wrote:
> From: liuderong <liuderong@xxxxxxxx>
>
> When segs_per_sec is larger than 1, section may contain free segments,
> mtime should be the mean value of each valid segments,
> so introduce get_section_mtime to exclude free segments in a section.
>
> Signed-off-by: liuderong <liuderong@xxxxxxxx>
> ---
>   fs/f2fs/f2fs.h    |  2 ++
>   fs/f2fs/gc.c      | 15 ++-------------
>   fs/f2fs/segment.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
>   3 files changed, 48 insertions(+), 18 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 4dcdcdd..d6adf0f 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3762,6 +3762,8 @@ enum rw_hint f2fs_io_type_to_rw_hint(struct f2fs_sb_info *sbi,
>   unsigned int f2fs_usable_segs_in_sec(struct f2fs_sb_info *sbi);
>   unsigned int f2fs_usable_blks_in_seg(struct f2fs_sb_info *sbi,
>                          unsigned int segno);
> +unsigned long long get_section_mtime(struct f2fs_sb_info *sbi,
> +                       unsigned int segno);
>
>   #define DEF_FRAGMENT_SIZE      4
>   #define MIN_FRAGMENT_SIZE      1
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 6299639..03c6117 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -332,20 +332,14 @@ static unsigned int check_bg_victims(struct f2fs_sb_info *sbi)
>   static unsigned int get_cb_cost(struct f2fs_sb_info *sbi, unsigned int segno)
>   {
>          struct sit_info *sit_i = SIT_I(sbi);
> -       unsigned int secno = GET_SEC_FROM_SEG(sbi, segno);
> -       unsigned int start = GET_SEG_FROM_SEC(sbi, secno);
>          unsigned long long mtime = 0;
>          unsigned int vblocks;
>          unsigned char age = 0;
>          unsigned char u;
> -       unsigned int i;
>          unsigned int usable_segs_per_sec = f2fs_usable_segs_in_sec(sbi);
>
> -       for (i = 0; i < usable_segs_per_sec; i++)
> -               mtime += get_seg_entry(sbi, start + i)->mtime;
> +       mtime = get_section_mtime(sbi, segno);
>          vblocks = get_valid_blocks(sbi, segno, true);
> -
> -       mtime = div_u64(mtime, usable_segs_per_sec);
>          vblocks = div_u64(vblocks, usable_segs_per_sec);
>
>          u = BLKS_TO_SEGS(sbi, vblocks * 100);
> @@ -485,10 +479,7 @@ static void add_victim_entry(struct f2fs_sb_info *sbi,
>                                  struct victim_sel_policy *p, unsigned int segno)
>   {
>          struct sit_info *sit_i = SIT_I(sbi);
> -       unsigned int secno = GET_SEC_FROM_SEG(sbi, segno);
> -       unsigned int start = GET_SEG_FROM_SEC(sbi, secno);
>          unsigned long long mtime = 0;
> -       unsigned int i;
>
>          if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) {
>                  if (p->gc_mode == GC_AT &&
> @@ -496,9 +487,7 @@ static void add_victim_entry(struct f2fs_sb_info *sbi,
>                          return;
>          }
>
> -       for (i = 0; i < SEGS_PER_SEC(sbi); i++)
> -               mtime += get_seg_entry(sbi, start + i)->mtime;
> -       mtime = div_u64(mtime, SEGS_PER_SEC(sbi));
> +       mtime = get_section_mtime(sbi, segno);
>
>          /* Handle if the system time has changed by the user */
>          if (mtime < sit_i->min_mtime)
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 6627394..b63f35fa 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -5389,6 +5389,49 @@ unsigned int f2fs_usable_segs_in_sec(struct f2fs_sb_info *sbi)
>          return SEGS_PER_SEC(sbi);
>   }
>
> +unsigned long long get_section_mtime(struct f2fs_sb_info *sbi,
> +       unsigned int segno)
> +{
> +       unsigned int usable_segs_per_sec = f2fs_usable_segs_in_sec(sbi);
> +       unsigned int secno = 0, start = 0;
> +       struct free_segmap_info *free_i = FREE_I(sbi);
> +       unsigned int valid_seg_count = 0;
> +       unsigned long long mtime = 0;
> +       unsigned int i;
> +
> +       if (segno == NULL_SEGNO)
> +               return 0;
> +
> +       secno = GET_SEC_FROM_SEG(sbi, segno);
> +       start = GET_SEG_FROM_SEC(sbi, secno);
> +
> +       for (i = 0; i < usable_segs_per_sec; i++) {
> +               /* for large section, only check the mtime of valid segments */
> +               if (__is_large_section(sbi)) {
> +                       spin_lock(&free_i->segmap_lock);
> +                       if (test_bit(start + i, free_i->free_segmap)) {
> +                               mtime += get_seg_entry(sbi, start + i)->mtime;
> +                               valid_seg_count++;
> +                       }
> +                       spin_unlock(&free_i->segmap_lock);
> +               } else {
> +                       mtime += get_seg_entry(sbi, start + i)->mtime;
> +               }
> +       }
> +
> +       if (__is_large_section(sbi)) {
> +               if (!valid_seg_count)
> +                       goto out;
> +               mtime = div_u64(mtime, valid_seg_count);
> +       } else {
> +               mtime = div_u64(mtime, usable_segs_per_sec);
> +       }

What about:

if (!__is_large_section(sbi))
	return get_seg_entry(sbi, start + i)->mtime;

for (i = 0; i < usable_segs_per_sec; i++) {
	/* for large section, only check the mtime of valid segments */
	...
}

return div_u64(mtime, usable_segs_per_sec);

Thanks,

> +
> +out:
> +       return mtime;
> +}
> +
>   /*
>    * Update min, max modified time for cost-benefit GC algorithm
>    */
> @@ -5402,13 +5445,9 @@ static void init_min_max_mtime(struct f2fs_sb_info *sbi)
>          sit_i->min_mtime = ULLONG_MAX;
>
>          for (segno = 0; segno < MAIN_SEGS(sbi); segno += SEGS_PER_SEC(sbi)) {
> -               unsigned int i;
>                  unsigned long long mtime = 0;
>
> -               for (i = 0; i < SEGS_PER_SEC(sbi); i++)
> -                       mtime += get_seg_entry(sbi, segno + i)->mtime;
> -
> -               mtime = div_u64(mtime, SEGS_PER_SEC(sbi));
> +               mtime = get_section_mtime(sbi, segno);
>
>                  if (sit_i->min_mtime > mtime)
>                          sit_i->min_mtime = mtime;
> --
> 2.7.4