f2fs: remove gc_urgent_high_remaining node

Message ID 20221028130716.80850-1-frank.li@vivo.com
State New
Headers
Series f2fs: remove gc_urgent_high_remaining node |

Commit Message

李扬韬 Oct. 28, 2022, 1:07 p.m. UTC
  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 <frank.li@vivo.com>
---
 Documentation/ABI/testing/sysfs-fs-f2fs | 6 +++---
 fs/f2fs/f2fs.h                          | 1 -
 fs/f2fs/sysfs.c                         | 6 ------
 3 files changed, 3 insertions(+), 10 deletions(-)
  

Comments

Jaegeuk Kim Oct. 28, 2022, 5:25 p.m. UTC | #1
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 <frank.li@vivo.com>
> ---
>  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" <jaegeuk@kernel.org>
> -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" <daehojeong@google.com>
> -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
  
李扬韬 Nov. 10, 2022, 8:19 a.m. UTC | #2
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
  
Jaegeuk Kim Nov. 10, 2022, 9:07 p.m. UTC | #3
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
  

Patch

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" <jaegeuk@kernel.org>
-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" <daehojeong@google.com>
-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),