Message-ID: <fe663b7c-2aa3-c15c-5ce7-942a47310742@kernel.org>
Date:   Sat, 28 Jan 2023 17:37:36 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH 3/4] f2fs: introduce ipu_mode sysfs node
Content-Language: en-US
To:     Yangtao Li <frank.li@vivo.com>, jaegeuk@kernel.org
Cc:     linux-f2fs-devel@lists.sourceforge.net,
        linux-kernel@vger.kernel.org
References: <20230120134029.69200-1-frank.li@vivo.com>
 <20230120134029.69200-3-frank.li@vivo.com>
From:   Chao Yu <chao@kernel.org>
In-Reply-To: <20230120134029.69200-3-frank.li@vivo.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2023/1/20 21:40, Yangtao Li wrote:
> Add a ipu_mode sysfs node to show the current ipu policy as a string.

Why do we need to add another sysfs entry for ipu_policy? we can check
the description of ipu_policy bits in below sysfs document.

What:           /sys/fs/f2fs/<disk>/ipu_policy
Date:           November 2013
Contact:        "Jaegeuk Kim" <jaegeuk.kim@samsung.com>
Description:    Controls the in-place-update policy.
                 updates in f2fs. User can set:

                 ====  =================
                 0x01  F2FS_IPU_FORCE
                 0x02  F2FS_IPU_SSR
                 0x04  F2FS_IPU_UTIL
                 0x08  F2FS_IPU_SSR_UTIL
                 0x10  F2FS_IPU_FSYNC
                 0x20  F2FS_IPU_ASYNC
                 0x40  F2FS_IPU_NOCACHE
                 0x80  F2FS_IPU_HONOR_OPU_WRITE
                 ====  =================

                 Refer segment.h for details.

Thanks,

> Since we use ipu_policy as a bitmap, and the bitmap API parameter is
> unsigned long type data, let's change ipu_policy to unsigned long type.
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>   Documentation/ABI/testing/sysfs-fs-f2fs |  6 ++++++
>   fs/f2fs/f2fs.h                          |  4 ++--
>   fs/f2fs/segment.h                       |  1 +
>   fs/f2fs/sysfs.c                         | 26 +++++++++++++++++++++++++
>   4 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 0f17adc80488..64b15a28fe30 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -722,3 +722,9 @@ What:		/sys/fs/f2fs/<disk>/last_age_weight
>   Date:		January 2023
>   Contact:	"Ping Xiong" <xiongping1@xiaomi.com>
>   Description:	When DATA SEPARATION is on, it controls the weight of last data block age.
> +
> +What:		/sys/fs/f2fs/<disk>/ipu_mode
> +Date:		January 2023
> +Contact:	"Yangtao Li" <frank.li@vivo.com>
> +Description:	Show the current ipu policy as a string.
> +		This is a read-only entry.
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 9edad9ccc2cd..b221a3bdb3fe 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1071,7 +1071,7 @@ struct f2fs_sm_info {
>   
>   	struct list_head sit_entry_set;	/* sit entry set list */
>   
> -	unsigned int ipu_policy;	/* in-place-update policy */
> +	unsigned long ipu_policy;	/* in-place-update policy */
>   	unsigned int min_ipu_util;	/* in-place-update threshold */
>   	unsigned int min_fsync_blocks;	/* threshold for fsync */
>   	unsigned int min_seq_blocks;	/* threshold for sequential blocks */
> @@ -1323,7 +1323,7 @@ enum {
>   	MAX_TIME,
>   };
>   
> -/* Note that you need to keep synchronization with this gc_mode_names array */
> +/* Modification on enum should be synchronized with gc_mode_names array */
>   enum {
>   	GC_NORMAL,
>   	GC_IDLE_CB,
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index d73e988566a5..50c63155ad56 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -672,6 +672,7 @@ static inline int utilization(struct f2fs_sb_info *sbi)
>   
>   #define F2FS_IPU_DISABLE	0
>   
> +/* Modification on enum should be synchronized with ipu_mode_names array */
>   enum {
>   	F2FS_IPU_FORCE,
>   	F2FS_IPU_SSR,
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index 576e6416ffb9..aaf5d8650518 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -51,6 +51,17 @@ static const char *gc_mode_names[MAX_GC_MODE] = {
>   	"GC_URGENT_MID"
>   };
>   
> +static const char *ipu_mode_names[F2FS_IPU_MAX] = {
> +	"FORCE",
> +	"SSR",
> +	"UTIL",
> +	"SSR_UTIL",
> +	"FSYNC",
> +	"ASYNC",
> +	"NOCACHE",
> +	"HONOR_OPU_WRITE",
> +};
> +
>   struct f2fs_attr {
>   	struct attribute attr;
>   	ssize_t (*show)(struct f2fs_attr *a, struct f2fs_sb_info *sbi, char *buf);
> @@ -149,6 +160,19 @@ static ssize_t gc_mode_show(struct f2fs_attr *a,
>   	return sysfs_emit(buf, "%s\n", gc_mode_names[sbi->gc_mode]);
>   }
>   
> +static ssize_t ipu_mode_show(struct f2fs_attr *a,
> +		struct f2fs_sb_info *sbi, char *buf)
> +{
> +	int len = 0, i = 0;
> +
> +	if (SM_I(sbi)->ipu_policy == F2FS_IPU_DISABLE)
> +		return sysfs_emit(buf, "DISABLE\n");
> +
> +	for_each_set_bit(i, &SM_I(sbi)->ipu_policy, F2FS_IPU_MAX)
> +		len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n", ipu_mode_names[i]);
> +	return len;
> +}
> +
>   static ssize_t features_show(struct f2fs_attr *a,
>   		struct f2fs_sb_info *sbi, char *buf)
>   {
> @@ -907,6 +931,7 @@ F2FS_GENERAL_RO_ATTR(mounted_time_sec);
>   F2FS_GENERAL_RO_ATTR(main_blkaddr);
>   F2FS_GENERAL_RO_ATTR(pending_discard);
>   F2FS_GENERAL_RO_ATTR(gc_mode);
> +F2FS_GENERAL_RO_ATTR(ipu_mode);
>   #ifdef CONFIG_F2FS_STAT_FS
>   F2FS_STAT_ATTR(STAT_INFO, f2fs_stat_info, cp_foreground_calls, cp_count);
>   F2FS_STAT_ATTR(STAT_INFO, f2fs_stat_info, cp_background_calls, bg_cp_count);
> @@ -997,6 +1022,7 @@ static struct attribute *f2fs_attrs[] = {
>   	ATTR_LIST(max_ordered_discard),
>   	ATTR_LIST(pending_discard),
>   	ATTR_LIST(gc_mode),
> +	ATTR_LIST(ipu_mode),
>   	ATTR_LIST(ipu_policy),
>   	ATTR_LIST(min_ipu_util),
>   	ATTR_LIST(min_fsync_blocks),