| 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: <CACOAw_xqi7cy5j5FVNKJk=QLGzda+pzAmq=weHNOpiasG+zeog@mail.gmail.com>
Date: Thu, 13 Feb 2025 10:48:52 -0800
From: Daeho Jeong <daeho43@...il.com>
To: Yohan Joung <jyh429@...il.com>
Cc: jaegeuk@...nel.org, chao@...nel.org,
linux-f2fs-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
Yohan Joung <yohan.joung@...com>
Subject: Re: [PATCH v1] f2fs: separate discard and zone reset command from
pend list
On Sat, Feb 8, 2025 at 5:54 AM Yohan Joung <jyh429@...il.com> wrote:
>
> currently, zone reset only occurs when there is urgent utilization and
> when pending blocks are reallocated. this causes performance
> degradation, so we are modifying it to allow pending reset zones to be
> issued.
>
> Signed-off-by: Yohan Joung <yohan.joung@...com>
> ---
> fs/f2fs/f2fs.h | 3 ++-
> fs/f2fs/segment.c | 21 +++++++++++++++------
> 2 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 1afa7be16e7d..09a7e13c0d00 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -349,6 +349,7 @@ struct discard_entry {
>
> /* max discard pend list number */
> #define MAX_PLIST_NUM 512
> +#define ZONE_PLIST_NUM 1
> #define plist_idx(blk_num) ((blk_num) >= MAX_PLIST_NUM ? \
> (MAX_PLIST_NUM - 1) : ((blk_num) - 1))
>
> @@ -410,7 +411,7 @@ struct discard_policy {
> struct discard_cmd_control {
> struct task_struct *f2fs_issue_discard; /* discard thread */
> struct list_head entry_list; /* 4KB discard entry list */
> - struct list_head pend_list[MAX_PLIST_NUM];/* store pending entries */
> + struct list_head pend_list[MAX_PLIST_NUM + ZONE_PLIST_NUM];/* store pending entries */
> struct list_head wait_list; /* store on-flushing entries */
> struct list_head fstrim_list; /* in-flight discard from fstrim */
> wait_queue_head_t discard_wait_queue; /* waiting queue for wake-up */
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index c282e8a0a2ec..1c32252db525 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -962,7 +962,10 @@ static struct discard_cmd *__create_discard_cmd(struct f2fs_sb_info *sbi,
>
> f2fs_bug_on(sbi, !len);
>
> - pend_list = &dcc->pend_list[plist_idx(len)];
> + if (f2fs_sb_has_blkzoned(sbi) && bdev_is_zoned(bdev))
> + pend_list = &dcc->pend_list[MAX_PLIST_NUM];
> + else
> + pend_list = &dcc->pend_list[plist_idx(len)];
>
> dc = f2fs_kmem_cache_alloc(discard_cmd_slab, GFP_NOFS, true, NULL);
> INIT_LIST_HEAD(&dc->list);
> @@ -1649,6 +1652,8 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
> struct discard_cmd *dc, *tmp;
> struct blk_plug plug;
> int i, issued;
> + int plist_num = f2fs_sb_has_blkzoned(sbi) ?
> + MAX_PLIST_NUM + ZONE_PLIST_NUM : MAX_PLIST_NUM;
> bool io_interrupted = false;
>
> if (dpolicy->timeout)
> @@ -1656,12 +1661,12 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
>
> retry:
> issued = 0;
> - for (i = MAX_PLIST_NUM - 1; i >= 0; i--) {
> + for (i = plist_num - 1; i >= 0; i--) {
> if (dpolicy->timeout &&
> f2fs_time_over(sbi, UMOUNT_DISCARD_TIMEOUT))
> break;
>
> - if (i + 1 < dpolicy->granularity)
> + if (i + 1 < dpolicy->granularity && i + 1 != plist_num)
To me, this part is kind of a hack, since it just skips checking
granularity for the largest pending list.
It might not work for conventional devices. The fundamental problem is
the current pend_list doesn't
work with zoned devices now. So, I think we need a new design covering
zoned devices for discard
such as using another discard pending list for zoned devices and
controlling it in a different way.
Thanks for letting me know about this issue. I will come up with a new
discard design for zoned devices.
> break;
>
> if (i + 1 < dcc->max_ordered_discard && dpolicy->ordered) {
> @@ -1720,6 +1725,8 @@ static bool __drop_discard_cmd(struct f2fs_sb_info *sbi)
> struct list_head *pend_list;
> struct discard_cmd *dc, *tmp;
> int i;
> + int plist_num = f2fs_sb_has_blkzoned(sbi) ?
> + MAX_PLIST_NUM + ZONE_PLIST_NUM : MAX_PLIST_NUM;
> bool dropped = false;
>
> mutex_lock(&dcc->cmd_lock);
> @@ -2305,7 +2312,7 @@ int f2fs_start_discard_thread(struct f2fs_sb_info *sbi)
> static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
> {
> struct discard_cmd_control *dcc;
> - int err = 0, i;
> + int err = 0, i, plist_num;
>
> if (SM_I(sbi)->dcc_info) {
> dcc = SM_I(sbi)->dcc_info;
> @@ -2316,7 +2323,9 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
> if (!dcc)
> return -ENOMEM;
>
> - dcc->discard_io_aware_gran = MAX_PLIST_NUM;
> + plist_num = f2fs_sb_has_blkzoned(sbi) ?
> + MAX_PLIST_NUM + ZONE_PLIST_NUM : MAX_PLIST_NUM;
> + dcc->discard_io_aware_gran = plist_num;
> dcc->discard_granularity = DEFAULT_DISCARD_GRANULARITY;
> dcc->max_ordered_discard = DEFAULT_MAX_ORDERED_DISCARD_GRANULARITY;
> dcc->discard_io_aware = DPOLICY_IO_AWARE_ENABLE;
> @@ -2326,7 +2335,7 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
> dcc->discard_granularity = BLKS_PER_SEC(sbi);
>
> INIT_LIST_HEAD(&dcc->entry_list);
> - for (i = 0; i < MAX_PLIST_NUM; i++)
> + for (i = 0; i < plist_num; i++)
> INIT_LIST_HEAD(&dcc->pend_list[i]);
> INIT_LIST_HEAD(&dcc->wait_list);
> INIT_LIST_HEAD(&dcc->fstrim_list);
> --
> 2.25.1
>
Powered by blists - more mailing lists