2024-02-07 06:26:00

by Chao Yu

[permalink] [raw]
Subject: [PATCH] f2fs: support compress extension update via sysfs interface

Introduce /sys/fs/f2fs/<disk>/compress_extension to support
adding/deleting compress extension via sysfs interface, in
comparison to mount option, it's more easy to use and less
authority issue for applications.

Usage:
- Query: cat /sys/fs/f2fs/<disk>/compress_extension
- Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
- Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
- [c] means add/del compress extension
- [n] means add/del nocompress extension

Signed-off-by: Sheng Yong <[email protected]>
Signed-off-by: Chao Yu <[email protected]>
---
Documentation/ABI/testing/sysfs-fs-f2fs | 10 ++++
Documentation/filesystems/f2fs.rst | 6 ++-
fs/f2fs/compress.c | 61 +++++++++++++++++++++++
fs/f2fs/f2fs.h | 4 +-
fs/f2fs/sysfs.c | 65 +++++++++++++++++++++++--
5 files changed, 139 insertions(+), 7 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
index 48c135e24eb5..1f2cc0913e45 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -762,3 +762,13 @@ Date: November 2023
Contact: "Chao Yu" <[email protected]>
Description: It controls to enable/disable IO aware feature for background discard.
By default, the value is 1 which indicates IO aware is on.
+
+What: /sys/fs/f2fs/<disk>/compress_extension
+Date: October 2023
+Contact: "Chao Yu" <[email protected]>
+Description: Used to control configure [|no]compress_extension list:
+ - Query: cat /sys/fs/f2fs/<disk>/compress_extension
+ - Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
+ - Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
+ - [c] means add/del compress extension
+ - [n] means add/del nocompress extension
diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index 32cbfa864f38..c82a8fd7316b 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -821,17 +821,19 @@ Compression implementation
all logical blocks in cluster contain valid data and compress ratio of
cluster data is lower than specified threshold.

-- To enable compression on regular inode, there are four ways:
+- To enable compression on regular inode, there are five ways:

* chattr +c file
* chattr +c dir; touch dir/file
* mount w/ -o compress_extension=ext; touch file.ext
* mount w/ -o compress_extension=*; touch any_file
+ * echo '[c]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext

-- To disable compression on regular inode, there are two ways:
+- To disable compression on regular inode, there are three ways:

* chattr -c file
* mount w/ -o nocompress_extension=ext; touch file.ext
+ * echo '[n]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext

- Priority in between FS_COMPR_FL, FS_NOCOMP_FS, extensions:

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 3dc488ce882b..a5257882c772 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -20,6 +20,67 @@
#include "segment.h"
#include <trace/events/f2fs.h>

+static int is_compress_extension_exist(struct f2fs_sb_info *sbi,
+ unsigned char (*ext)[F2FS_EXTENSION_LEN],
+ int ext_cnt, unsigned char *new_ext)
+{
+ int i;
+
+ for (i = 0; i < ext_cnt; i++) {
+ if (!strcasecmp(new_ext, ext[i]))
+ return i;
+ }
+ return -1;
+}
+
+int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
+ unsigned char *new_ext, bool is_ext, bool set)
+{
+ unsigned char (*ext)[F2FS_EXTENSION_LEN];
+ unsigned char *ext_cnt;
+
+ if (is_ext) {
+ ext = F2FS_OPTION(sbi).extensions;
+ ext_cnt = &F2FS_OPTION(sbi).compress_ext_cnt;
+ } else {
+ ext = F2FS_OPTION(sbi).noextensions;
+ ext_cnt = &F2FS_OPTION(sbi).nocompress_ext_cnt;
+ }
+
+ if (set) {
+ if (*ext_cnt >= COMPRESS_EXT_NUM)
+ return -EINVAL;
+
+ if (is_compress_extension_exist(sbi,
+ F2FS_OPTION(sbi).extensions,
+ F2FS_OPTION(sbi).compress_ext_cnt,
+ new_ext) >= 0)
+ return -EEXIST;
+
+ if (is_compress_extension_exist(sbi,
+ F2FS_OPTION(sbi).noextensions,
+ F2FS_OPTION(sbi).nocompress_ext_cnt,
+ new_ext) >= 0)
+ return -EEXIST;
+
+ strcpy(ext[*ext_cnt], new_ext);
+ (*ext_cnt)++;
+ } else {
+ int pos = is_compress_extension_exist(sbi, ext,
+ *ext_cnt, new_ext);
+ if (pos < 0)
+ return -ENOENT;
+
+ if (pos < *ext_cnt - 1)
+ memmove(ext + pos, ext + pos + 1,
+ F2FS_EXTENSION_LEN * (*ext_cnt - pos - 1));
+ memset(ext + *ext_cnt - 1, 0, F2FS_EXTENSION_LEN);
+ (*ext_cnt)--;
+ }
+
+ return 0;
+}
+
static struct kmem_cache *cic_entry_slab;
static struct kmem_cache *dic_entry_slab;

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index c5e7460d1a0a..d44e2c43d8ab 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -186,7 +186,7 @@ struct f2fs_mount_info {
unsigned char compress_level; /* compress level */
bool compress_chksum; /* compressed data chksum */
unsigned char compress_ext_cnt; /* extension count */
- unsigned char nocompress_ext_cnt; /* nocompress extension count */
+ unsigned char nocompress_ext_cnt; /* nocompress extension count */
int compress_mode; /* compression mode */
unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
unsigned char noextensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
@@ -4273,6 +4273,8 @@ static inline bool f2fs_post_read_required(struct inode *inode)
* compress.c
*/
#ifdef CONFIG_F2FS_FS_COMPRESSION
+int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
+ unsigned char *new_ext, bool is_ext, bool set);
bool f2fs_is_compressed_page(struct page *page);
struct page *f2fs_compress_control_page(struct page *page);
int f2fs_prepare_compress_overwrite(struct inode *inode,
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index a7ec55c7bb20..a8f05a02e202 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -39,6 +39,7 @@ enum {
RESERVED_BLOCKS, /* struct f2fs_sb_info */
CPRC_INFO, /* struct ckpt_req_control */
ATGC_INFO, /* struct atgc_management */
+ MOUNT_INFO, /* struct f2fs_mount_info */
};

static const char *gc_mode_names[MAX_GC_MODE] = {
@@ -89,6 +90,8 @@ static unsigned char *__struct_ptr(struct f2fs_sb_info *sbi, int struct_type)
return (unsigned char *)&sbi->cprc_info;
else if (struct_type == ATGC_INFO)
return (unsigned char *)&sbi->am;
+ else if (struct_type == MOUNT_INFO)
+ return (unsigned char *)&F2FS_OPTION(sbi);
return NULL;
}

@@ -358,6 +361,25 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a,

if (!strcmp(a->attr.name, "compr_new_inode"))
return sysfs_emit(buf, "%u\n", sbi->compr_new_inode);
+
+ if (!strcmp(a->attr.name, "compress_extension")) {
+ int len = 0, i;
+
+ f2fs_down_read(&sbi->sb_lock);
+ len += scnprintf(buf + len, PAGE_SIZE - len,
+ "compress extension:\n");
+ for (i = 0; i < F2FS_OPTION(sbi).compress_ext_cnt; i++)
+ len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
+ F2FS_OPTION(sbi).extensions[i]);
+
+ len += scnprintf(buf + len, PAGE_SIZE - len,
+ "nocompress extension:\n");
+ for (i = 0; i < F2FS_OPTION(sbi).nocompress_ext_cnt; i++)
+ len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
+ F2FS_OPTION(sbi).noextensions[i]);
+ f2fs_up_read(&sbi->sb_lock);
+ return len;
+ }
#endif

if (!strcmp(a->attr.name, "gc_segment_mode"))
@@ -446,6 +468,35 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
return ret ? ret : count;
}

+#ifdef CONFIG_F2FS_FS_COMPRESSION
+ if (!strcmp(a->attr.name, "compress_extension")) {
+ char *name = strim((char *)buf);
+ bool set = true, cmpr;
+
+ if (!strncmp(name, "[c]", 3))
+ cmpr = true;
+ else if (!strncmp(name, "[n]", 3))
+ cmpr = false;
+ else
+ return -EINVAL;
+
+ name += 3;
+
+ if (*name == '!') {
+ name++;
+ set = false;
+ }
+
+ if (!strlen(name) || strlen(name) >= F2FS_EXTENSION_LEN)
+ return -EINVAL;
+
+ f2fs_down_write(&sbi->sb_lock);
+ ret = f2fs_update_compress_extension(sbi, name, cmpr, set);
+ f2fs_up_write(&sbi->sb_lock);
+ return ret ? ret : count;
+ }
+#endif
+
if (!strcmp(a->attr.name, "ckpt_thread_ioprio")) {
const char *name = strim((char *)buf);
struct ckpt_req_control *cprc = &sbi->cprc_info;
@@ -785,15 +836,16 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
const char *buf, size_t count)
{
ssize_t ret;
- bool gc_entry = (!strcmp(a->attr.name, "gc_urgent") ||
- a->struct_type == GC_THREAD);
+ bool need_lock = (!strcmp(a->attr.name, "gc_urgent") ||
+ a->struct_type == GC_THREAD ||
+ a->struct_type == MOUNT_INFO);

- if (gc_entry) {
+ if (need_lock) {
if (!down_read_trylock(&sbi->sb->s_umount))
return -EAGAIN;
}
ret = __sbi_store(a, sbi, buf, count);
- if (gc_entry)
+ if (need_lock)
up_read(&sbi->sb->s_umount);

return ret;
@@ -942,6 +994,9 @@ static struct f2fs_attr f2fs_attr_##name = __ATTR(name, 0444, name##_show, NULL)
#define ATGC_INFO_RW_ATTR(name, elname) \
F2FS_RW_ATTR(ATGC_INFO, atgc_management, name, elname)

+#define MOUNT_INFO_RW_ATTR(name, elname) \
+ F2FS_RW_ATTR(MOUNT_INFO, f2fs_mount_info, name, elname)
+
/* GC_THREAD ATTR */
GC_THREAD_RW_ATTR(gc_urgent_sleep_time, urgent_sleep_time);
GC_THREAD_RW_ATTR(gc_min_sleep_time, min_sleep_time);
@@ -1008,6 +1063,7 @@ F2FS_SBI_GENERAL_RW_ATTR(compr_saved_block);
F2FS_SBI_GENERAL_RW_ATTR(compr_new_inode);
F2FS_SBI_GENERAL_RW_ATTR(compress_percent);
F2FS_SBI_GENERAL_RW_ATTR(compress_watermark);
+MOUNT_INFO_RW_ATTR(compress_extension, extensions);
#endif
/* atomic write */
F2FS_SBI_GENERAL_RO_ATTR(current_atomic_write);
@@ -1181,6 +1237,7 @@ static struct attribute *f2fs_attrs[] = {
ATTR_LIST(compr_new_inode),
ATTR_LIST(compress_percent),
ATTR_LIST(compress_watermark),
+ ATTR_LIST(compress_extension),
#endif
/* For ATGC */
ATTR_LIST(atgc_candidate_ratio),
--
2.40.1



2024-02-08 00:07:47

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH] f2fs: support compress extension update via sysfs interface

On 02/07, Chao Yu wrote:
> Introduce /sys/fs/f2fs/<disk>/compress_extension to support
> adding/deleting compress extension via sysfs interface, in
> comparison to mount option, it's more easy to use and less
> authority issue for applications.
>
> Usage:
> - Query: cat /sys/fs/f2fs/<disk>/compress_extension
> - Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
> - Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
> - [c] means add/del compress extension
> - [n] means add/del nocompress extension
>
> Signed-off-by: Sheng Yong <[email protected]>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-fs-f2fs | 10 ++++
> Documentation/filesystems/f2fs.rst | 6 ++-
> fs/f2fs/compress.c | 61 +++++++++++++++++++++++
> fs/f2fs/f2fs.h | 4 +-
> fs/f2fs/sysfs.c | 65 +++++++++++++++++++++++--
> 5 files changed, 139 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 48c135e24eb5..1f2cc0913e45 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -762,3 +762,13 @@ Date: November 2023
> Contact: "Chao Yu" <[email protected]>
> Description: It controls to enable/disable IO aware feature for background discard.
> By default, the value is 1 which indicates IO aware is on.
> +
> +What: /sys/fs/f2fs/<disk>/compress_extension
> +Date: October 2023
> +Contact: "Chao Yu" <[email protected]>
> +Description: Used to control configure [|no]compress_extension list:
> + - Query: cat /sys/fs/f2fs/<disk>/compress_extension
> + - Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
> + - Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
> + - [c] means add/del compress extension
> + - [n] means add/del nocompress extension
> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> index 32cbfa864f38..c82a8fd7316b 100644
> --- a/Documentation/filesystems/f2fs.rst
> +++ b/Documentation/filesystems/f2fs.rst
> @@ -821,17 +821,19 @@ Compression implementation
> all logical blocks in cluster contain valid data and compress ratio of
> cluster data is lower than specified threshold.
>
> -- To enable compression on regular inode, there are four ways:
> +- To enable compression on regular inode, there are five ways:
>
> * chattr +c file
> * chattr +c dir; touch dir/file
> * mount w/ -o compress_extension=ext; touch file.ext
> * mount w/ -o compress_extension=*; touch any_file
> + * echo '[c]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext
>
> -- To disable compression on regular inode, there are two ways:
> +- To disable compression on regular inode, there are three ways:
>
> * chattr -c file
> * mount w/ -o nocompress_extension=ext; touch file.ext
> + * echo '[n]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext
>
> - Priority in between FS_COMPR_FL, FS_NOCOMP_FS, extensions:
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 3dc488ce882b..a5257882c772 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -20,6 +20,67 @@
> #include "segment.h"
> #include <trace/events/f2fs.h>
>
> +static int is_compress_extension_exist(struct f2fs_sb_info *sbi,
> + unsigned char (*ext)[F2FS_EXTENSION_LEN],
> + int ext_cnt, unsigned char *new_ext)
> +{
> + int i;
> +
> + for (i = 0; i < ext_cnt; i++) {
> + if (!strcasecmp(new_ext, ext[i]))
> + return i;
> + }
> + return -1;
> +}
> +
> +int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
> + unsigned char *new_ext, bool is_ext, bool set)
> +{
> + unsigned char (*ext)[F2FS_EXTENSION_LEN];
> + unsigned char *ext_cnt;
> +
> + if (is_ext) {
> + ext = F2FS_OPTION(sbi).extensions;
> + ext_cnt = &F2FS_OPTION(sbi).compress_ext_cnt;
> + } else {
> + ext = F2FS_OPTION(sbi).noextensions;
> + ext_cnt = &F2FS_OPTION(sbi).nocompress_ext_cnt;
> + }
> +
> + if (set) {
> + if (*ext_cnt >= COMPRESS_EXT_NUM)
> + return -EINVAL;
> +
> + if (is_compress_extension_exist(sbi,
> + F2FS_OPTION(sbi).extensions,
> + F2FS_OPTION(sbi).compress_ext_cnt,
> + new_ext) >= 0)
> + return -EEXIST;
> +
> + if (is_compress_extension_exist(sbi,
> + F2FS_OPTION(sbi).noextensions,
> + F2FS_OPTION(sbi).nocompress_ext_cnt,
> + new_ext) >= 0)
> + return -EEXIST;
> +
> + strcpy(ext[*ext_cnt], new_ext);
> + (*ext_cnt)++;
> + } else {
> + int pos = is_compress_extension_exist(sbi, ext,
> + *ext_cnt, new_ext);
> + if (pos < 0)
> + return -ENOENT;
> +
> + if (pos < *ext_cnt - 1)
> + memmove(ext + pos, ext + pos + 1,
> + F2FS_EXTENSION_LEN * (*ext_cnt - pos - 1));
> + memset(ext + *ext_cnt - 1, 0, F2FS_EXTENSION_LEN);
> + (*ext_cnt)--;
> + }
> +
> + return 0;
> +}
> +
> static struct kmem_cache *cic_entry_slab;
> static struct kmem_cache *dic_entry_slab;
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index c5e7460d1a0a..d44e2c43d8ab 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -186,7 +186,7 @@ struct f2fs_mount_info {
> unsigned char compress_level; /* compress level */
> bool compress_chksum; /* compressed data chksum */
> unsigned char compress_ext_cnt; /* extension count */
> - unsigned char nocompress_ext_cnt; /* nocompress extension count */
> + unsigned char nocompress_ext_cnt; /* nocompress extension count */
> int compress_mode; /* compression mode */
> unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
> unsigned char noextensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
> @@ -4273,6 +4273,8 @@ static inline bool f2fs_post_read_required(struct inode *inode)
> * compress.c
> */
> #ifdef CONFIG_F2FS_FS_COMPRESSION
> +int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
> + unsigned char *new_ext, bool is_ext, bool set);
> bool f2fs_is_compressed_page(struct page *page);
> struct page *f2fs_compress_control_page(struct page *page);
> int f2fs_prepare_compress_overwrite(struct inode *inode,
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index a7ec55c7bb20..a8f05a02e202 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -39,6 +39,7 @@ enum {
> RESERVED_BLOCKS, /* struct f2fs_sb_info */
> CPRC_INFO, /* struct ckpt_req_control */
> ATGC_INFO, /* struct atgc_management */
> + MOUNT_INFO, /* struct f2fs_mount_info */
> };
>
> static const char *gc_mode_names[MAX_GC_MODE] = {
> @@ -89,6 +90,8 @@ static unsigned char *__struct_ptr(struct f2fs_sb_info *sbi, int struct_type)
> return (unsigned char *)&sbi->cprc_info;
> else if (struct_type == ATGC_INFO)
> return (unsigned char *)&sbi->am;
> + else if (struct_type == MOUNT_INFO)
> + return (unsigned char *)&F2FS_OPTION(sbi);
> return NULL;
> }
>
> @@ -358,6 +361,25 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>
> if (!strcmp(a->attr.name, "compr_new_inode"))
> return sysfs_emit(buf, "%u\n", sbi->compr_new_inode);
> +
> + if (!strcmp(a->attr.name, "compress_extension")) {
> + int len = 0, i;
> +
> + f2fs_down_read(&sbi->sb_lock);
> + len += scnprintf(buf + len, PAGE_SIZE - len,
> + "compress extension:\n");
> + for (i = 0; i < F2FS_OPTION(sbi).compress_ext_cnt; i++)
> + len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
> + F2FS_OPTION(sbi).extensions[i]);
> +
> + len += scnprintf(buf + len, PAGE_SIZE - len,
> + "nocompress extension:\n");
> + for (i = 0; i < F2FS_OPTION(sbi).nocompress_ext_cnt; i++)
> + len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
> + F2FS_OPTION(sbi).noextensions[i]);

I don't think this is acceptable in sysfs.

> + f2fs_up_read(&sbi->sb_lock);
> + return len;
> + }
> #endif
>
> if (!strcmp(a->attr.name, "gc_segment_mode"))
> @@ -446,6 +468,35 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
> return ret ? ret : count;
> }
>
> +#ifdef CONFIG_F2FS_FS_COMPRESSION
> + if (!strcmp(a->attr.name, "compress_extension")) {
> + char *name = strim((char *)buf);
> + bool set = true, cmpr;
> +
> + if (!strncmp(name, "[c]", 3))
> + cmpr = true;
> + else if (!strncmp(name, "[n]", 3))
> + cmpr = false;
> + else
> + return -EINVAL;
> +
> + name += 3;
> +
> + if (*name == '!') {
> + name++;
> + set = false;
> + }
> +
> + if (!strlen(name) || strlen(name) >= F2FS_EXTENSION_LEN)
> + return -EINVAL;
> +
> + f2fs_down_write(&sbi->sb_lock);
> + ret = f2fs_update_compress_extension(sbi, name, cmpr, set);
> + f2fs_up_write(&sbi->sb_lock);
> + return ret ? ret : count;
> + }
> +#endif
> +
> if (!strcmp(a->attr.name, "ckpt_thread_ioprio")) {
> const char *name = strim((char *)buf);
> struct ckpt_req_control *cprc = &sbi->cprc_info;
> @@ -785,15 +836,16 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
> const char *buf, size_t count)
> {
> ssize_t ret;
> - bool gc_entry = (!strcmp(a->attr.name, "gc_urgent") ||
> - a->struct_type == GC_THREAD);
> + bool need_lock = (!strcmp(a->attr.name, "gc_urgent") ||
> + a->struct_type == GC_THREAD ||
> + a->struct_type == MOUNT_INFO);
>
> - if (gc_entry) {
> + if (need_lock) {
> if (!down_read_trylock(&sbi->sb->s_umount))
> return -EAGAIN;
> }
> ret = __sbi_store(a, sbi, buf, count);
> - if (gc_entry)
> + if (need_lock)
> up_read(&sbi->sb->s_umount);
>
> return ret;
> @@ -942,6 +994,9 @@ static struct f2fs_attr f2fs_attr_##name = __ATTR(name, 0444, name##_show, NULL)
> #define ATGC_INFO_RW_ATTR(name, elname) \
> F2FS_RW_ATTR(ATGC_INFO, atgc_management, name, elname)
>
> +#define MOUNT_INFO_RW_ATTR(name, elname) \
> + F2FS_RW_ATTR(MOUNT_INFO, f2fs_mount_info, name, elname)
> +
> /* GC_THREAD ATTR */
> GC_THREAD_RW_ATTR(gc_urgent_sleep_time, urgent_sleep_time);
> GC_THREAD_RW_ATTR(gc_min_sleep_time, min_sleep_time);
> @@ -1008,6 +1063,7 @@ F2FS_SBI_GENERAL_RW_ATTR(compr_saved_block);
> F2FS_SBI_GENERAL_RW_ATTR(compr_new_inode);
> F2FS_SBI_GENERAL_RW_ATTR(compress_percent);
> F2FS_SBI_GENERAL_RW_ATTR(compress_watermark);
> +MOUNT_INFO_RW_ATTR(compress_extension, extensions);
> #endif
> /* atomic write */
> F2FS_SBI_GENERAL_RO_ATTR(current_atomic_write);
> @@ -1181,6 +1237,7 @@ static struct attribute *f2fs_attrs[] = {
> ATTR_LIST(compr_new_inode),
> ATTR_LIST(compress_percent),
> ATTR_LIST(compress_watermark),
> + ATTR_LIST(compress_extension),
> #endif
> /* For ATGC */
> ATTR_LIST(atgc_candidate_ratio),
> --
> 2.40.1

2024-02-19 06:14:52

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH] f2fs: support compress extension update via sysfs interface

On 2024/2/8 8:07, Jaegeuk Kim wrote:
> On 02/07, Chao Yu wrote:
>> Introduce /sys/fs/f2fs/<disk>/compress_extension to support
>> adding/deleting compress extension via sysfs interface, in
>> comparison to mount option, it's more easy to use and less
>> authority issue for applications.
>>
>> Usage:
>> - Query: cat /sys/fs/f2fs/<disk>/compress_extension
>> - Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
>> - Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
>> - [c] means add/del compress extension
>> - [n] means add/del nocompress extension
>>
>> Signed-off-by: Sheng Yong <[email protected]>
>> Signed-off-by: Chao Yu <[email protected]>
>> ---
>> Documentation/ABI/testing/sysfs-fs-f2fs | 10 ++++
>> Documentation/filesystems/f2fs.rst | 6 ++-
>> fs/f2fs/compress.c | 61 +++++++++++++++++++++++
>> fs/f2fs/f2fs.h | 4 +-
>> fs/f2fs/sysfs.c | 65 +++++++++++++++++++++++--
>> 5 files changed, 139 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>> index 48c135e24eb5..1f2cc0913e45 100644
>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>> @@ -762,3 +762,13 @@ Date: November 2023
>> Contact: "Chao Yu" <[email protected]>
>> Description: It controls to enable/disable IO aware feature for background discard.
>> By default, the value is 1 which indicates IO aware is on.
>> +
>> +What: /sys/fs/f2fs/<disk>/compress_extension
>> +Date: October 2023
>> +Contact: "Chao Yu" <[email protected]>
>> +Description: Used to control configure [|no]compress_extension list:
>> + - Query: cat /sys/fs/f2fs/<disk>/compress_extension
>> + - Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
>> + - Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
>> + - [c] means add/del compress extension
>> + - [n] means add/del nocompress extension
>> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
>> index 32cbfa864f38..c82a8fd7316b 100644
>> --- a/Documentation/filesystems/f2fs.rst
>> +++ b/Documentation/filesystems/f2fs.rst
>> @@ -821,17 +821,19 @@ Compression implementation
>> all logical blocks in cluster contain valid data and compress ratio of
>> cluster data is lower than specified threshold.
>>
>> -- To enable compression on regular inode, there are four ways:
>> +- To enable compression on regular inode, there are five ways:
>>
>> * chattr +c file
>> * chattr +c dir; touch dir/file
>> * mount w/ -o compress_extension=ext; touch file.ext
>> * mount w/ -o compress_extension=*; touch any_file
>> + * echo '[c]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext
>>
>> -- To disable compression on regular inode, there are two ways:
>> +- To disable compression on regular inode, there are three ways:
>>
>> * chattr -c file
>> * mount w/ -o nocompress_extension=ext; touch file.ext
>> + * echo '[n]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext
>>
>> - Priority in between FS_COMPR_FL, FS_NOCOMP_FS, extensions:
>>
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>> index 3dc488ce882b..a5257882c772 100644
>> --- a/fs/f2fs/compress.c
>> +++ b/fs/f2fs/compress.c
>> @@ -20,6 +20,67 @@
>> #include "segment.h"
>> #include <trace/events/f2fs.h>
>>
>> +static int is_compress_extension_exist(struct f2fs_sb_info *sbi,
>> + unsigned char (*ext)[F2FS_EXTENSION_LEN],
>> + int ext_cnt, unsigned char *new_ext)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ext_cnt; i++) {
>> + if (!strcasecmp(new_ext, ext[i]))
>> + return i;
>> + }
>> + return -1;
>> +}
>> +
>> +int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
>> + unsigned char *new_ext, bool is_ext, bool set)
>> +{
>> + unsigned char (*ext)[F2FS_EXTENSION_LEN];
>> + unsigned char *ext_cnt;
>> +
>> + if (is_ext) {
>> + ext = F2FS_OPTION(sbi).extensions;
>> + ext_cnt = &F2FS_OPTION(sbi).compress_ext_cnt;
>> + } else {
>> + ext = F2FS_OPTION(sbi).noextensions;
>> + ext_cnt = &F2FS_OPTION(sbi).nocompress_ext_cnt;
>> + }
>> +
>> + if (set) {
>> + if (*ext_cnt >= COMPRESS_EXT_NUM)
>> + return -EINVAL;
>> +
>> + if (is_compress_extension_exist(sbi,
>> + F2FS_OPTION(sbi).extensions,
>> + F2FS_OPTION(sbi).compress_ext_cnt,
>> + new_ext) >= 0)
>> + return -EEXIST;
>> +
>> + if (is_compress_extension_exist(sbi,
>> + F2FS_OPTION(sbi).noextensions,
>> + F2FS_OPTION(sbi).nocompress_ext_cnt,
>> + new_ext) >= 0)
>> + return -EEXIST;
>> +
>> + strcpy(ext[*ext_cnt], new_ext);
>> + (*ext_cnt)++;
>> + } else {
>> + int pos = is_compress_extension_exist(sbi, ext,
>> + *ext_cnt, new_ext);
>> + if (pos < 0)
>> + return -ENOENT;
>> +
>> + if (pos < *ext_cnt - 1)
>> + memmove(ext + pos, ext + pos + 1,
>> + F2FS_EXTENSION_LEN * (*ext_cnt - pos - 1));
>> + memset(ext + *ext_cnt - 1, 0, F2FS_EXTENSION_LEN);
>> + (*ext_cnt)--;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static struct kmem_cache *cic_entry_slab;
>> static struct kmem_cache *dic_entry_slab;
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index c5e7460d1a0a..d44e2c43d8ab 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -186,7 +186,7 @@ struct f2fs_mount_info {
>> unsigned char compress_level; /* compress level */
>> bool compress_chksum; /* compressed data chksum */
>> unsigned char compress_ext_cnt; /* extension count */
>> - unsigned char nocompress_ext_cnt; /* nocompress extension count */
>> + unsigned char nocompress_ext_cnt; /* nocompress extension count */
>> int compress_mode; /* compression mode */
>> unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
>> unsigned char noextensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
>> @@ -4273,6 +4273,8 @@ static inline bool f2fs_post_read_required(struct inode *inode)
>> * compress.c
>> */
>> #ifdef CONFIG_F2FS_FS_COMPRESSION
>> +int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
>> + unsigned char *new_ext, bool is_ext, bool set);
>> bool f2fs_is_compressed_page(struct page *page);
>> struct page *f2fs_compress_control_page(struct page *page);
>> int f2fs_prepare_compress_overwrite(struct inode *inode,
>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>> index a7ec55c7bb20..a8f05a02e202 100644
>> --- a/fs/f2fs/sysfs.c
>> +++ b/fs/f2fs/sysfs.c
>> @@ -39,6 +39,7 @@ enum {
>> RESERVED_BLOCKS, /* struct f2fs_sb_info */
>> CPRC_INFO, /* struct ckpt_req_control */
>> ATGC_INFO, /* struct atgc_management */
>> + MOUNT_INFO, /* struct f2fs_mount_info */
>> };
>>
>> static const char *gc_mode_names[MAX_GC_MODE] = {
>> @@ -89,6 +90,8 @@ static unsigned char *__struct_ptr(struct f2fs_sb_info *sbi, int struct_type)
>> return (unsigned char *)&sbi->cprc_info;
>> else if (struct_type == ATGC_INFO)
>> return (unsigned char *)&sbi->am;
>> + else if (struct_type == MOUNT_INFO)
>> + return (unsigned char *)&F2FS_OPTION(sbi);
>> return NULL;
>> }
>>
>> @@ -358,6 +361,25 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>>
>> if (!strcmp(a->attr.name, "compr_new_inode"))
>> return sysfs_emit(buf, "%u\n", sbi->compr_new_inode);
>> +
>> + if (!strcmp(a->attr.name, "compress_extension")) {
>> + int len = 0, i;
>> +
>> + f2fs_down_read(&sbi->sb_lock);
>> + len += scnprintf(buf + len, PAGE_SIZE - len,
>> + "compress extension:\n");
>> + for (i = 0; i < F2FS_OPTION(sbi).compress_ext_cnt; i++)
>> + len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
>> + F2FS_OPTION(sbi).extensions[i]);
>> +
>> + len += scnprintf(buf + len, PAGE_SIZE - len,
>> + "nocompress extension:\n");
>> + for (i = 0; i < F2FS_OPTION(sbi).nocompress_ext_cnt; i++)
>> + len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
>> + F2FS_OPTION(sbi).noextensions[i]);
>
> I don't think this is acceptable in sysfs.

It looks block layer output similar string:

cat /sys/block/<dev>/queue/scheduler
mq-deadline kyber [bfq] none

Or, can we add extension entries into /sys/fs/f2fs/<dev>/compress_extensions/
directory? like feature entries in /sys/fs/f2fs/<dev>/feature_list/.

e.g.

ls /sys/fs/f2fs/<dev>/compress_extensions/
so dex

Show enable/disable to indicate current extension is in
compress_extension/nocompress_extension array.

cat /sys/fs/f2fs/<dev>/compress_extensions/so
enable

cat /sys/fs/f2fs/<dev>/compress_extensions/dex
disable

Thanks,

>
>> + f2fs_up_read(&sbi->sb_lock);
>> + return len;
>> + }
>> #endif
>>
>> if (!strcmp(a->attr.name, "gc_segment_mode"))
>> @@ -446,6 +468,35 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
>> return ret ? ret : count;
>> }
>>
>> +#ifdef CONFIG_F2FS_FS_COMPRESSION
>> + if (!strcmp(a->attr.name, "compress_extension")) {
>> + char *name = strim((char *)buf);
>> + bool set = true, cmpr;
>> +
>> + if (!strncmp(name, "[c]", 3))
>> + cmpr = true;
>> + else if (!strncmp(name, "[n]", 3))
>> + cmpr = false;
>> + else
>> + return -EINVAL;
>> +
>> + name += 3;
>> +
>> + if (*name == '!') {
>> + name++;
>> + set = false;
>> + }
>> +
>> + if (!strlen(name) || strlen(name) >= F2FS_EXTENSION_LEN)
>> + return -EINVAL;
>> +
>> + f2fs_down_write(&sbi->sb_lock);
>> + ret = f2fs_update_compress_extension(sbi, name, cmpr, set);
>> + f2fs_up_write(&sbi->sb_lock);
>> + return ret ? ret : count;
>> + }
>> +#endif
>> +
>> if (!strcmp(a->attr.name, "ckpt_thread_ioprio")) {
>> const char *name = strim((char *)buf);
>> struct ckpt_req_control *cprc = &sbi->cprc_info;
>> @@ -785,15 +836,16 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>> const char *buf, size_t count)
>> {
>> ssize_t ret;
>> - bool gc_entry = (!strcmp(a->attr.name, "gc_urgent") ||
>> - a->struct_type == GC_THREAD);
>> + bool need_lock = (!strcmp(a->attr.name, "gc_urgent") ||
>> + a->struct_type == GC_THREAD ||
>> + a->struct_type == MOUNT_INFO);
>>
>> - if (gc_entry) {
>> + if (need_lock) {
>> if (!down_read_trylock(&sbi->sb->s_umount))
>> return -EAGAIN;
>> }
>> ret = __sbi_store(a, sbi, buf, count);
>> - if (gc_entry)
>> + if (need_lock)
>> up_read(&sbi->sb->s_umount);
>>
>> return ret;
>> @@ -942,6 +994,9 @@ static struct f2fs_attr f2fs_attr_##name = __ATTR(name, 0444, name##_show, NULL)
>> #define ATGC_INFO_RW_ATTR(name, elname) \
>> F2FS_RW_ATTR(ATGC_INFO, atgc_management, name, elname)
>>
>> +#define MOUNT_INFO_RW_ATTR(name, elname) \
>> + F2FS_RW_ATTR(MOUNT_INFO, f2fs_mount_info, name, elname)
>> +
>> /* GC_THREAD ATTR */
>> GC_THREAD_RW_ATTR(gc_urgent_sleep_time, urgent_sleep_time);
>> GC_THREAD_RW_ATTR(gc_min_sleep_time, min_sleep_time);
>> @@ -1008,6 +1063,7 @@ F2FS_SBI_GENERAL_RW_ATTR(compr_saved_block);
>> F2FS_SBI_GENERAL_RW_ATTR(compr_new_inode);
>> F2FS_SBI_GENERAL_RW_ATTR(compress_percent);
>> F2FS_SBI_GENERAL_RW_ATTR(compress_watermark);
>> +MOUNT_INFO_RW_ATTR(compress_extension, extensions);
>> #endif
>> /* atomic write */
>> F2FS_SBI_GENERAL_RO_ATTR(current_atomic_write);
>> @@ -1181,6 +1237,7 @@ static struct attribute *f2fs_attrs[] = {
>> ATTR_LIST(compr_new_inode),
>> ATTR_LIST(compress_percent),
>> ATTR_LIST(compress_watermark),
>> + ATTR_LIST(compress_extension),
>> #endif
>> /* For ATGC */
>> ATTR_LIST(atgc_candidate_ratio),
>> --
>> 2.40.1

2024-02-20 19:28:50

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH] f2fs: support compress extension update via sysfs interface

On 02/19, Chao Yu wrote:
> On 2024/2/8 8:07, Jaegeuk Kim wrote:
> > On 02/07, Chao Yu wrote:
> > > Introduce /sys/fs/f2fs/<disk>/compress_extension to support
> > > adding/deleting compress extension via sysfs interface, in
> > > comparison to mount option, it's more easy to use and less
> > > authority issue for applications.
> > >
> > > Usage:
> > > - Query: cat /sys/fs/f2fs/<disk>/compress_extension
> > > - Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
> > > - Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
> > > - [c] means add/del compress extension
> > > - [n] means add/del nocompress extension
> > >
> > > Signed-off-by: Sheng Yong <[email protected]>
> > > Signed-off-by: Chao Yu <[email protected]>
> > > ---
> > > Documentation/ABI/testing/sysfs-fs-f2fs | 10 ++++
> > > Documentation/filesystems/f2fs.rst | 6 ++-
> > > fs/f2fs/compress.c | 61 +++++++++++++++++++++++
> > > fs/f2fs/f2fs.h | 4 +-
> > > fs/f2fs/sysfs.c | 65 +++++++++++++++++++++++--
> > > 5 files changed, 139 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> > > index 48c135e24eb5..1f2cc0913e45 100644
> > > --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> > > +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> > > @@ -762,3 +762,13 @@ Date: November 2023
> > > Contact: "Chao Yu" <[email protected]>
> > > Description: It controls to enable/disable IO aware feature for background discard.
> > > By default, the value is 1 which indicates IO aware is on.
> > > +
> > > +What: /sys/fs/f2fs/<disk>/compress_extension
> > > +Date: October 2023
> > > +Contact: "Chao Yu" <[email protected]>
> > > +Description: Used to control configure [|no]compress_extension list:
> > > + - Query: cat /sys/fs/f2fs/<disk>/compress_extension
> > > + - Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
> > > + - Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
> > > + - [c] means add/del compress extension
> > > + - [n] means add/del nocompress extension
> > > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> > > index 32cbfa864f38..c82a8fd7316b 100644
> > > --- a/Documentation/filesystems/f2fs.rst
> > > +++ b/Documentation/filesystems/f2fs.rst
> > > @@ -821,17 +821,19 @@ Compression implementation
> > > all logical blocks in cluster contain valid data and compress ratio of
> > > cluster data is lower than specified threshold.
> > > -- To enable compression on regular inode, there are four ways:
> > > +- To enable compression on regular inode, there are five ways:
> > > * chattr +c file
> > > * chattr +c dir; touch dir/file
> > > * mount w/ -o compress_extension=ext; touch file.ext
> > > * mount w/ -o compress_extension=*; touch any_file
> > > + * echo '[c]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext
> > > -- To disable compression on regular inode, there are two ways:
> > > +- To disable compression on regular inode, there are three ways:
> > > * chattr -c file
> > > * mount w/ -o nocompress_extension=ext; touch file.ext
> > > + * echo '[n]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext
> > > - Priority in between FS_COMPR_FL, FS_NOCOMP_FS, extensions:
> > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > > index 3dc488ce882b..a5257882c772 100644
> > > --- a/fs/f2fs/compress.c
> > > +++ b/fs/f2fs/compress.c
> > > @@ -20,6 +20,67 @@
> > > #include "segment.h"
> > > #include <trace/events/f2fs.h>
> > > +static int is_compress_extension_exist(struct f2fs_sb_info *sbi,
> > > + unsigned char (*ext)[F2FS_EXTENSION_LEN],
> > > + int ext_cnt, unsigned char *new_ext)
> > > +{
> > > + int i;
> > > +
> > > + for (i = 0; i < ext_cnt; i++) {
> > > + if (!strcasecmp(new_ext, ext[i]))
> > > + return i;
> > > + }
> > > + return -1;
> > > +}
> > > +
> > > +int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
> > > + unsigned char *new_ext, bool is_ext, bool set)
> > > +{
> > > + unsigned char (*ext)[F2FS_EXTENSION_LEN];
> > > + unsigned char *ext_cnt;
> > > +
> > > + if (is_ext) {
> > > + ext = F2FS_OPTION(sbi).extensions;
> > > + ext_cnt = &F2FS_OPTION(sbi).compress_ext_cnt;
> > > + } else {
> > > + ext = F2FS_OPTION(sbi).noextensions;
> > > + ext_cnt = &F2FS_OPTION(sbi).nocompress_ext_cnt;
> > > + }
> > > +
> > > + if (set) {
> > > + if (*ext_cnt >= COMPRESS_EXT_NUM)
> > > + return -EINVAL;
> > > +
> > > + if (is_compress_extension_exist(sbi,
> > > + F2FS_OPTION(sbi).extensions,
> > > + F2FS_OPTION(sbi).compress_ext_cnt,
> > > + new_ext) >= 0)
> > > + return -EEXIST;
> > > +
> > > + if (is_compress_extension_exist(sbi,
> > > + F2FS_OPTION(sbi).noextensions,
> > > + F2FS_OPTION(sbi).nocompress_ext_cnt,
> > > + new_ext) >= 0)
> > > + return -EEXIST;
> > > +
> > > + strcpy(ext[*ext_cnt], new_ext);
> > > + (*ext_cnt)++;
> > > + } else {
> > > + int pos = is_compress_extension_exist(sbi, ext,
> > > + *ext_cnt, new_ext);
> > > + if (pos < 0)
> > > + return -ENOENT;
> > > +
> > > + if (pos < *ext_cnt - 1)
> > > + memmove(ext + pos, ext + pos + 1,
> > > + F2FS_EXTENSION_LEN * (*ext_cnt - pos - 1));
> > > + memset(ext + *ext_cnt - 1, 0, F2FS_EXTENSION_LEN);
> > > + (*ext_cnt)--;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static struct kmem_cache *cic_entry_slab;
> > > static struct kmem_cache *dic_entry_slab;
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index c5e7460d1a0a..d44e2c43d8ab 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -186,7 +186,7 @@ struct f2fs_mount_info {
> > > unsigned char compress_level; /* compress level */
> > > bool compress_chksum; /* compressed data chksum */
> > > unsigned char compress_ext_cnt; /* extension count */
> > > - unsigned char nocompress_ext_cnt; /* nocompress extension count */
> > > + unsigned char nocompress_ext_cnt; /* nocompress extension count */
> > > int compress_mode; /* compression mode */
> > > unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
> > > unsigned char noextensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
> > > @@ -4273,6 +4273,8 @@ static inline bool f2fs_post_read_required(struct inode *inode)
> > > * compress.c
> > > */
> > > #ifdef CONFIG_F2FS_FS_COMPRESSION
> > > +int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
> > > + unsigned char *new_ext, bool is_ext, bool set);
> > > bool f2fs_is_compressed_page(struct page *page);
> > > struct page *f2fs_compress_control_page(struct page *page);
> > > int f2fs_prepare_compress_overwrite(struct inode *inode,
> > > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> > > index a7ec55c7bb20..a8f05a02e202 100644
> > > --- a/fs/f2fs/sysfs.c
> > > +++ b/fs/f2fs/sysfs.c
> > > @@ -39,6 +39,7 @@ enum {
> > > RESERVED_BLOCKS, /* struct f2fs_sb_info */
> > > CPRC_INFO, /* struct ckpt_req_control */
> > > ATGC_INFO, /* struct atgc_management */
> > > + MOUNT_INFO, /* struct f2fs_mount_info */
> > > };
> > > static const char *gc_mode_names[MAX_GC_MODE] = {
> > > @@ -89,6 +90,8 @@ static unsigned char *__struct_ptr(struct f2fs_sb_info *sbi, int struct_type)
> > > return (unsigned char *)&sbi->cprc_info;
> > > else if (struct_type == ATGC_INFO)
> > > return (unsigned char *)&sbi->am;
> > > + else if (struct_type == MOUNT_INFO)
> > > + return (unsigned char *)&F2FS_OPTION(sbi);
> > > return NULL;
> > > }
> > > @@ -358,6 +361,25 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
> > > if (!strcmp(a->attr.name, "compr_new_inode"))
> > > return sysfs_emit(buf, "%u\n", sbi->compr_new_inode);
> > > +
> > > + if (!strcmp(a->attr.name, "compress_extension")) {
> > > + int len = 0, i;
> > > +
> > > + f2fs_down_read(&sbi->sb_lock);
> > > + len += scnprintf(buf + len, PAGE_SIZE - len,
> > > + "compress extension:\n");
> > > + for (i = 0; i < F2FS_OPTION(sbi).compress_ext_cnt; i++)
> > > + len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
> > > + F2FS_OPTION(sbi).extensions[i]);
> > > +
> > > + len += scnprintf(buf + len, PAGE_SIZE - len,
> > > + "nocompress extension:\n");
> > > + for (i = 0; i < F2FS_OPTION(sbi).nocompress_ext_cnt; i++)
> > > + len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
> > > + F2FS_OPTION(sbi).noextensions[i]);
> >
> > I don't think this is acceptable in sysfs.
>
> It looks block layer output similar string:
>
> cat /sys/block/<dev>/queue/scheduler
> mq-deadline kyber [bfq] none
>
> Or, can we add extension entries into /sys/fs/f2fs/<dev>/compress_extensions/
> directory? like feature entries in /sys/fs/f2fs/<dev>/feature_list/.
>
> e.g.
>
> ls /sys/fs/f2fs/<dev>/compress_extensions/
> so dex
>
> Show enable/disable to indicate current extension is in
> compress_extension/nocompress_extension array.
>
> cat /sys/fs/f2fs/<dev>/compress_extensions/so
> enable
>
> cat /sys/fs/f2fs/<dev>/compress_extensions/dex
> disable

How about adding all the file extentions in a separate entries and eash showing
the hot/cold/compression information together?

>
> Thanks,
>
> >
> > > + f2fs_up_read(&sbi->sb_lock);
> > > + return len;
> > > + }
> > > #endif
> > > if (!strcmp(a->attr.name, "gc_segment_mode"))
> > > @@ -446,6 +468,35 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
> > > return ret ? ret : count;
> > > }
> > > +#ifdef CONFIG_F2FS_FS_COMPRESSION
> > > + if (!strcmp(a->attr.name, "compress_extension")) {
> > > + char *name = strim((char *)buf);
> > > + bool set = true, cmpr;
> > > +
> > > + if (!strncmp(name, "[c]", 3))
> > > + cmpr = true;
> > > + else if (!strncmp(name, "[n]", 3))
> > > + cmpr = false;
> > > + else
> > > + return -EINVAL;
> > > +
> > > + name += 3;
> > > +
> > > + if (*name == '!') {
> > > + name++;
> > > + set = false;
> > > + }
> > > +
> > > + if (!strlen(name) || strlen(name) >= F2FS_EXTENSION_LEN)
> > > + return -EINVAL;
> > > +
> > > + f2fs_down_write(&sbi->sb_lock);
> > > + ret = f2fs_update_compress_extension(sbi, name, cmpr, set);
> > > + f2fs_up_write(&sbi->sb_lock);
> > > + return ret ? ret : count;
> > > + }
> > > +#endif
> > > +
> > > if (!strcmp(a->attr.name, "ckpt_thread_ioprio")) {
> > > const char *name = strim((char *)buf);
> > > struct ckpt_req_control *cprc = &sbi->cprc_info;
> > > @@ -785,15 +836,16 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
> > > const char *buf, size_t count)
> > > {
> > > ssize_t ret;
> > > - bool gc_entry = (!strcmp(a->attr.name, "gc_urgent") ||
> > > - a->struct_type == GC_THREAD);
> > > + bool need_lock = (!strcmp(a->attr.name, "gc_urgent") ||
> > > + a->struct_type == GC_THREAD ||
> > > + a->struct_type == MOUNT_INFO);
> > > - if (gc_entry) {
> > > + if (need_lock) {
> > > if (!down_read_trylock(&sbi->sb->s_umount))
> > > return -EAGAIN;
> > > }
> > > ret = __sbi_store(a, sbi, buf, count);
> > > - if (gc_entry)
> > > + if (need_lock)
> > > up_read(&sbi->sb->s_umount);
> > > return ret;
> > > @@ -942,6 +994,9 @@ static struct f2fs_attr f2fs_attr_##name = __ATTR(name, 0444, name##_show, NULL)
> > > #define ATGC_INFO_RW_ATTR(name, elname) \
> > > F2FS_RW_ATTR(ATGC_INFO, atgc_management, name, elname)
> > > +#define MOUNT_INFO_RW_ATTR(name, elname) \
> > > + F2FS_RW_ATTR(MOUNT_INFO, f2fs_mount_info, name, elname)
> > > +
> > > /* GC_THREAD ATTR */
> > > GC_THREAD_RW_ATTR(gc_urgent_sleep_time, urgent_sleep_time);
> > > GC_THREAD_RW_ATTR(gc_min_sleep_time, min_sleep_time);
> > > @@ -1008,6 +1063,7 @@ F2FS_SBI_GENERAL_RW_ATTR(compr_saved_block);
> > > F2FS_SBI_GENERAL_RW_ATTR(compr_new_inode);
> > > F2FS_SBI_GENERAL_RW_ATTR(compress_percent);
> > > F2FS_SBI_GENERAL_RW_ATTR(compress_watermark);
> > > +MOUNT_INFO_RW_ATTR(compress_extension, extensions);
> > > #endif
> > > /* atomic write */
> > > F2FS_SBI_GENERAL_RO_ATTR(current_atomic_write);
> > > @@ -1181,6 +1237,7 @@ static struct attribute *f2fs_attrs[] = {
> > > ATTR_LIST(compr_new_inode),
> > > ATTR_LIST(compress_percent),
> > > ATTR_LIST(compress_watermark),
> > > + ATTR_LIST(compress_extension),
> > > #endif
> > > /* For ATGC */
> > > ATTR_LIST(atgc_candidate_ratio),
> > > --
> > > 2.40.1

2024-02-21 01:08:05

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH] f2fs: support compress extension update via sysfs interface

On 2024/2/21 3:28, Jaegeuk Kim wrote:
> On 02/19, Chao Yu wrote:
>> On 2024/2/8 8:07, Jaegeuk Kim wrote:
>>> On 02/07, Chao Yu wrote:
>>>> Introduce /sys/fs/f2fs/<disk>/compress_extension to support
>>>> adding/deleting compress extension via sysfs interface, in
>>>> comparison to mount option, it's more easy to use and less
>>>> authority issue for applications.
>>>>
>>>> Usage:
>>>> - Query: cat /sys/fs/f2fs/<disk>/compress_extension
>>>> - Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
>>>> - Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
>>>> - [c] means add/del compress extension
>>>> - [n] means add/del nocompress extension
>>>>
>>>> Signed-off-by: Sheng Yong <[email protected]>
>>>> Signed-off-by: Chao Yu <[email protected]>
>>>> ---
>>>> Documentation/ABI/testing/sysfs-fs-f2fs | 10 ++++
>>>> Documentation/filesystems/f2fs.rst | 6 ++-
>>>> fs/f2fs/compress.c | 61 +++++++++++++++++++++++
>>>> fs/f2fs/f2fs.h | 4 +-
>>>> fs/f2fs/sysfs.c | 65 +++++++++++++++++++++++--
>>>> 5 files changed, 139 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>> index 48c135e24eb5..1f2cc0913e45 100644
>>>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>>>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>>>> @@ -762,3 +762,13 @@ Date: November 2023
>>>> Contact: "Chao Yu" <[email protected]>
>>>> Description: It controls to enable/disable IO aware feature for background discard.
>>>> By default, the value is 1 which indicates IO aware is on.
>>>> +
>>>> +What: /sys/fs/f2fs/<disk>/compress_extension
>>>> +Date: October 2023
>>>> +Contact: "Chao Yu" <[email protected]>
>>>> +Description: Used to control configure [|no]compress_extension list:
>>>> + - Query: cat /sys/fs/f2fs/<disk>/compress_extension
>>>> + - Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
>>>> + - Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
>>>> + - [c] means add/del compress extension
>>>> + - [n] means add/del nocompress extension
>>>> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
>>>> index 32cbfa864f38..c82a8fd7316b 100644
>>>> --- a/Documentation/filesystems/f2fs.rst
>>>> +++ b/Documentation/filesystems/f2fs.rst
>>>> @@ -821,17 +821,19 @@ Compression implementation
>>>> all logical blocks in cluster contain valid data and compress ratio of
>>>> cluster data is lower than specified threshold.
>>>> -- To enable compression on regular inode, there are four ways:
>>>> +- To enable compression on regular inode, there are five ways:
>>>> * chattr +c file
>>>> * chattr +c dir; touch dir/file
>>>> * mount w/ -o compress_extension=ext; touch file.ext
>>>> * mount w/ -o compress_extension=*; touch any_file
>>>> + * echo '[c]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext
>>>> -- To disable compression on regular inode, there are two ways:
>>>> +- To disable compression on regular inode, there are three ways:
>>>> * chattr -c file
>>>> * mount w/ -o nocompress_extension=ext; touch file.ext
>>>> + * echo '[n]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext
>>>> - Priority in between FS_COMPR_FL, FS_NOCOMP_FS, extensions:
>>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>>>> index 3dc488ce882b..a5257882c772 100644
>>>> --- a/fs/f2fs/compress.c
>>>> +++ b/fs/f2fs/compress.c
>>>> @@ -20,6 +20,67 @@
>>>> #include "segment.h"
>>>> #include <trace/events/f2fs.h>
>>>> +static int is_compress_extension_exist(struct f2fs_sb_info *sbi,
>>>> + unsigned char (*ext)[F2FS_EXTENSION_LEN],
>>>> + int ext_cnt, unsigned char *new_ext)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < ext_cnt; i++) {
>>>> + if (!strcasecmp(new_ext, ext[i]))
>>>> + return i;
>>>> + }
>>>> + return -1;
>>>> +}
>>>> +
>>>> +int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
>>>> + unsigned char *new_ext, bool is_ext, bool set)
>>>> +{
>>>> + unsigned char (*ext)[F2FS_EXTENSION_LEN];
>>>> + unsigned char *ext_cnt;
>>>> +
>>>> + if (is_ext) {
>>>> + ext = F2FS_OPTION(sbi).extensions;
>>>> + ext_cnt = &F2FS_OPTION(sbi).compress_ext_cnt;
>>>> + } else {
>>>> + ext = F2FS_OPTION(sbi).noextensions;
>>>> + ext_cnt = &F2FS_OPTION(sbi).nocompress_ext_cnt;
>>>> + }
>>>> +
>>>> + if (set) {
>>>> + if (*ext_cnt >= COMPRESS_EXT_NUM)
>>>> + return -EINVAL;
>>>> +
>>>> + if (is_compress_extension_exist(sbi,
>>>> + F2FS_OPTION(sbi).extensions,
>>>> + F2FS_OPTION(sbi).compress_ext_cnt,
>>>> + new_ext) >= 0)
>>>> + return -EEXIST;
>>>> +
>>>> + if (is_compress_extension_exist(sbi,
>>>> + F2FS_OPTION(sbi).noextensions,
>>>> + F2FS_OPTION(sbi).nocompress_ext_cnt,
>>>> + new_ext) >= 0)
>>>> + return -EEXIST;
>>>> +
>>>> + strcpy(ext[*ext_cnt], new_ext);
>>>> + (*ext_cnt)++;
>>>> + } else {
>>>> + int pos = is_compress_extension_exist(sbi, ext,
>>>> + *ext_cnt, new_ext);
>>>> + if (pos < 0)
>>>> + return -ENOENT;
>>>> +
>>>> + if (pos < *ext_cnt - 1)
>>>> + memmove(ext + pos, ext + pos + 1,
>>>> + F2FS_EXTENSION_LEN * (*ext_cnt - pos - 1));
>>>> + memset(ext + *ext_cnt - 1, 0, F2FS_EXTENSION_LEN);
>>>> + (*ext_cnt)--;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static struct kmem_cache *cic_entry_slab;
>>>> static struct kmem_cache *dic_entry_slab;
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index c5e7460d1a0a..d44e2c43d8ab 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -186,7 +186,7 @@ struct f2fs_mount_info {
>>>> unsigned char compress_level; /* compress level */
>>>> bool compress_chksum; /* compressed data chksum */
>>>> unsigned char compress_ext_cnt; /* extension count */
>>>> - unsigned char nocompress_ext_cnt; /* nocompress extension count */
>>>> + unsigned char nocompress_ext_cnt; /* nocompress extension count */
>>>> int compress_mode; /* compression mode */
>>>> unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
>>>> unsigned char noextensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
>>>> @@ -4273,6 +4273,8 @@ static inline bool f2fs_post_read_required(struct inode *inode)
>>>> * compress.c
>>>> */
>>>> #ifdef CONFIG_F2FS_FS_COMPRESSION
>>>> +int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
>>>> + unsigned char *new_ext, bool is_ext, bool set);
>>>> bool f2fs_is_compressed_page(struct page *page);
>>>> struct page *f2fs_compress_control_page(struct page *page);
>>>> int f2fs_prepare_compress_overwrite(struct inode *inode,
>>>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>>>> index a7ec55c7bb20..a8f05a02e202 100644
>>>> --- a/fs/f2fs/sysfs.c
>>>> +++ b/fs/f2fs/sysfs.c
>>>> @@ -39,6 +39,7 @@ enum {
>>>> RESERVED_BLOCKS, /* struct f2fs_sb_info */
>>>> CPRC_INFO, /* struct ckpt_req_control */
>>>> ATGC_INFO, /* struct atgc_management */
>>>> + MOUNT_INFO, /* struct f2fs_mount_info */
>>>> };
>>>> static const char *gc_mode_names[MAX_GC_MODE] = {
>>>> @@ -89,6 +90,8 @@ static unsigned char *__struct_ptr(struct f2fs_sb_info *sbi, int struct_type)
>>>> return (unsigned char *)&sbi->cprc_info;
>>>> else if (struct_type == ATGC_INFO)
>>>> return (unsigned char *)&sbi->am;
>>>> + else if (struct_type == MOUNT_INFO)
>>>> + return (unsigned char *)&F2FS_OPTION(sbi);
>>>> return NULL;
>>>> }
>>>> @@ -358,6 +361,25 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>>>> if (!strcmp(a->attr.name, "compr_new_inode"))
>>>> return sysfs_emit(buf, "%u\n", sbi->compr_new_inode);
>>>> +
>>>> + if (!strcmp(a->attr.name, "compress_extension")) {
>>>> + int len = 0, i;
>>>> +
>>>> + f2fs_down_read(&sbi->sb_lock);
>>>> + len += scnprintf(buf + len, PAGE_SIZE - len,
>>>> + "compress extension:\n");
>>>> + for (i = 0; i < F2FS_OPTION(sbi).compress_ext_cnt; i++)
>>>> + len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
>>>> + F2FS_OPTION(sbi).extensions[i]);
>>>> +
>>>> + len += scnprintf(buf + len, PAGE_SIZE - len,
>>>> + "nocompress extension:\n");
>>>> + for (i = 0; i < F2FS_OPTION(sbi).nocompress_ext_cnt; i++)
>>>> + len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
>>>> + F2FS_OPTION(sbi).noextensions[i]);
>>>
>>> I don't think this is acceptable in sysfs.
>>
>> It looks block layer output similar string:
>>
>> cat /sys/block/<dev>/queue/scheduler
>> mq-deadline kyber [bfq] none
>>
>> Or, can we add extension entries into /sys/fs/f2fs/<dev>/compress_extensions/
>> directory? like feature entries in /sys/fs/f2fs/<dev>/feature_list/.
>>
>> e.g.
>>
>> ls /sys/fs/f2fs/<dev>/compress_extensions/
>> so dex
>>
>> Show enable/disable to indicate current extension is in
>> compress_extension/nocompress_extension array.
>>
>> cat /sys/fs/f2fs/<dev>/compress_extensions/so
>> enable
>>
>> cat /sys/fs/f2fs/<dev>/compress_extensions/dex
>> disable
>
> How about adding all the file extentions in a separate entries and eash showing
> the hot/cold/compression information together?

Ah, better,

If there is no objection, I'd like to add an extra 'extensions' directory
into /sys/fs/f2fs/<dev>/, and add all file extensions as separated entries
under 'extensions' directory.

Thanks,

>
>>
>> Thanks,
>>
>>>
>>>> + f2fs_up_read(&sbi->sb_lock);
>>>> + return len;
>>>> + }
>>>> #endif
>>>> if (!strcmp(a->attr.name, "gc_segment_mode"))
>>>> @@ -446,6 +468,35 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
>>>> return ret ? ret : count;
>>>> }
>>>> +#ifdef CONFIG_F2FS_FS_COMPRESSION
>>>> + if (!strcmp(a->attr.name, "compress_extension")) {
>>>> + char *name = strim((char *)buf);
>>>> + bool set = true, cmpr;
>>>> +
>>>> + if (!strncmp(name, "[c]", 3))
>>>> + cmpr = true;
>>>> + else if (!strncmp(name, "[n]", 3))
>>>> + cmpr = false;
>>>> + else
>>>> + return -EINVAL;
>>>> +
>>>> + name += 3;
>>>> +
>>>> + if (*name == '!') {
>>>> + name++;
>>>> + set = false;
>>>> + }
>>>> +
>>>> + if (!strlen(name) || strlen(name) >= F2FS_EXTENSION_LEN)
>>>> + return -EINVAL;
>>>> +
>>>> + f2fs_down_write(&sbi->sb_lock);
>>>> + ret = f2fs_update_compress_extension(sbi, name, cmpr, set);
>>>> + f2fs_up_write(&sbi->sb_lock);
>>>> + return ret ? ret : count;
>>>> + }
>>>> +#endif
>>>> +
>>>> if (!strcmp(a->attr.name, "ckpt_thread_ioprio")) {
>>>> const char *name = strim((char *)buf);
>>>> struct ckpt_req_control *cprc = &sbi->cprc_info;
>>>> @@ -785,15 +836,16 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
>>>> const char *buf, size_t count)
>>>> {
>>>> ssize_t ret;
>>>> - bool gc_entry = (!strcmp(a->attr.name, "gc_urgent") ||
>>>> - a->struct_type == GC_THREAD);
>>>> + bool need_lock = (!strcmp(a->attr.name, "gc_urgent") ||
>>>> + a->struct_type == GC_THREAD ||
>>>> + a->struct_type == MOUNT_INFO);
>>>> - if (gc_entry) {
>>>> + if (need_lock) {
>>>> if (!down_read_trylock(&sbi->sb->s_umount))
>>>> return -EAGAIN;
>>>> }
>>>> ret = __sbi_store(a, sbi, buf, count);
>>>> - if (gc_entry)
>>>> + if (need_lock)
>>>> up_read(&sbi->sb->s_umount);
>>>> return ret;
>>>> @@ -942,6 +994,9 @@ static struct f2fs_attr f2fs_attr_##name = __ATTR(name, 0444, name##_show, NULL)
>>>> #define ATGC_INFO_RW_ATTR(name, elname) \
>>>> F2FS_RW_ATTR(ATGC_INFO, atgc_management, name, elname)
>>>> +#define MOUNT_INFO_RW_ATTR(name, elname) \
>>>> + F2FS_RW_ATTR(MOUNT_INFO, f2fs_mount_info, name, elname)
>>>> +
>>>> /* GC_THREAD ATTR */
>>>> GC_THREAD_RW_ATTR(gc_urgent_sleep_time, urgent_sleep_time);
>>>> GC_THREAD_RW_ATTR(gc_min_sleep_time, min_sleep_time);
>>>> @@ -1008,6 +1063,7 @@ F2FS_SBI_GENERAL_RW_ATTR(compr_saved_block);
>>>> F2FS_SBI_GENERAL_RW_ATTR(compr_new_inode);
>>>> F2FS_SBI_GENERAL_RW_ATTR(compress_percent);
>>>> F2FS_SBI_GENERAL_RW_ATTR(compress_watermark);
>>>> +MOUNT_INFO_RW_ATTR(compress_extension, extensions);
>>>> #endif
>>>> /* atomic write */
>>>> F2FS_SBI_GENERAL_RO_ATTR(current_atomic_write);
>>>> @@ -1181,6 +1237,7 @@ static struct attribute *f2fs_attrs[] = {
>>>> ATTR_LIST(compr_new_inode),
>>>> ATTR_LIST(compress_percent),
>>>> ATTR_LIST(compress_watermark),
>>>> + ATTR_LIST(compress_extension),
>>>> #endif
>>>> /* For ATGC */
>>>> ATTR_LIST(atgc_candidate_ratio),
>>>> --
>>>> 2.40.1

2024-02-21 01:58:15

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH] f2fs: support compress extension update via sysfs interface

On 02/21, Chao Yu wrote:
> On 2024/2/21 3:28, Jaegeuk Kim wrote:
> > On 02/19, Chao Yu wrote:
> > > On 2024/2/8 8:07, Jaegeuk Kim wrote:
> > > > On 02/07, Chao Yu wrote:
> > > > > Introduce /sys/fs/f2fs/<disk>/compress_extension to support
> > > > > adding/deleting compress extension via sysfs interface, in
> > > > > comparison to mount option, it's more easy to use and less
> > > > > authority issue for applications.
> > > > >
> > > > > Usage:
> > > > > - Query: cat /sys/fs/f2fs/<disk>/compress_extension
> > > > > - Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
> > > > > - Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
> > > > > - [c] means add/del compress extension
> > > > > - [n] means add/del nocompress extension
> > > > >
> > > > > Signed-off-by: Sheng Yong <[email protected]>
> > > > > Signed-off-by: Chao Yu <[email protected]>
> > > > > ---
> > > > > Documentation/ABI/testing/sysfs-fs-f2fs | 10 ++++
> > > > > Documentation/filesystems/f2fs.rst | 6 ++-
> > > > > fs/f2fs/compress.c | 61 +++++++++++++++++++++++
> > > > > fs/f2fs/f2fs.h | 4 +-
> > > > > fs/f2fs/sysfs.c | 65 +++++++++++++++++++++++--
> > > > > 5 files changed, 139 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> > > > > index 48c135e24eb5..1f2cc0913e45 100644
> > > > > --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> > > > > +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> > > > > @@ -762,3 +762,13 @@ Date: November 2023
> > > > > Contact: "Chao Yu" <[email protected]>
> > > > > Description: It controls to enable/disable IO aware feature for background discard.
> > > > > By default, the value is 1 which indicates IO aware is on.
> > > > > +
> > > > > +What: /sys/fs/f2fs/<disk>/compress_extension
> > > > > +Date: October 2023
> > > > > +Contact: "Chao Yu" <[email protected]>
> > > > > +Description: Used to control configure [|no]compress_extension list:
> > > > > + - Query: cat /sys/fs/f2fs/<disk>/compress_extension
> > > > > + - Add: echo '[c|n]extension' > /sys/fs/f2fs/<disk>/compress_extension
> > > > > + - Del: echo '[c|n]!extension' > /sys/fs/f2fs/<disk>/compress_extension
> > > > > + - [c] means add/del compress extension
> > > > > + - [n] means add/del nocompress extension
> > > > > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> > > > > index 32cbfa864f38..c82a8fd7316b 100644
> > > > > --- a/Documentation/filesystems/f2fs.rst
> > > > > +++ b/Documentation/filesystems/f2fs.rst
> > > > > @@ -821,17 +821,19 @@ Compression implementation
> > > > > all logical blocks in cluster contain valid data and compress ratio of
> > > > > cluster data is lower than specified threshold.
> > > > > -- To enable compression on regular inode, there are four ways:
> > > > > +- To enable compression on regular inode, there are five ways:
> > > > > * chattr +c file
> > > > > * chattr +c dir; touch dir/file
> > > > > * mount w/ -o compress_extension=ext; touch file.ext
> > > > > * mount w/ -o compress_extension=*; touch any_file
> > > > > + * echo '[c]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext
> > > > > -- To disable compression on regular inode, there are two ways:
> > > > > +- To disable compression on regular inode, there are three ways:
> > > > > * chattr -c file
> > > > > * mount w/ -o nocompress_extension=ext; touch file.ext
> > > > > + * echo '[n]ext' > /sys/fs/f2fs/<disk>/compress_extension; touch file.ext
> > > > > - Priority in between FS_COMPR_FL, FS_NOCOMP_FS, extensions:
> > > > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > > > > index 3dc488ce882b..a5257882c772 100644
> > > > > --- a/fs/f2fs/compress.c
> > > > > +++ b/fs/f2fs/compress.c
> > > > > @@ -20,6 +20,67 @@
> > > > > #include "segment.h"
> > > > > #include <trace/events/f2fs.h>
> > > > > +static int is_compress_extension_exist(struct f2fs_sb_info *sbi,
> > > > > + unsigned char (*ext)[F2FS_EXTENSION_LEN],
> > > > > + int ext_cnt, unsigned char *new_ext)
> > > > > +{
> > > > > + int i;
> > > > > +
> > > > > + for (i = 0; i < ext_cnt; i++) {
> > > > > + if (!strcasecmp(new_ext, ext[i]))
> > > > > + return i;
> > > > > + }
> > > > > + return -1;
> > > > > +}
> > > > > +
> > > > > +int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
> > > > > + unsigned char *new_ext, bool is_ext, bool set)
> > > > > +{
> > > > > + unsigned char (*ext)[F2FS_EXTENSION_LEN];
> > > > > + unsigned char *ext_cnt;
> > > > > +
> > > > > + if (is_ext) {
> > > > > + ext = F2FS_OPTION(sbi).extensions;
> > > > > + ext_cnt = &F2FS_OPTION(sbi).compress_ext_cnt;
> > > > > + } else {
> > > > > + ext = F2FS_OPTION(sbi).noextensions;
> > > > > + ext_cnt = &F2FS_OPTION(sbi).nocompress_ext_cnt;
> > > > > + }
> > > > > +
> > > > > + if (set) {
> > > > > + if (*ext_cnt >= COMPRESS_EXT_NUM)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + if (is_compress_extension_exist(sbi,
> > > > > + F2FS_OPTION(sbi).extensions,
> > > > > + F2FS_OPTION(sbi).compress_ext_cnt,
> > > > > + new_ext) >= 0)
> > > > > + return -EEXIST;
> > > > > +
> > > > > + if (is_compress_extension_exist(sbi,
> > > > > + F2FS_OPTION(sbi).noextensions,
> > > > > + F2FS_OPTION(sbi).nocompress_ext_cnt,
> > > > > + new_ext) >= 0)
> > > > > + return -EEXIST;
> > > > > +
> > > > > + strcpy(ext[*ext_cnt], new_ext);
> > > > > + (*ext_cnt)++;
> > > > > + } else {
> > > > > + int pos = is_compress_extension_exist(sbi, ext,
> > > > > + *ext_cnt, new_ext);
> > > > > + if (pos < 0)
> > > > > + return -ENOENT;
> > > > > +
> > > > > + if (pos < *ext_cnt - 1)
> > > > > + memmove(ext + pos, ext + pos + 1,
> > > > > + F2FS_EXTENSION_LEN * (*ext_cnt - pos - 1));
> > > > > + memset(ext + *ext_cnt - 1, 0, F2FS_EXTENSION_LEN);
> > > > > + (*ext_cnt)--;
> > > > > + }
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > static struct kmem_cache *cic_entry_slab;
> > > > > static struct kmem_cache *dic_entry_slab;
> > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > > index c5e7460d1a0a..d44e2c43d8ab 100644
> > > > > --- a/fs/f2fs/f2fs.h
> > > > > +++ b/fs/f2fs/f2fs.h
> > > > > @@ -186,7 +186,7 @@ struct f2fs_mount_info {
> > > > > unsigned char compress_level; /* compress level */
> > > > > bool compress_chksum; /* compressed data chksum */
> > > > > unsigned char compress_ext_cnt; /* extension count */
> > > > > - unsigned char nocompress_ext_cnt; /* nocompress extension count */
> > > > > + unsigned char nocompress_ext_cnt; /* nocompress extension count */
> > > > > int compress_mode; /* compression mode */
> > > > > unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
> > > > > unsigned char noextensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* extensions */
> > > > > @@ -4273,6 +4273,8 @@ static inline bool f2fs_post_read_required(struct inode *inode)
> > > > > * compress.c
> > > > > */
> > > > > #ifdef CONFIG_F2FS_FS_COMPRESSION
> > > > > +int f2fs_update_compress_extension(struct f2fs_sb_info *sbi,
> > > > > + unsigned char *new_ext, bool is_ext, bool set);
> > > > > bool f2fs_is_compressed_page(struct page *page);
> > > > > struct page *f2fs_compress_control_page(struct page *page);
> > > > > int f2fs_prepare_compress_overwrite(struct inode *inode,
> > > > > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> > > > > index a7ec55c7bb20..a8f05a02e202 100644
> > > > > --- a/fs/f2fs/sysfs.c
> > > > > +++ b/fs/f2fs/sysfs.c
> > > > > @@ -39,6 +39,7 @@ enum {
> > > > > RESERVED_BLOCKS, /* struct f2fs_sb_info */
> > > > > CPRC_INFO, /* struct ckpt_req_control */
> > > > > ATGC_INFO, /* struct atgc_management */
> > > > > + MOUNT_INFO, /* struct f2fs_mount_info */
> > > > > };
> > > > > static const char *gc_mode_names[MAX_GC_MODE] = {
> > > > > @@ -89,6 +90,8 @@ static unsigned char *__struct_ptr(struct f2fs_sb_info *sbi, int struct_type)
> > > > > return (unsigned char *)&sbi->cprc_info;
> > > > > else if (struct_type == ATGC_INFO)
> > > > > return (unsigned char *)&sbi->am;
> > > > > + else if (struct_type == MOUNT_INFO)
> > > > > + return (unsigned char *)&F2FS_OPTION(sbi);
> > > > > return NULL;
> > > > > }
> > > > > @@ -358,6 +361,25 @@ static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
> > > > > if (!strcmp(a->attr.name, "compr_new_inode"))
> > > > > return sysfs_emit(buf, "%u\n", sbi->compr_new_inode);
> > > > > +
> > > > > + if (!strcmp(a->attr.name, "compress_extension")) {
> > > > > + int len = 0, i;
> > > > > +
> > > > > + f2fs_down_read(&sbi->sb_lock);
> > > > > + len += scnprintf(buf + len, PAGE_SIZE - len,
> > > > > + "compress extension:\n");
> > > > > + for (i = 0; i < F2FS_OPTION(sbi).compress_ext_cnt; i++)
> > > > > + len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
> > > > > + F2FS_OPTION(sbi).extensions[i]);
> > > > > +
> > > > > + len += scnprintf(buf + len, PAGE_SIZE - len,
> > > > > + "nocompress extension:\n");
> > > > > + for (i = 0; i < F2FS_OPTION(sbi).nocompress_ext_cnt; i++)
> > > > > + len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
> > > > > + F2FS_OPTION(sbi).noextensions[i]);
> > > >
> > > > I don't think this is acceptable in sysfs.
> > >
> > > It looks block layer output similar string:
> > >
> > > cat /sys/block/<dev>/queue/scheduler
> > > mq-deadline kyber [bfq] none
> > >
> > > Or, can we add extension entries into /sys/fs/f2fs/<dev>/compress_extensions/
> > > directory? like feature entries in /sys/fs/f2fs/<dev>/feature_list/.
> > >
> > > e.g.
> > >
> > > ls /sys/fs/f2fs/<dev>/compress_extensions/
> > > so dex
> > >
> > > Show enable/disable to indicate current extension is in
> > > compress_extension/nocompress_extension array.
> > >
> > > cat /sys/fs/f2fs/<dev>/compress_extensions/so
> > > enable
> > >
> > > cat /sys/fs/f2fs/<dev>/compress_extensions/dex
> > > disable
> >
> > How about adding all the file extentions in a separate entries and eash showing
> > the hot/cold/compression information together?
>
> Ah, better,
>
> If there is no objection, I'd like to add an extra 'extensions' directory
> into /sys/fs/f2fs/<dev>/, and add all file extensions as separated entries
> under 'extensions' directory.

Looks good to me. Thanks.

>
> Thanks,
>
> >
> > >
> > > Thanks,
> > >
> > > >
> > > > > + f2fs_up_read(&sbi->sb_lock);
> > > > > + return len;
> > > > > + }
> > > > > #endif
> > > > > if (!strcmp(a->attr.name, "gc_segment_mode"))
> > > > > @@ -446,6 +468,35 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
> > > > > return ret ? ret : count;
> > > > > }
> > > > > +#ifdef CONFIG_F2FS_FS_COMPRESSION
> > > > > + if (!strcmp(a->attr.name, "compress_extension")) {
> > > > > + char *name = strim((char *)buf);
> > > > > + bool set = true, cmpr;
> > > > > +
> > > > > + if (!strncmp(name, "[c]", 3))
> > > > > + cmpr = true;
> > > > > + else if (!strncmp(name, "[n]", 3))
> > > > > + cmpr = false;
> > > > > + else
> > > > > + return -EINVAL;
> > > > > +
> > > > > + name += 3;
> > > > > +
> > > > > + if (*name == '!') {
> > > > > + name++;
> > > > > + set = false;
> > > > > + }
> > > > > +
> > > > > + if (!strlen(name) || strlen(name) >= F2FS_EXTENSION_LEN)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + f2fs_down_write(&sbi->sb_lock);
> > > > > + ret = f2fs_update_compress_extension(sbi, name, cmpr, set);
> > > > > + f2fs_up_write(&sbi->sb_lock);
> > > > > + return ret ? ret : count;
> > > > > + }
> > > > > +#endif
> > > > > +
> > > > > if (!strcmp(a->attr.name, "ckpt_thread_ioprio")) {
> > > > > const char *name = strim((char *)buf);
> > > > > struct ckpt_req_control *cprc = &sbi->cprc_info;
> > > > > @@ -785,15 +836,16 @@ static ssize_t f2fs_sbi_store(struct f2fs_attr *a,
> > > > > const char *buf, size_t count)
> > > > > {
> > > > > ssize_t ret;
> > > > > - bool gc_entry = (!strcmp(a->attr.name, "gc_urgent") ||
> > > > > - a->struct_type == GC_THREAD);
> > > > > + bool need_lock = (!strcmp(a->attr.name, "gc_urgent") ||
> > > > > + a->struct_type == GC_THREAD ||
> > > > > + a->struct_type == MOUNT_INFO);
> > > > > - if (gc_entry) {
> > > > > + if (need_lock) {
> > > > > if (!down_read_trylock(&sbi->sb->s_umount))
> > > > > return -EAGAIN;
> > > > > }
> > > > > ret = __sbi_store(a, sbi, buf, count);
> > > > > - if (gc_entry)
> > > > > + if (need_lock)
> > > > > up_read(&sbi->sb->s_umount);
> > > > > return ret;
> > > > > @@ -942,6 +994,9 @@ static struct f2fs_attr f2fs_attr_##name = __ATTR(name, 0444, name##_show, NULL)
> > > > > #define ATGC_INFO_RW_ATTR(name, elname) \
> > > > > F2FS_RW_ATTR(ATGC_INFO, atgc_management, name, elname)
> > > > > +#define MOUNT_INFO_RW_ATTR(name, elname) \
> > > > > + F2FS_RW_ATTR(MOUNT_INFO, f2fs_mount_info, name, elname)
> > > > > +
> > > > > /* GC_THREAD ATTR */
> > > > > GC_THREAD_RW_ATTR(gc_urgent_sleep_time, urgent_sleep_time);
> > > > > GC_THREAD_RW_ATTR(gc_min_sleep_time, min_sleep_time);
> > > > > @@ -1008,6 +1063,7 @@ F2FS_SBI_GENERAL_RW_ATTR(compr_saved_block);
> > > > > F2FS_SBI_GENERAL_RW_ATTR(compr_new_inode);
> > > > > F2FS_SBI_GENERAL_RW_ATTR(compress_percent);
> > > > > F2FS_SBI_GENERAL_RW_ATTR(compress_watermark);
> > > > > +MOUNT_INFO_RW_ATTR(compress_extension, extensions);
> > > > > #endif
> > > > > /* atomic write */
> > > > > F2FS_SBI_GENERAL_RO_ATTR(current_atomic_write);
> > > > > @@ -1181,6 +1237,7 @@ static struct attribute *f2fs_attrs[] = {
> > > > > ATTR_LIST(compr_new_inode),
> > > > > ATTR_LIST(compress_percent),
> > > > > ATTR_LIST(compress_watermark),
> > > > > + ATTR_LIST(compress_extension),
> > > > > #endif
> > > > > /* For ATGC */
> > > > > ATTR_LIST(atgc_candidate_ratio),
> > > > > --
> > > > > 2.40.1