| 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
| ||
|
Message-ID: <17ceed17-b17f-42d9-8c82-79f1f4814c1a@kernel.org>
Date: Wed, 18 Sep 2024 14:43:44 +0800
From: Chao Yu <chao@...nel.org>
To: liuderong@...o.com, jaegeuk@...nel.org
Cc: chao@...nel.org, linux-f2fs-devel@...ts.sourceforge.net,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] f2fs: introduce get_section_mtime
On 2024/9/12 14:40, liuderong@...o.com wrote:
> From: liuderong <liuderong@...o.com>
>
> 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@...o.com>
> ---
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/gc.c | 15 ++-------------
> fs/f2fs/segment.c | 41 ++++++++++++++++++++++++++++++++++++-----
> 3 files changed, 40 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);
Hi Derong,
It needs to add "f2fs_" prefix for get_section_mtime() to avoid global
namespace pollution.
>
> #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..e62e722 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -5389,6 +5389,41 @@ 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 = 0;
> +
> + if (segno == NULL_SEGNO)
> + return 0;
No needed.
> +
> + secno = GET_SEC_FROM_SEG(sbi, segno);
> + start = GET_SEG_FROM_SEC(sbi, secno);
> +
> + 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 */
> + 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);
> + }
After commit 6f3a01ae9b72 ("f2fs: record average update time of segment"),
mtime of segment starts to indicate average update time of segment.
So it needs to change like this?
for (i = 0; i < usable_segs_per_sec; i++) {
struct seg_entry *se = get_seg_entry(sbi, start + i);
mtime += se->mtime * se->valid_blocks;
total_valid_blocks += se->valid_blocks;
}
if (total_valid_blocks == 0)
return 0;
return div_u64(mtime, total_valid_blocks);
Thanks,
> +
> + if (valid_seg_count == 0)
> + return 0;
> +
> + return div_u64(mtime, valid_seg_count);
> +}
> +
> /*
> * Update min, max modified time for cost-benefit GC algorithm
> */
> @@ -5402,13 +5437,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;
Powered by blists - more mailing lists