2022-10-28 13:52:07

by 李扬韬

[permalink] [raw]
Subject: [PATCH] f2fs: remove gc_urgent_high_remaining node

As Jaegeuk suggested to remove it. And it is not a super critical node,
which just for internal tests now.

Let's remove the testing gc_urgent_high_remaining sysfs node.
As well as updating the description in the documentation to indicate
that readers need to use the new node.

BTW, fill in a missing '>', :)

Signed-off-by: Yangtao Li <[email protected]>
---
Documentation/ABI/testing/sysfs-fs-f2fs | 6 +++---
fs/f2fs/f2fs.h | 1 -
fs/f2fs/sysfs.c | 6 ------
3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 374f0c98f8a9..e6b87522518d 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -241,7 +241,7 @@ Description: Shows total written kbytes issued to disk.
What: /sys/fs/f2fs/<disk>/features
Date: July 2017
Contact: "Jaegeuk Kim" <[email protected]>
-Description: <deprecated: should use /sys/fs/f2fs/<disk>/feature_list/
+Description: <deprecated>: should use /sys/fs/f2fs/<disk>/feature_list/
Shows all enabled features in current device.
Supported features:
encryption, blkzoned, extra_attr, projquota, inode_checksum,
@@ -601,10 +601,10 @@ Description: With "mode=fragment:block" mount options, we can scatter block allo
What: /sys/fs/f2fs/<disk>/gc_urgent_high_remaining
Date: December 2021
Contact: "Daeho Jeong" <[email protected]>
-Description: You can set the trial count limit for GC urgent high mode with this value.
+Description: <deprecated>: should use /sys/fs/f2fs/<disk>/gc_urgent_idle_remaining
+ You can set the trial count limit for GC urgent high mode with this value.
If GC thread gets to the limit, the mode will turn back to GC normal mode.
By default, the value is zero, which means there is no limit like before.
- <deprecated>

What: /sys/fs/f2fs/<disk>/gc_urgent_idle_remaining
Date: October 2022
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index b467c0ca1118..d0a279605f20 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1747,7 +1747,6 @@ struct f2fs_sb_info {
spinlock_t gc_urgent_idle_lock;
/* remaining trial count for GC_URGENT_* and GC_IDLE_* */
unsigned int gc_urgent_idle_remaining;
- unsigned int gc_urgent_high_remaining; /* deprecated */

/* for skip statistic */
unsigned long long skipped_gc_rwsem; /* FG_GC only */
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index 9312954ec03b..9db77f18e8a7 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -538,10 +538,6 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
return count;
}

- /* deprecated */
- if (!strcmp(a->attr.name, "gc_urgent_high_remaining"))
- return -EINVAL;
-
if (!strcmp(a->attr.name, "gc_urgent_idle_remaining")) {
spin_lock(&sbi->gc_urgent_idle_lock);
sbi->gc_urgent_idle_remaining = t;
@@ -836,7 +832,6 @@ F2FS_RW_ATTR(FAULT_INFO_TYPE, f2fs_fault_info, inject_type, inject_type);
#endif
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, data_io_flag, data_io_flag);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, node_io_flag, node_io_flag);
-F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_urgent_high_remaining, gc_urgent_high_remaining);
F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_urgent_idle_remaining, gc_urgent_idle_remaining);
F2FS_RW_ATTR(CPRC_INFO, ckpt_req_control, ckpt_thread_ioprio, ckpt_thread_ioprio);
F2FS_GENERAL_RO_ATTR(dirty_segments);
@@ -966,7 +961,6 @@ static struct attribute *f2fs_attrs[] = {
#endif
ATTR_LIST(data_io_flag),
ATTR_LIST(node_io_flag),
- ATTR_LIST(gc_urgent_high_remaining),
ATTR_LIST(gc_urgent_idle_remaining),
ATTR_LIST(ckpt_thread_ioprio),
ATTR_LIST(dirty_segments),
--
2.25.1



2022-10-28 17:37:53

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH] f2fs: remove gc_urgent_high_remaining node

Hi,

I modified and integrated some patches as below. Could you please take a look?

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=d34f1e7212c1965a409d4581a32a92a1c91495fc

Thanks,

On 10/28, Yangtao Li wrote:
> As Jaegeuk suggested to remove it. And it is not a super critical node,
> which just for internal tests now.
>
> Let's remove the testing gc_urgent_high_remaining sysfs node.
> As well as updating the description in the documentation to indicate
> that readers need to use the new node.
>
> BTW, fill in a missing '>', :)
>
> Signed-off-by: Yangtao Li <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-fs-f2fs | 6 +++---
> fs/f2fs/f2fs.h | 1 -
> fs/f2fs/sysfs.c | 6 ------
> 3 files changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 374f0c98f8a9..e6b87522518d 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -241,7 +241,7 @@ Description: Shows total written kbytes issued to disk.
> What: /sys/fs/f2fs/<disk>/features
> Date: July 2017
> Contact: "Jaegeuk Kim" <[email protected]>
> -Description: <deprecated: should use /sys/fs/f2fs/<disk>/feature_list/
> +Description: <deprecated>: should use /sys/fs/f2fs/<disk>/feature_list/
> Shows all enabled features in current device.
> Supported features:
> encryption, blkzoned, extra_attr, projquota, inode_checksum,
> @@ -601,10 +601,10 @@ Description: With "mode=fragment:block" mount options, we can scatter block allo
> What: /sys/fs/f2fs/<disk>/gc_urgent_high_remaining
> Date: December 2021
> Contact: "Daeho Jeong" <[email protected]>
> -Description: You can set the trial count limit for GC urgent high mode with this value.
> +Description: <deprecated>: should use /sys/fs/f2fs/<disk>/gc_urgent_idle_remaining
> + You can set the trial count limit for GC urgent high mode with this value.
> If GC thread gets to the limit, the mode will turn back to GC normal mode.
> By default, the value is zero, which means there is no limit like before.
> - <deprecated>
>
> What: /sys/fs/f2fs/<disk>/gc_urgent_idle_remaining
> Date: October 2022
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index b467c0ca1118..d0a279605f20 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1747,7 +1747,6 @@ struct f2fs_sb_info {
> spinlock_t gc_urgent_idle_lock;
> /* remaining trial count for GC_URGENT_* and GC_IDLE_* */
> unsigned int gc_urgent_idle_remaining;
> - unsigned int gc_urgent_high_remaining; /* deprecated */
>
> /* for skip statistic */
> unsigned long long skipped_gc_rwsem; /* FG_GC only */
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index 9312954ec03b..9db77f18e8a7 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -538,10 +538,6 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
> return count;
> }
>
> - /* deprecated */
> - if (!strcmp(a->attr.name, "gc_urgent_high_remaining"))
> - return -EINVAL;
> -
> if (!strcmp(a->attr.name, "gc_urgent_idle_remaining")) {
> spin_lock(&sbi->gc_urgent_idle_lock);
> sbi->gc_urgent_idle_remaining = t;
> @@ -836,7 +832,6 @@ F2FS_RW_ATTR(FAULT_INFO_TYPE, f2fs_fault_info, inject_type, inject_type);
> #endif
> F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, data_io_flag, data_io_flag);
> F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, node_io_flag, node_io_flag);
> -F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_urgent_high_remaining, gc_urgent_high_remaining);
> F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_urgent_idle_remaining, gc_urgent_idle_remaining);
> F2FS_RW_ATTR(CPRC_INFO, ckpt_req_control, ckpt_thread_ioprio, ckpt_thread_ioprio);
> F2FS_GENERAL_RO_ATTR(dirty_segments);
> @@ -966,7 +961,6 @@ static struct attribute *f2fs_attrs[] = {
> #endif
> ATTR_LIST(data_io_flag),
> ATTR_LIST(node_io_flag),
> - ATTR_LIST(gc_urgent_high_remaining),
> ATTR_LIST(gc_urgent_idle_remaining),
> ATTR_LIST(ckpt_thread_ioprio),
> ATTR_LIST(dirty_segments),
> --
> 2.25.1

2022-11-10 09:19:08

by 李扬韬

[permalink] [raw]
Subject: Re: [PATCH] f2fs: remove gc_urgent_high_remaining node

Hi Jaegeuk,

> Hi,

> I modified and integrated some patches as below. Could you please take a look?

> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=d34f1e7212c1965a409d4581a32a92a1c91495fc

I've looked at the relevant changes on the branch, looks good to me.

One small question, does the name need to be changed to gc_remaining_count?
There are already controls based on the number of gcs, maybe we can add a
gc_remaining_ms later? Control the gc mode based on the expected time. When
the count is reduced to 0 or the gc ends and the expected value is reached,
switch back to the normal mode.

Thx,
Yangtao

2022-11-10 22:47:31

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH] f2fs: remove gc_urgent_high_remaining node

On 11/10, Yangtao Li wrote:
> Hi Jaegeuk,
>
> > Hi,
>
> > I modified and integrated some patches as below. Could you please take a look?
>
> > https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=d34f1e7212c1965a409d4581a32a92a1c91495fc
>
> I've looked at the relevant changes on the branch, looks good to me.
>
> One small question, does the name need to be changed to gc_remaining_count?
> There are already controls based on the number of gcs, maybe we can add a
> gc_remaining_ms later? Control the gc mode based on the expected time. When
> the count is reduced to 0 or the gc ends and the expected value is reached,
> switch back to the normal mode.

Please check the latest patch which replaced it with gc_remaining_trials.

I think the time-based control can be managed by user side instead of kernel.


>
> Thx,
> Yangtao