linux-erofs.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] erofs: add sysfs interface
@ 2021-11-09  2:54 Huang Jianan via Linux-erofs
  2021-11-09  2:54 ` [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan via Linux-erofs
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Huang Jianan via Linux-erofs @ 2021-11-09  2:54 UTC (permalink / raw)
  To: linux-erofs; +Cc: zhangshiming, linux-kernel, yh, guanyuwei, guoweichao

Add sysfs interface to configure erofs related parameters in the
future.

Signed-off-by: Huang Jianan <huangjianan@oppo.com>
---
 fs/erofs/Makefile   |   2 +-
 fs/erofs/internal.h |  10 ++
 fs/erofs/super.c    |  12 +++
 fs/erofs/sysfs.c    | 239 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 262 insertions(+), 1 deletion(-)
 create mode 100644 fs/erofs/sysfs.c

diff --git a/fs/erofs/Makefile b/fs/erofs/Makefile
index 756fe2d65272..8a3317e38e5a 100644
--- a/fs/erofs/Makefile
+++ b/fs/erofs/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-$(CONFIG_EROFS_FS) += erofs.o
-erofs-objs := super.o inode.o data.o namei.o dir.o utils.o pcpubuf.o
+erofs-objs := super.o inode.o data.o namei.o dir.o utils.o pcpubuf.o sysfs.o
 erofs-$(CONFIG_EROFS_FS_XATTR) += xattr.o
 erofs-$(CONFIG_EROFS_FS_ZIP) += decompressor.o zmap.o zdata.o
 erofs-$(CONFIG_EROFS_FS_ZIP_LZMA) += decompressor_lzma.o
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 3265688af7f9..d0cd712dc222 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -134,6 +134,10 @@ struct erofs_sb_info {
 	u8 volume_name[16];             /* volume name */
 	u32 feature_compat;
 	u32 feature_incompat;
+
+	/* sysfs support */
+	struct kobject s_kobj;		/* /sys/fs/erofs/<devname> */
+	struct completion s_kobj_unregister;
 };
 
 #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info)
@@ -498,6 +502,12 @@ int erofs_pcpubuf_growsize(unsigned int nrpages);
 void erofs_pcpubuf_init(void);
 void erofs_pcpubuf_exit(void);
 
+/* sysfs.c */
+int erofs_register_sysfs(struct super_block *sb);
+void erofs_unregister_sysfs(struct super_block *sb);
+int __init erofs_init_sysfs(void);
+void erofs_exit_sysfs(void);
+
 /* utils.c / zdata.c */
 struct page *erofs_allocpage(struct page **pagepool, gfp_t gfp);
 static inline void erofs_pagepool_add(struct page **pagepool,
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 6a969b1e0ee6..abc1da5d1719 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -695,6 +695,10 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (err)
 		return err;
 
+	err = erofs_register_sysfs(sb);
+	if (err)
+		return err;
+
 	erofs_info(sb, "mounted with root inode @ nid %llu.", ROOT_NID(sbi));
 	return 0;
 }
@@ -808,6 +812,7 @@ static void erofs_put_super(struct super_block *sb)
 
 	DBG_BUGON(!sbi);
 
+	erofs_unregister_sysfs(sb);
 	erofs_shrinker_unregister(sb);
 #ifdef CONFIG_EROFS_FS_ZIP
 	iput(sbi->managed_cache);
@@ -852,6 +857,10 @@ static int __init erofs_module_init(void)
 	if (err)
 		goto zip_err;
 
+	err = erofs_init_sysfs();
+	if (err)
+		goto sysfs_err;
+
 	err = register_filesystem(&erofs_fs_type);
 	if (err)
 		goto fs_err;
@@ -859,6 +868,8 @@ static int __init erofs_module_init(void)
 	return 0;
 
 fs_err:
+	erofs_exit_sysfs();
+sysfs_err:
 	z_erofs_exit_zip_subsystem();
 zip_err:
 	z_erofs_lzma_exit();
@@ -877,6 +888,7 @@ static void __exit erofs_module_exit(void)
 	/* Ensure all RCU free inodes / pclusters are safe to be destroyed. */
 	rcu_barrier();
 
+	erofs_exit_sysfs();
 	z_erofs_exit_zip_subsystem();
 	z_erofs_lzma_exit();
 	erofs_exit_shrinker();
diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
new file mode 100644
index 000000000000..c7685f1a8f34
--- /dev/null
+++ b/fs/erofs/sysfs.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C), 2008-2021, OPPO Mobile Comm Corp., Ltd.
+ *             https://www.oppo.com/
+ */
+#include <linux/sysfs.h>
+#include <linux/kobject.h>
+
+#include "internal.h"
+
+enum {
+	attr_feature,
+	attr_pointer_ui,
+	attr_pointer_bool,
+};
+
+enum {
+	struct_erofs_sb_info,
+};
+
+struct erofs_attr {
+	struct attribute attr;
+	short attr_id;
+	int struct_type;
+	int offset;
+};
+
+#define EROFS_ATTR(_name, _mode, _id)					\
+static struct erofs_attr erofs_attr_##_name = {				\
+	.attr = {.name = __stringify(_name), .mode = _mode },		\
+	.attr_id = attr_##_id,						\
+}
+#define EROFS_ATTR_FUNC(_name, _mode)	EROFS_ATTR(_name, _mode, _name)
+#define EROFS_FEATURE_ATTR(_name)	EROFS_ATTR(_name, 0444, feature)
+
+#define EROFS_ATTR_OFFSET(_name, _mode, _id, _struct)	\
+static struct erofs_attr erofs_attr_##_name = {			\
+	.attr = {.name = __stringify(_name), .mode = _mode },	\
+	.attr_id = attr_##_id,					\
+	.struct_type = struct_##_struct,			\
+	.offset = offsetof(struct _struct, _name),\
+}
+
+#define EROFS_RW_ATTR(_name, _id, _struct)	\
+	EROFS_ATTR_OFFSET(_name, 0644, _id, _struct)
+
+#define EROFS_RO_ATTR(_name, _id, _struct)	\
+	EROFS_ATTR_OFFSET(_name, 0444, _id, _struct)
+
+#define EROFS_RW_ATTR_UI(_name, _struct)	\
+	EROFS_RW_ATTR(_name, pointer_ui, _struct)
+
+#define EROFS_RW_ATTR_BOOL(_name, _struct)	\
+	EROFS_RW_ATTR(_name, pointer_bool, _struct)
+
+#define ATTR_LIST(name) (&erofs_attr_##name.attr)
+
+static struct attribute *erofs_attrs[] = {
+	NULL,
+};
+ATTRIBUTE_GROUPS(erofs);
+
+/* Features this copy of erofs supports */
+EROFS_FEATURE_ATTR(lz4_0padding);
+EROFS_FEATURE_ATTR(compr_cfgs);
+EROFS_FEATURE_ATTR(big_pcluster);
+EROFS_FEATURE_ATTR(device_table);
+EROFS_FEATURE_ATTR(sb_chksum);
+
+static struct attribute *erofs_feat_attrs[] = {
+	ATTR_LIST(lz4_0padding),
+	ATTR_LIST(compr_cfgs),
+	ATTR_LIST(big_pcluster),
+	ATTR_LIST(device_table),
+	ATTR_LIST(sb_chksum),
+	NULL,
+};
+ATTRIBUTE_GROUPS(erofs_feat);
+
+static unsigned char *__struct_ptr(struct erofs_sb_info *sbi,
+					  int struct_type, int offset)
+{
+	if (struct_type == struct_erofs_sb_info)
+		return (unsigned char *)sbi + offset;
+	return NULL;
+}
+
+static ssize_t erofs_attr_show(struct kobject *kobj,
+				struct attribute *attr, char *buf)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+						s_kobj);
+	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
+	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
+
+	switch (a->attr_id) {
+	case attr_feature:
+		return snprintf(buf, PAGE_SIZE, "supported\n");
+	case attr_pointer_ui:
+		if (!ptr)
+			return 0;
+		return snprintf(buf, PAGE_SIZE, "%u\n",
+				*((unsigned int *) ptr));
+	case attr_pointer_bool:
+		if (!ptr)
+			return 0;
+		return snprintf(buf, PAGE_SIZE,
+				*((bool *) ptr) ? "true\n" : "false\n");
+	}
+
+	return 0;
+}
+
+static ssize_t erofs_attr_store(struct kobject *kobj, struct attribute *attr,
+						const char *buf, size_t len)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+						s_kobj);
+	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
+	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
+	unsigned long t;
+	int ret;
+
+	switch (a->attr_id) {
+	case attr_pointer_ui:
+		if (!ptr)
+			return 0;
+		ret = kstrtoul(skip_spaces(buf), 0, &t);
+		if (ret)
+			return ret;
+		*((unsigned int *) ptr) = t;
+		return len;
+	case attr_pointer_bool:
+		if (!ptr)
+			return 0;
+		ret = kstrtoul(skip_spaces(buf), 0, &t);
+		if (ret)
+			return ret;
+		*((bool *) ptr) = !!t;
+		return len;
+	}
+
+	return 0;
+}
+
+static void erofs_sb_release(struct kobject *kobj)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+								s_kobj);
+	complete(&sbi->s_kobj_unregister);
+}
+
+static const struct sysfs_ops erofs_attr_ops = {
+	.show	= erofs_attr_show,
+	.store	= erofs_attr_store,
+};
+
+static struct kobj_type erofs_sb_ktype = {
+	.default_groups = erofs_groups,
+	.sysfs_ops	= &erofs_attr_ops,
+	.release	= erofs_sb_release,
+};
+
+static struct kobj_type erofs_ktype = {
+	.sysfs_ops	= &erofs_attr_ops,
+};
+
+static struct kset erofs_root = {
+	.kobj	= {.ktype = &erofs_ktype},
+};
+
+static struct kobj_type erofs_feat_ktype = {
+	.default_groups = erofs_feat_groups,
+	.sysfs_ops	= &erofs_attr_ops,
+};
+
+static struct kobject erofs_feat = {
+	.kset	= &erofs_root,
+};
+
+int erofs_register_sysfs(struct super_block *sb)
+{
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	int err;
+
+	sbi->s_kobj.kset = &erofs_root;
+	init_completion(&sbi->s_kobj_unregister);
+	err = kobject_init_and_add(&sbi->s_kobj, &erofs_sb_ktype, NULL,
+				"%s", sb->s_id);
+	if (err)
+		goto put_sb_kobj;
+
+	return 0;
+
+put_sb_kobj:
+	kobject_put(&sbi->s_kobj);
+	wait_for_completion(&sbi->s_kobj_unregister);
+	return err;
+}
+
+void erofs_unregister_sysfs(struct super_block *sb)
+{
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+
+	kobject_del(&sbi->s_kobj);
+	kobject_put(&sbi->s_kobj);
+	wait_for_completion(&sbi->s_kobj_unregister);
+}
+
+int __init erofs_init_sysfs(void)
+{
+	int ret;
+
+	kobject_set_name(&erofs_root.kobj, "erofs");
+	erofs_root.kobj.parent = fs_kobj;
+	ret = kset_register(&erofs_root);
+	if (ret)
+		goto root_err;
+
+	ret = kobject_init_and_add(&erofs_feat, &erofs_feat_ktype,
+				   NULL, "features");
+	if (ret)
+		goto feat_err;
+
+	return ret;
+
+feat_err:
+	kobject_put(&erofs_feat);
+	kset_unregister(&erofs_root);
+root_err:
+	return ret;
+}
+
+void erofs_exit_sysfs(void)
+{
+	kobject_put(&erofs_feat);
+	kset_unregister(&erofs_root);
+}
+
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy
  2021-11-09  2:54 [PATCH 1/2] erofs: add sysfs interface Huang Jianan via Linux-erofs
@ 2021-11-09  2:54 ` Huang Jianan via Linux-erofs
  2021-11-09  3:16   ` Gao Xiang
  2021-11-11 20:17   ` [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy kernel test robot
  2021-11-09  3:10 ` [PATCH 1/2] erofs: add sysfs interface Gao Xiang
  2021-11-09  3:14 ` Joe Perches
  2 siblings, 2 replies; 20+ messages in thread
From: Huang Jianan via Linux-erofs @ 2021-11-09  2:54 UTC (permalink / raw)
  To: linux-erofs; +Cc: zhangshiming, linux-kernel, yh, guanyuwei, guoweichao

Although readpage is a synchronous path, there will be no additional
kworker scheduling overhead in non-atomic contexts. So we can add a
sysfs node to allow disable sync decompression.

Signed-off-by: Huang Jianan <huangjianan@oppo.com>
---
 fs/erofs/internal.h | 2 +-
 fs/erofs/super.c    | 2 +-
 fs/erofs/sysfs.c    | 6 ++++++
 fs/erofs/zdata.c    | 9 ++++-----
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index d0cd712dc222..1ab96878009d 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -60,7 +60,7 @@ struct erofs_mount_opts {
 #ifdef CONFIG_EROFS_FS_ZIP
 	/* current strategy of how to use managed cache */
 	unsigned char cache_strategy;
-	/* strategy of sync decompression (false - auto, true - force on) */
+	/* strategy of sync decompression (false - disable, true - force on) */
 	bool readahead_sync_decompress;
 
 	/* threshold for decompression synchronously */
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index abc1da5d1719..26585d865503 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -423,7 +423,7 @@ static void erofs_default_options(struct erofs_fs_context *ctx)
 #ifdef CONFIG_EROFS_FS_ZIP
 	ctx->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
 	ctx->opt.max_sync_decompress_pages = 3;
-	ctx->opt.readahead_sync_decompress = false;
+	ctx->opt.readahead_sync_decompress = true;
 #endif
 #ifdef CONFIG_EROFS_FS_XATTR
 	set_opt(&ctx->opt, XATTR_USER);
diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
index c7685f1a8f34..fe67ed490735 100644
--- a/fs/erofs/sysfs.c
+++ b/fs/erofs/sysfs.c
@@ -16,6 +16,7 @@ enum {
 
 enum {
 	struct_erofs_sb_info,
+	struct_erofs_mount_opts,
 };
 
 struct erofs_attr {
@@ -55,7 +56,10 @@ static struct erofs_attr erofs_attr_##_name = {			\
 
 #define ATTR_LIST(name) (&erofs_attr_##name.attr)
 
+EROFS_RW_ATTR_BOOL(readahead_sync_decompress, erofs_mount_opts);
+
 static struct attribute *erofs_attrs[] = {
+	ATTR_LIST(readahead_sync_decompress),
 	NULL,
 };
 ATTRIBUTE_GROUPS(erofs);
@@ -82,6 +86,8 @@ static unsigned char *__struct_ptr(struct erofs_sb_info *sbi,
 {
 	if (struct_type == struct_erofs_sb_info)
 		return (unsigned char *)sbi + offset;
+	if (struct_type == struct_erofs_mount_opts)
+		return (unsigned char *)&sbi->opt + offset;
 	return NULL;
 }
 
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index bcb1b91b234f..ad11333b367a 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -776,8 +776,6 @@ static void z_erofs_decompressqueue_work(struct work_struct *work);
 static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
 				       bool sync, int bios)
 {
-	struct erofs_sb_info *const sbi = EROFS_SB(io->sb);
-
 	/* wake up the caller thread for sync decompression */
 	if (sync) {
 		unsigned long flags;
@@ -791,10 +789,9 @@ static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
 
 	if (atomic_add_return(bios, &io->pending_bios))
 		return;
-	/* Use workqueue and sync decompression for atomic contexts only */
+	/* Use workqueue decompression for atomic contexts only */
 	if (in_atomic() || irqs_disabled()) {
 		queue_work(z_erofs_workqueue, &io->u.work);
-		sbi->opt.readahead_sync_decompress = true;
 		return;
 	}
 	z_erofs_decompressqueue_work(&io->u.work);
@@ -1454,6 +1451,7 @@ static void z_erofs_pcluster_readmore(struct z_erofs_decompress_frontend *f,
 static int z_erofs_readpage(struct file *file, struct page *page)
 {
 	struct inode *const inode = page->mapping->host;
+	struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
 	struct z_erofs_decompress_frontend f = DECOMPRESS_FRONTEND_INIT(inode);
 	struct page *pagepool = NULL;
 	int err;
@@ -1469,7 +1467,8 @@ static int z_erofs_readpage(struct file *file, struct page *page)
 	(void)z_erofs_collector_end(&f.clt);
 
 	/* if some compressed cluster ready, need submit them anyway */
-	z_erofs_runqueue(inode->i_sb, &f, &pagepool, true);
+	z_erofs_runqueue(inode->i_sb, &f, &pagepool,
+			 sbi->opt.readahead_sync_decompress);
 
 	if (err)
 		erofs_err(inode->i_sb, "failed to read, err [%d]", err);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] erofs: add sysfs interface
  2021-11-09  2:54 [PATCH 1/2] erofs: add sysfs interface Huang Jianan via Linux-erofs
  2021-11-09  2:54 ` [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan via Linux-erofs
@ 2021-11-09  3:10 ` Gao Xiang
  2021-11-09  7:29   ` Huang Jianan via Linux-erofs
  2021-11-09  3:14 ` Joe Perches
  2 siblings, 1 reply; 20+ messages in thread
From: Gao Xiang @ 2021-11-09  3:10 UTC (permalink / raw)
  To: Huang Jianan
  Cc: zhangshiming, linux-kernel, yh, guanyuwei, guoweichao, linux-erofs

Hi Jianan,

On Tue, Nov 09, 2021 at 10:54:44AM +0800, Huang Jianan via Linux-erofs wrote:

You might need to add a "From:" tag here, otherwise, the author will
show "Huang Jianan via Linux-erofs" due to your mailing server...

> Add sysfs interface to configure erofs related parameters in the
> future.

s/in the future/Later/

> 
> Signed-off-by: Huang Jianan <huangjianan@oppo.com>
> ---
>  fs/erofs/Makefile   |   2 +-
>  fs/erofs/internal.h |  10 ++
>  fs/erofs/super.c    |  12 +++
>  fs/erofs/sysfs.c    | 239 ++++++++++++++++++++++++++++++++++++++++++++

At a quick glance, we might need to add sysfs API documentation
as well:
Documentation/ABI/testing/sysfs-fs-erofs

Thanks,
Gao Xiang


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] erofs: add sysfs interface
  2021-11-09  2:54 [PATCH 1/2] erofs: add sysfs interface Huang Jianan via Linux-erofs
  2021-11-09  2:54 ` [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan via Linux-erofs
  2021-11-09  3:10 ` [PATCH 1/2] erofs: add sysfs interface Gao Xiang
@ 2021-11-09  3:14 ` Joe Perches
  2021-11-09  7:31   ` Huang Jianan via Linux-erofs
  2 siblings, 1 reply; 20+ messages in thread
From: Joe Perches @ 2021-11-09  3:14 UTC (permalink / raw)
  To: Huang Jianan, linux-erofs
  Cc: yh, guoweichao, zhangshiming, guanyuwei, linux-kernel

On Tue, 2021-11-09 at 10:54 +0800, Huang Jianan wrote:
> Add sysfs interface to configure erofs related parameters in the
> future.
[]
> diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
[]
> +static ssize_t erofs_attr_show(struct kobject *kobj,
> +				struct attribute *attr, char *buf)
> +{
> +	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
> +						s_kobj);
> +	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
> +	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
> +
> +	switch (a->attr_id) {
> +	case attr_feature:
> +		return snprintf(buf, PAGE_SIZE, "supported\n");
> +	case attr_pointer_ui:
> +		if (!ptr)
> +			return 0;
> +		return snprintf(buf, PAGE_SIZE, "%u\n",
> +				*((unsigned int *) ptr));

Prefer sysfs_emit over snprintf

	case attr_feature:
		return sysfs_emit(buf, "supported\n");
	case attr_pointer_ui:
		...
		return sysfs_emit(buf, "%u\n", *(unsigned int *)ptr);

etc...



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy
  2021-11-09  2:54 ` [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan via Linux-erofs
@ 2021-11-09  3:16   ` Gao Xiang
  2021-11-09  7:43     ` Huang Jianan via Linux-erofs
  2021-11-11 20:17   ` [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy kernel test robot
  1 sibling, 1 reply; 20+ messages in thread
From: Gao Xiang @ 2021-11-09  3:16 UTC (permalink / raw)
  To: Huang Jianan
  Cc: zhangshiming, linux-kernel, yh, guanyuwei, guoweichao, linux-erofs

On Tue, Nov 09, 2021 at 10:54:45AM +0800, Huang Jianan via Linux-erofs wrote:
> Although readpage is a synchronous path, there will be no additional
> kworker scheduling overhead in non-atomic contexts. So we can add a
> sysfs node to allow disable sync decompression.
> 
> Signed-off-by: Huang Jianan <huangjianan@oppo.com>
> ---
>  fs/erofs/internal.h | 2 +-
>  fs/erofs/super.c    | 2 +-
>  fs/erofs/sysfs.c    | 6 ++++++
>  fs/erofs/zdata.c    | 9 ++++-----
>  4 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index d0cd712dc222..1ab96878009d 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -60,7 +60,7 @@ struct erofs_mount_opts {
>  #ifdef CONFIG_EROFS_FS_ZIP
>  	/* current strategy of how to use managed cache */
>  	unsigned char cache_strategy;
> -	/* strategy of sync decompression (false - auto, true - force on) */
> +	/* strategy of sync decompression (false - disable, true - force on) */

Please leave false - auto.

>  	bool readahead_sync_decompress;
>  
>  	/* threshold for decompression synchronously */
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index abc1da5d1719..26585d865503 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -423,7 +423,7 @@ static void erofs_default_options(struct erofs_fs_context *ctx)
>  #ifdef CONFIG_EROFS_FS_ZIP
>  	ctx->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
>  	ctx->opt.max_sync_decompress_pages = 3;
> -	ctx->opt.readahead_sync_decompress = false;
> +	ctx->opt.readahead_sync_decompress = true;

Please leave readahead_sync_decompress = false 'auto' by default.

I don't like .readpage() applies async decompression since it's
actually a sync way, otherwise, it will cause more scheduling
overhead, see:
https://lore.kernel.org/r/20201016160443.18685-1-willy@infradead.org
https://lore.kernel.org/r/20201102184312.25926-16-willy@infradead.org

Thanks,
Gao Xiang

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] erofs: add sysfs interface
  2021-11-09  3:10 ` [PATCH 1/2] erofs: add sysfs interface Gao Xiang
@ 2021-11-09  7:29   ` Huang Jianan via Linux-erofs
  0 siblings, 0 replies; 20+ messages in thread
From: Huang Jianan via Linux-erofs @ 2021-11-09  7:29 UTC (permalink / raw)
  To: Gao Xiang
  Cc: zhangshiming, linux-kernel, yh, guanyuwei, guoweichao, linux-erofs

Hi Xiang,

在 2021/11/9 11:10, Gao Xiang 写道:
> Hi Jianan,
>
> On Tue, Nov 09, 2021 at 10:54:44AM +0800, Huang Jianan via Linux-erofs wrote:
>
> You might need to add a "From:" tag here, otherwise, the author will
> show "Huang Jianan via Linux-erofs" due to your mailing server...
I have used --from, maybe I need to add on the patch manually ... 🙁
>> Add sysfs interface to configure erofs related parameters in the
>> future.
> s/in the future/Later/
Will be fixed.
>> Signed-off-by: Huang Jianan <huangjianan@oppo.com>
>> ---
>>   fs/erofs/Makefile   |   2 +-
>>   fs/erofs/internal.h |  10 ++
>>   fs/erofs/super.c    |  12 +++
>>   fs/erofs/sysfs.c    | 239 ++++++++++++++++++++++++++++++++++++++++++++
> At a quick glance, we might need to add sysfs API documentation
> as well:
> Documentation/ABI/testing/sysfs-fs-erofs
I will attach it in the next version.

Thanks,
Jianan
> Thanks,
> Gao Xiang
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] erofs: add sysfs interface
  2021-11-09  3:14 ` Joe Perches
@ 2021-11-09  7:31   ` Huang Jianan via Linux-erofs
  0 siblings, 0 replies; 20+ messages in thread
From: Huang Jianan via Linux-erofs @ 2021-11-09  7:31 UTC (permalink / raw)
  To: Joe Perches, linux-erofs
  Cc: yh, guoweichao, zhangshiming, guanyuwei, linux-kernel

在 2021/11/9 11:14, Joe Perches 写道:
> On Tue, 2021-11-09 at 10:54 +0800, Huang Jianan wrote:
>> Add sysfs interface to configure erofs related parameters in the
>> future.
> []
>> diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
> []
>> +static ssize_t erofs_attr_show(struct kobject *kobj,
>> +				struct attribute *attr, char *buf)
>> +{
>> +	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
>> +						s_kobj);
>> +	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
>> +	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
>> +
>> +	switch (a->attr_id) {
>> +	case attr_feature:
>> +		return snprintf(buf, PAGE_SIZE, "supported\n");
>> +	case attr_pointer_ui:
>> +		if (!ptr)
>> +			return 0;
>> +		return snprintf(buf, PAGE_SIZE, "%u\n",
>> +				*((unsigned int *) ptr));
> Prefer sysfs_emit over snprintf
>
> 	case attr_feature:
> 		return sysfs_emit(buf, "supported\n");
> 	case attr_pointer_ui:
> 		...
> 		return sysfs_emit(buf, "%u\n", *(unsigned int *)ptr);
>
> etc...
>
Thanks for reminding this, I will fix it in the next version.

Thanks,
Jianan


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy
  2021-11-09  3:16   ` Gao Xiang
@ 2021-11-09  7:43     ` Huang Jianan via Linux-erofs
  2021-11-09  7:45       ` [PATCH v2 1/2] erofs: add sysfs interface Huang Jianan via Linux-erofs
  0 siblings, 1 reply; 20+ messages in thread
From: Huang Jianan via Linux-erofs @ 2021-11-09  7:43 UTC (permalink / raw)
  To: Gao Xiang
  Cc: zhangshiming, linux-kernel, yh, guanyuwei, guoweichao, linux-erofs

Hi Xiang,

在 2021/11/9 11:16, Gao Xiang 写道:
> On Tue, Nov 09, 2021 at 10:54:45AM +0800, Huang Jianan via Linux-erofs wrote:
>> Although readpage is a synchronous path, there will be no additional
>> kworker scheduling overhead in non-atomic contexts. So we can add a
>> sysfs node to allow disable sync decompression.
>>
>> Signed-off-by: Huang Jianan <huangjianan@oppo.com>
>> ---
>>   fs/erofs/internal.h | 2 +-
>>   fs/erofs/super.c    | 2 +-
>>   fs/erofs/sysfs.c    | 6 ++++++
>>   fs/erofs/zdata.c    | 9 ++++-----
>>   4 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
>> index d0cd712dc222..1ab96878009d 100644
>> --- a/fs/erofs/internal.h
>> +++ b/fs/erofs/internal.h
>> @@ -60,7 +60,7 @@ struct erofs_mount_opts {
>>   #ifdef CONFIG_EROFS_FS_ZIP
>>   	/* current strategy of how to use managed cache */
>>   	unsigned char cache_strategy;
>> -	/* strategy of sync decompression (false - auto, true - force on) */
>> +	/* strategy of sync decompression (false - disable, true - force on) */
> Please leave false - auto.
Agree.
>>   	bool readahead_sync_decompress;
>>   
>>   	/* threshold for decompression synchronously */
>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>> index abc1da5d1719..26585d865503 100644
>> --- a/fs/erofs/super.c
>> +++ b/fs/erofs/super.c
>> @@ -423,7 +423,7 @@ static void erofs_default_options(struct erofs_fs_context *ctx)
>>   #ifdef CONFIG_EROFS_FS_ZIP
>>   	ctx->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
>>   	ctx->opt.max_sync_decompress_pages = 3;
>> -	ctx->opt.readahead_sync_decompress = false;
>> +	ctx->opt.readahead_sync_decompress = true;
> Please leave readahead_sync_decompress = false 'auto' by default.
>
> I don't like .readpage() applies async decompression since it's
> actually a sync way, otherwise, it will cause more scheduling
> overhead, see:
> https://lore.kernel.org/r/20201016160443.18685-1-willy@infradead.org
> https://lore.kernel.org/r/20201102184312.25926-16-willy@infradead.org

Maybe consider adding a disbale strategy? And then we will have:
0 - auto, 1 - force on, 2 - disable
I will try this in the next version.

for .readpage(), If the decompression can be done in a non-atomic context,
there will be no additional scheduling overhead? In this case, we can 
turn off
the syncdecompression.

Thanks,
Jianan

> Thanks,
> Gao Xiang


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/2] erofs: add sysfs interface
  2021-11-09  7:43     ` Huang Jianan via Linux-erofs
@ 2021-11-09  7:45       ` Huang Jianan via Linux-erofs
  2021-11-09  7:45         ` [PATCH v2 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan via Linux-erofs
  2021-11-09 13:14         ` [PATCH v2 1/2] erofs: add sysfs interface Chao Yu
  0 siblings, 2 replies; 20+ messages in thread
From: Huang Jianan via Linux-erofs @ 2021-11-09  7:45 UTC (permalink / raw)
  To: linux-erofs; +Cc: zhangshiming, linux-kernel, yh, guanyuwei, guoweichao

Add sysfs interface to configure erofs related parameters later.

Signed-off-by: Huang Jianan <huangjianan@oppo.com>
---
changes since v1:
- Add sysfs API documentation.
- Use sysfs_emit over snprintf.

 Documentation/ABI/testing/sysfs-fs-erofs |   7 +
 fs/erofs/Makefile                        |   2 +-
 fs/erofs/internal.h                      |  10 +
 fs/erofs/super.c                         |  12 ++
 fs/erofs/sysfs.c                         | 237 +++++++++++++++++++++++
 5 files changed, 267 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-fs-erofs
 create mode 100644 fs/erofs/sysfs.c

diff --git a/Documentation/ABI/testing/sysfs-fs-erofs b/Documentation/ABI/testing/sysfs-fs-erofs
new file mode 100644
index 000000000000..86d0d0234473
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-fs-erofs
@@ -0,0 +1,7 @@
+What:		/sys/fs/erofs/features/
+Date:		November 2021
+Contact:	"Huang Jianan" <huangjianan@oppo.com>
+Description:	Shows all enabled kernel features.
+		Supported features:
+		lz4_0padding, compr_cfgs, big_pcluster, device_table,
+		sb_chksum.
diff --git a/fs/erofs/Makefile b/fs/erofs/Makefile
index 756fe2d65272..8a3317e38e5a 100644
--- a/fs/erofs/Makefile
+++ b/fs/erofs/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-$(CONFIG_EROFS_FS) += erofs.o
-erofs-objs := super.o inode.o data.o namei.o dir.o utils.o pcpubuf.o
+erofs-objs := super.o inode.o data.o namei.o dir.o utils.o pcpubuf.o sysfs.o
 erofs-$(CONFIG_EROFS_FS_XATTR) += xattr.o
 erofs-$(CONFIG_EROFS_FS_ZIP) += decompressor.o zmap.o zdata.o
 erofs-$(CONFIG_EROFS_FS_ZIP_LZMA) += decompressor_lzma.o
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 3265688af7f9..d0cd712dc222 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -134,6 +134,10 @@ struct erofs_sb_info {
 	u8 volume_name[16];             /* volume name */
 	u32 feature_compat;
 	u32 feature_incompat;
+
+	/* sysfs support */
+	struct kobject s_kobj;		/* /sys/fs/erofs/<devname> */
+	struct completion s_kobj_unregister;
 };
 
 #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info)
@@ -498,6 +502,12 @@ int erofs_pcpubuf_growsize(unsigned int nrpages);
 void erofs_pcpubuf_init(void);
 void erofs_pcpubuf_exit(void);
 
+/* sysfs.c */
+int erofs_register_sysfs(struct super_block *sb);
+void erofs_unregister_sysfs(struct super_block *sb);
+int __init erofs_init_sysfs(void);
+void erofs_exit_sysfs(void);
+
 /* utils.c / zdata.c */
 struct page *erofs_allocpage(struct page **pagepool, gfp_t gfp);
 static inline void erofs_pagepool_add(struct page **pagepool,
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 6a969b1e0ee6..abc1da5d1719 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -695,6 +695,10 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (err)
 		return err;
 
+	err = erofs_register_sysfs(sb);
+	if (err)
+		return err;
+
 	erofs_info(sb, "mounted with root inode @ nid %llu.", ROOT_NID(sbi));
 	return 0;
 }
@@ -808,6 +812,7 @@ static void erofs_put_super(struct super_block *sb)
 
 	DBG_BUGON(!sbi);
 
+	erofs_unregister_sysfs(sb);
 	erofs_shrinker_unregister(sb);
 #ifdef CONFIG_EROFS_FS_ZIP
 	iput(sbi->managed_cache);
@@ -852,6 +857,10 @@ static int __init erofs_module_init(void)
 	if (err)
 		goto zip_err;
 
+	err = erofs_init_sysfs();
+	if (err)
+		goto sysfs_err;
+
 	err = register_filesystem(&erofs_fs_type);
 	if (err)
 		goto fs_err;
@@ -859,6 +868,8 @@ static int __init erofs_module_init(void)
 	return 0;
 
 fs_err:
+	erofs_exit_sysfs();
+sysfs_err:
 	z_erofs_exit_zip_subsystem();
 zip_err:
 	z_erofs_lzma_exit();
@@ -877,6 +888,7 @@ static void __exit erofs_module_exit(void)
 	/* Ensure all RCU free inodes / pclusters are safe to be destroyed. */
 	rcu_barrier();
 
+	erofs_exit_sysfs();
 	z_erofs_exit_zip_subsystem();
 	z_erofs_lzma_exit();
 	erofs_exit_shrinker();
diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
new file mode 100644
index 000000000000..dd328d20c451
--- /dev/null
+++ b/fs/erofs/sysfs.c
@@ -0,0 +1,237 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C), 2008-2021, OPPO Mobile Comm Corp., Ltd.
+ *             https://www.oppo.com/
+ */
+#include <linux/sysfs.h>
+#include <linux/kobject.h>
+
+#include "internal.h"
+
+enum {
+	attr_feature,
+	attr_pointer_ui,
+	attr_pointer_bool,
+};
+
+enum {
+	struct_erofs_sb_info,
+};
+
+struct erofs_attr {
+	struct attribute attr;
+	short attr_id;
+	int struct_type;
+	int offset;
+};
+
+#define EROFS_ATTR(_name, _mode, _id)					\
+static struct erofs_attr erofs_attr_##_name = {				\
+	.attr = {.name = __stringify(_name), .mode = _mode },		\
+	.attr_id = attr_##_id,						\
+}
+#define EROFS_ATTR_FUNC(_name, _mode)	EROFS_ATTR(_name, _mode, _name)
+#define EROFS_ATTR_FEATURE(_name)	EROFS_ATTR(_name, 0444, feature)
+
+#define EROFS_ATTR_OFFSET(_name, _mode, _id, _struct)	\
+static struct erofs_attr erofs_attr_##_name = {			\
+	.attr = {.name = __stringify(_name), .mode = _mode },	\
+	.attr_id = attr_##_id,					\
+	.struct_type = struct_##_struct,			\
+	.offset = offsetof(struct _struct, _name),\
+}
+
+#define EROFS_ATTR_RW(_name, _id, _struct)	\
+	EROFS_ATTR_OFFSET(_name, 0644, _id, _struct)
+
+#define EROFS_RO_ATTR(_name, _id, _struct)	\
+	EROFS_ATTR_OFFSET(_name, 0444, _id, _struct)
+
+#define EROFS_ATTR_RW_UI(_name, _struct)	\
+	EROFS_ATTR_RW(_name, pointer_ui, _struct)
+
+#define EROFS_ATTR_RW_BOOL(_name, _struct)	\
+	EROFS_ATTR_RW(_name, pointer_bool, _struct)
+
+#define ATTR_LIST(name) (&erofs_attr_##name.attr)
+
+static struct attribute *erofs_attrs[] = {
+	NULL,
+};
+ATTRIBUTE_GROUPS(erofs);
+
+/* Features this copy of erofs supports */
+EROFS_ATTR_FEATURE(lz4_0padding);
+EROFS_ATTR_FEATURE(compr_cfgs);
+EROFS_ATTR_FEATURE(big_pcluster);
+EROFS_ATTR_FEATURE(device_table);
+EROFS_ATTR_FEATURE(sb_chksum);
+
+static struct attribute *erofs_feat_attrs[] = {
+	ATTR_LIST(lz4_0padding),
+	ATTR_LIST(compr_cfgs),
+	ATTR_LIST(big_pcluster),
+	ATTR_LIST(device_table),
+	ATTR_LIST(sb_chksum),
+	NULL,
+};
+ATTRIBUTE_GROUPS(erofs_feat);
+
+static unsigned char *__struct_ptr(struct erofs_sb_info *sbi,
+					  int struct_type, int offset)
+{
+	if (struct_type == struct_erofs_sb_info)
+		return (unsigned char *)sbi + offset;
+	return NULL;
+}
+
+static ssize_t erofs_attr_show(struct kobject *kobj,
+				struct attribute *attr, char *buf)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+						s_kobj);
+	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
+	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
+
+	switch (a->attr_id) {
+	case attr_feature:
+		return sysfs_emit(buf, "supported\n");
+	case attr_pointer_ui:
+		if (!ptr)
+			return 0;
+		return sysfs_emit(buf, "%u\n", *(unsigned int *)ptr);
+	case attr_pointer_bool:
+		if (!ptr)
+			return 0;
+		return sysfs_emit(buf, *(bool *)ptr ? "true\n" : "false\n");
+	}
+
+	return 0;
+}
+
+static ssize_t erofs_attr_store(struct kobject *kobj, struct attribute *attr,
+						const char *buf, size_t len)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+						s_kobj);
+	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
+	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
+	unsigned long t;
+	int ret;
+
+	switch (a->attr_id) {
+	case attr_pointer_ui:
+		if (!ptr)
+			return 0;
+		ret = kstrtoul(skip_spaces(buf), 0, &t);
+		if (ret)
+			return ret;
+		*((unsigned int *) ptr) = t;
+		return len;
+	case attr_pointer_bool:
+		if (!ptr)
+			return 0;
+		ret = kstrtoul(skip_spaces(buf), 0, &t);
+		if (ret)
+			return ret;
+		*((bool *) ptr) = !!t;
+		return len;
+	}
+
+	return 0;
+}
+
+static void erofs_sb_release(struct kobject *kobj)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+								s_kobj);
+	complete(&sbi->s_kobj_unregister);
+}
+
+static const struct sysfs_ops erofs_attr_ops = {
+	.show	= erofs_attr_show,
+	.store	= erofs_attr_store,
+};
+
+static struct kobj_type erofs_sb_ktype = {
+	.default_groups = erofs_groups,
+	.sysfs_ops	= &erofs_attr_ops,
+	.release	= erofs_sb_release,
+};
+
+static struct kobj_type erofs_ktype = {
+	.sysfs_ops	= &erofs_attr_ops,
+};
+
+static struct kset erofs_root = {
+	.kobj	= {.ktype = &erofs_ktype},
+};
+
+static struct kobj_type erofs_feat_ktype = {
+	.default_groups = erofs_feat_groups,
+	.sysfs_ops	= &erofs_attr_ops,
+};
+
+static struct kobject erofs_feat = {
+	.kset	= &erofs_root,
+};
+
+int erofs_register_sysfs(struct super_block *sb)
+{
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	int err;
+
+	sbi->s_kobj.kset = &erofs_root;
+	init_completion(&sbi->s_kobj_unregister);
+	err = kobject_init_and_add(&sbi->s_kobj, &erofs_sb_ktype, NULL,
+				"%s", sb->s_id);
+	if (err)
+		goto put_sb_kobj;
+
+	return 0;
+
+put_sb_kobj:
+	kobject_put(&sbi->s_kobj);
+	wait_for_completion(&sbi->s_kobj_unregister);
+	return err;
+}
+
+void erofs_unregister_sysfs(struct super_block *sb)
+{
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+
+	kobject_del(&sbi->s_kobj);
+	kobject_put(&sbi->s_kobj);
+	wait_for_completion(&sbi->s_kobj_unregister);
+}
+
+int __init erofs_init_sysfs(void)
+{
+	int ret;
+
+	kobject_set_name(&erofs_root.kobj, "erofs");
+	erofs_root.kobj.parent = fs_kobj;
+	ret = kset_register(&erofs_root);
+	if (ret)
+		goto root_err;
+
+	ret = kobject_init_and_add(&erofs_feat, &erofs_feat_ktype,
+				   NULL, "features");
+	if (ret)
+		goto feat_err;
+
+	return ret;
+
+feat_err:
+	kobject_put(&erofs_feat);
+	kset_unregister(&erofs_root);
+root_err:
+	return ret;
+}
+
+void erofs_exit_sysfs(void)
+{
+	kobject_put(&erofs_feat);
+	kset_unregister(&erofs_root);
+}
+
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/2] erofs: add sysfs node to control sync decompression strategy
  2021-11-09  7:45       ` [PATCH v2 1/2] erofs: add sysfs interface Huang Jianan via Linux-erofs
@ 2021-11-09  7:45         ` Huang Jianan via Linux-erofs
  2021-11-09 13:27           ` Chao Yu
  2021-11-09 13:14         ` [PATCH v2 1/2] erofs: add sysfs interface Chao Yu
  1 sibling, 1 reply; 20+ messages in thread
From: Huang Jianan via Linux-erofs @ 2021-11-09  7:45 UTC (permalink / raw)
  To: linux-erofs; +Cc: zhangshiming, linux-kernel, yh, guanyuwei, guoweichao

Although readpage is a synchronous path, there will be no additional
kworker scheduling overhead in non-atomic contexts. So add a sysfs
node to allow disable sync decompression.

Signed-off-by: Huang Jianan <huangjianan@oppo.com>
---
changes since v1:
- leave auto default
- add a disable strategy for sync_decompress

 Documentation/ABI/testing/sysfs-fs-erofs |  9 +++++++++
 fs/erofs/internal.h                      |  4 ++--
 fs/erofs/super.c                         |  2 +-
 fs/erofs/sysfs.c                         | 10 ++++++++++
 fs/erofs/zdata.c                         | 19 ++++++++++++++-----
 5 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-erofs b/Documentation/ABI/testing/sysfs-fs-erofs
index 86d0d0234473..c84f12004f02 100644
--- a/Documentation/ABI/testing/sysfs-fs-erofs
+++ b/Documentation/ABI/testing/sysfs-fs-erofs
@@ -5,3 +5,12 @@ Description:	Shows all enabled kernel features.
 		Supported features:
 		lz4_0padding, compr_cfgs, big_pcluster, device_table,
 		sb_chksum.
+
+What:		/sys/fs/erofs/<disk>/sync_decompress
+Date:		November 2021
+Contact:	"Huang Jianan" <huangjianan@oppo.com>
+Description:	Control strategy of sync decompression
+		- 0 (defalut, auto): enable for readpage, and enable for
+				     readahead on atomic contexts only,
+		- 1 (force on): enable for readpage and readahead.
+		- 2 (disable): disable for all situations.
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index d0cd712dc222..06a8bbdb378f 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -60,8 +60,8 @@ struct erofs_mount_opts {
 #ifdef CONFIG_EROFS_FS_ZIP
 	/* current strategy of how to use managed cache */
 	unsigned char cache_strategy;
-	/* strategy of sync decompression (false - auto, true - force on) */
-	bool readahead_sync_decompress;
+	/* strategy of sync decompression (0 - auto, 1 - force on, 2 - disable) */
+	unsigned int sync_decompress;
 
 	/* threshold for decompression synchronously */
 	unsigned int max_sync_decompress_pages;
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index abc1da5d1719..ea223d6c7985 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -423,7 +423,7 @@ static void erofs_default_options(struct erofs_fs_context *ctx)
 #ifdef CONFIG_EROFS_FS_ZIP
 	ctx->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
 	ctx->opt.max_sync_decompress_pages = 3;
-	ctx->opt.readahead_sync_decompress = false;
+	ctx->opt.sync_decompress = 0;
 #endif
 #ifdef CONFIG_EROFS_FS_XATTR
 	set_opt(&ctx->opt, XATTR_USER);
diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
index dd328d20c451..a8889b2b65f3 100644
--- a/fs/erofs/sysfs.c
+++ b/fs/erofs/sysfs.c
@@ -16,6 +16,7 @@ enum {
 
 enum {
 	struct_erofs_sb_info,
+	struct_erofs_mount_opts,
 };
 
 struct erofs_attr {
@@ -55,7 +56,10 @@ static struct erofs_attr erofs_attr_##_name = {			\
 
 #define ATTR_LIST(name) (&erofs_attr_##name.attr)
 
+EROFS_ATTR_RW_UI(sync_decompress, erofs_mount_opts);
+
 static struct attribute *erofs_attrs[] = {
+	ATTR_LIST(sync_decompress),
 	NULL,
 };
 ATTRIBUTE_GROUPS(erofs);
@@ -82,6 +86,8 @@ static unsigned char *__struct_ptr(struct erofs_sb_info *sbi,
 {
 	if (struct_type == struct_erofs_sb_info)
 		return (unsigned char *)sbi + offset;
+	if (struct_type == struct_erofs_mount_opts)
+		return (unsigned char *)&sbi->opt + offset;
 	return NULL;
 }
 
@@ -126,6 +132,10 @@ static ssize_t erofs_attr_store(struct kobject *kobj, struct attribute *attr,
 		ret = kstrtoul(skip_spaces(buf), 0, &t);
 		if (ret)
 			return ret;
+		
+		if (!strcmp(a->attr.name, "sync_decompress") && (t > 2))
+			return -EINVAL;
+
 		*((unsigned int *) ptr) = t;
 		return len;
 	case attr_pointer_bool:
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index bcb1b91b234f..70ec51fa7131 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -794,7 +794,8 @@ static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
 	/* Use workqueue and sync decompression for atomic contexts only */
 	if (in_atomic() || irqs_disabled()) {
 		queue_work(z_erofs_workqueue, &io->u.work);
-		sbi->opt.readahead_sync_decompress = true;
+		if (sbi->opt.sync_decompress == 0)
+			sbi->opt.sync_decompress = 1;
 		return;
 	}
 	z_erofs_decompressqueue_work(&io->u.work);
@@ -1454,9 +1455,11 @@ static void z_erofs_pcluster_readmore(struct z_erofs_decompress_frontend *f,
 static int z_erofs_readpage(struct file *file, struct page *page)
 {
 	struct inode *const inode = page->mapping->host;
+	struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
 	struct z_erofs_decompress_frontend f = DECOMPRESS_FRONTEND_INIT(inode);
 	struct page *pagepool = NULL;
 	int err;
+	bool force_fg = true;
 
 	trace_erofs_readpage(page, false);
 	f.headoffset = (erofs_off_t)page->index << PAGE_SHIFT;
@@ -1468,8 +1471,11 @@ static int z_erofs_readpage(struct file *file, struct page *page)
 
 	(void)z_erofs_collector_end(&f.clt);
 
+	if (sbi->opt.sync_decompress == 2)
+		force_fg = false;
+
 	/* if some compressed cluster ready, need submit them anyway */
-	z_erofs_runqueue(inode->i_sb, &f, &pagepool, true);
+	z_erofs_runqueue(inode->i_sb, &f, &pagepool, force_fg);
 
 	if (err)
 		erofs_err(inode->i_sb, "failed to read, err [%d]", err);
@@ -1488,6 +1494,7 @@ static void z_erofs_readahead(struct readahead_control *rac)
 	struct z_erofs_decompress_frontend f = DECOMPRESS_FRONTEND_INIT(inode);
 	struct page *pagepool = NULL, *head = NULL, *page;
 	unsigned int nr_pages;
+	bool force_fg = false;
 
 	f.readahead = true;
 	f.headoffset = readahead_pos(rac);
@@ -1519,9 +1526,11 @@ static void z_erofs_readahead(struct readahead_control *rac)
 	z_erofs_pcluster_readmore(&f, rac, 0, &pagepool, false);
 	(void)z_erofs_collector_end(&f.clt);
 
-	z_erofs_runqueue(inode->i_sb, &f, &pagepool,
-			 sbi->opt.readahead_sync_decompress &&
-			 nr_pages <= sbi->opt.max_sync_decompress_pages);
+	if (sbi->opt.sync_decompress == 1)
+		force_fg = true;
+
+	z_erofs_runqueue(inode->i_sb, &f, &pagepool, force_fg
+			 && nr_pages <= sbi->opt.max_sync_decompress_pages);
 	if (f.map.mpage)
 		put_page(f.map.mpage);
 	erofs_release_pages(&pagepool);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/2] erofs: add sysfs interface
  2021-11-09  7:45       ` [PATCH v2 1/2] erofs: add sysfs interface Huang Jianan via Linux-erofs
  2021-11-09  7:45         ` [PATCH v2 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan via Linux-erofs
@ 2021-11-09 13:14         ` Chao Yu
  1 sibling, 0 replies; 20+ messages in thread
From: Chao Yu @ 2021-11-09 13:14 UTC (permalink / raw)
  To: Huang Jianan, linux-erofs
  Cc: yh, zhangshiming, guoweichao, linux-kernel, guanyuwei

On 2021/11/9 15:45, Huang Jianan via Linux-erofs wrote:
> Add sysfs interface to configure erofs related parameters later.
> 
> Signed-off-by: Huang Jianan <huangjianan@oppo.com>
> ---
> changes since v1:
> - Add sysfs API documentation.
> - Use sysfs_emit over snprintf.
> 
>   Documentation/ABI/testing/sysfs-fs-erofs |   7 +
>   fs/erofs/Makefile                        |   2 +-
>   fs/erofs/internal.h                      |  10 +
>   fs/erofs/super.c                         |  12 ++
>   fs/erofs/sysfs.c                         | 237 +++++++++++++++++++++++
>   5 files changed, 267 insertions(+), 1 deletion(-)
>   create mode 100644 Documentation/ABI/testing/sysfs-fs-erofs
>   create mode 100644 fs/erofs/sysfs.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-fs-erofs b/Documentation/ABI/testing/sysfs-fs-erofs
> new file mode 100644
> index 000000000000..86d0d0234473
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-fs-erofs
> @@ -0,0 +1,7 @@
> +What:		/sys/fs/erofs/features/
> +Date:		November 2021
> +Contact:	"Huang Jianan" <huangjianan@oppo.com>
> +Description:	Shows all enabled kernel features.
> +		Supported features:
> +		lz4_0padding, compr_cfgs, big_pcluster, device_table,
> +		sb_chksum.
> diff --git a/fs/erofs/Makefile b/fs/erofs/Makefile
> index 756fe2d65272..8a3317e38e5a 100644
> --- a/fs/erofs/Makefile
> +++ b/fs/erofs/Makefile
> @@ -1,7 +1,7 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   
>   obj-$(CONFIG_EROFS_FS) += erofs.o
> -erofs-objs := super.o inode.o data.o namei.o dir.o utils.o pcpubuf.o
> +erofs-objs := super.o inode.o data.o namei.o dir.o utils.o pcpubuf.o sysfs.o
>   erofs-$(CONFIG_EROFS_FS_XATTR) += xattr.o
>   erofs-$(CONFIG_EROFS_FS_ZIP) += decompressor.o zmap.o zdata.o
>   erofs-$(CONFIG_EROFS_FS_ZIP_LZMA) += decompressor_lzma.o
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 3265688af7f9..d0cd712dc222 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -134,6 +134,10 @@ struct erofs_sb_info {
>   	u8 volume_name[16];             /* volume name */
>   	u32 feature_compat;
>   	u32 feature_incompat;
> +
> +	/* sysfs support */
> +	struct kobject s_kobj;		/* /sys/fs/erofs/<devname> */
> +	struct completion s_kobj_unregister;
>   };
>   
>   #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info)
> @@ -498,6 +502,12 @@ int erofs_pcpubuf_growsize(unsigned int nrpages);
>   void erofs_pcpubuf_init(void);
>   void erofs_pcpubuf_exit(void);
>   
> +/* sysfs.c */
> +int erofs_register_sysfs(struct super_block *sb);
> +void erofs_unregister_sysfs(struct super_block *sb);
> +int __init erofs_init_sysfs(void);
> +void erofs_exit_sysfs(void);
> +
>   /* utils.c / zdata.c */
>   struct page *erofs_allocpage(struct page **pagepool, gfp_t gfp);
>   static inline void erofs_pagepool_add(struct page **pagepool,
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 6a969b1e0ee6..abc1da5d1719 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -695,6 +695,10 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>   	if (err)
>   		return err;
>   
> +	err = erofs_register_sysfs(sb);
> +	if (err)
> +		return err;
> +
>   	erofs_info(sb, "mounted with root inode @ nid %llu.", ROOT_NID(sbi));
>   	return 0;
>   }
> @@ -808,6 +812,7 @@ static void erofs_put_super(struct super_block *sb)
>   
>   	DBG_BUGON(!sbi);
>   
> +	erofs_unregister_sysfs(sb);
>   	erofs_shrinker_unregister(sb);
>   #ifdef CONFIG_EROFS_FS_ZIP
>   	iput(sbi->managed_cache);
> @@ -852,6 +857,10 @@ static int __init erofs_module_init(void)
>   	if (err)
>   		goto zip_err;
>   
> +	err = erofs_init_sysfs();
> +	if (err)
> +		goto sysfs_err;
> +
>   	err = register_filesystem(&erofs_fs_type);
>   	if (err)
>   		goto fs_err;
> @@ -859,6 +868,8 @@ static int __init erofs_module_init(void)
>   	return 0;
>   
>   fs_err:
> +	erofs_exit_sysfs();
> +sysfs_err:
>   	z_erofs_exit_zip_subsystem();
>   zip_err:
>   	z_erofs_lzma_exit();
> @@ -877,6 +888,7 @@ static void __exit erofs_module_exit(void)
>   	/* Ensure all RCU free inodes / pclusters are safe to be destroyed. */
>   	rcu_barrier();
>   
> +	erofs_exit_sysfs();
>   	z_erofs_exit_zip_subsystem();
>   	z_erofs_lzma_exit();
>   	erofs_exit_shrinker();
> diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
> new file mode 100644
> index 000000000000..dd328d20c451
> --- /dev/null
> +++ b/fs/erofs/sysfs.c
> @@ -0,0 +1,237 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C), 2008-2021, OPPO Mobile Comm Corp., Ltd.
> + *             https://www.oppo.com/
> + */
> +#include <linux/sysfs.h>
> +#include <linux/kobject.h>
> +
> +#include "internal.h"
> +
> +enum {
> +	attr_feature,
> +	attr_pointer_ui,
> +	attr_pointer_bool,
> +};
> +
> +enum {
> +	struct_erofs_sb_info,
> +};
> +
> +struct erofs_attr {
> +	struct attribute attr;
> +	short attr_id;
> +	int struct_type;
> +	int offset;
> +};
> +
> +#define EROFS_ATTR(_name, _mode, _id)					\
> +static struct erofs_attr erofs_attr_##_name = {				\
> +	.attr = {.name = __stringify(_name), .mode = _mode },		\
> +	.attr_id = attr_##_id,						\
> +}
> +#define EROFS_ATTR_FUNC(_name, _mode)	EROFS_ATTR(_name, _mode, _name)
> +#define EROFS_ATTR_FEATURE(_name)	EROFS_ATTR(_name, 0444, feature)
> +
> +#define EROFS_ATTR_OFFSET(_name, _mode, _id, _struct)	\
> +static struct erofs_attr erofs_attr_##_name = {			\
> +	.attr = {.name = __stringify(_name), .mode = _mode },	\
> +	.attr_id = attr_##_id,					\
> +	.struct_type = struct_##_struct,			\
> +	.offset = offsetof(struct _struct, _name),\
> +}
> +
> +#define EROFS_ATTR_RW(_name, _id, _struct)	\
> +	EROFS_ATTR_OFFSET(_name, 0644, _id, _struct)
> +
> +#define EROFS_RO_ATTR(_name, _id, _struct)	\
> +	EROFS_ATTR_OFFSET(_name, 0444, _id, _struct)
> +
> +#define EROFS_ATTR_RW_UI(_name, _struct)	\
> +	EROFS_ATTR_RW(_name, pointer_ui, _struct)
> +
> +#define EROFS_ATTR_RW_BOOL(_name, _struct)	\
> +	EROFS_ATTR_RW(_name, pointer_bool, _struct)
> +
> +#define ATTR_LIST(name) (&erofs_attr_##name.attr)
> +
> +static struct attribute *erofs_attrs[] = {
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(erofs);
> +
> +/* Features this copy of erofs supports */
> +EROFS_ATTR_FEATURE(lz4_0padding);
> +EROFS_ATTR_FEATURE(compr_cfgs);
> +EROFS_ATTR_FEATURE(big_pcluster);
> +EROFS_ATTR_FEATURE(device_table);
> +EROFS_ATTR_FEATURE(sb_chksum);
> +
> +static struct attribute *erofs_feat_attrs[] = {
> +	ATTR_LIST(lz4_0padding),
> +	ATTR_LIST(compr_cfgs),
> +	ATTR_LIST(big_pcluster),
> +	ATTR_LIST(device_table),
> +	ATTR_LIST(sb_chksum),
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(erofs_feat);
> +
> +static unsigned char *__struct_ptr(struct erofs_sb_info *sbi,
> +					  int struct_type, int offset)
> +{
> +	if (struct_type == struct_erofs_sb_info)
> +		return (unsigned char *)sbi + offset;
> +	return NULL;
> +}
> +
> +static ssize_t erofs_attr_show(struct kobject *kobj,
> +				struct attribute *attr, char *buf)
> +{
> +	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
> +						s_kobj);
> +	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
> +	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
> +
> +	switch (a->attr_id) {
> +	case attr_feature:
> +		return sysfs_emit(buf, "supported\n");
> +	case attr_pointer_ui:
> +		if (!ptr)
> +			return 0;
> +		return sysfs_emit(buf, "%u\n", *(unsigned int *)ptr);
> +	case attr_pointer_bool:
> +		if (!ptr)
> +			return 0;
> +		return sysfs_emit(buf, *(bool *)ptr ? "true\n" : "false\n");

Is it fine to just print 1 or 0 like the value user set via the sysfs entry
to indicate true or false.

> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t erofs_attr_store(struct kobject *kobj, struct attribute *attr,
> +						const char *buf, size_t len)
> +{
> +	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
> +						s_kobj);
> +	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
> +	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
> +	unsigned long t;
> +	int ret;
> +
> +	switch (a->attr_id) {
> +	case attr_pointer_ui:
> +		if (!ptr)
> +			return 0;
> +		ret = kstrtoul(skip_spaces(buf), 0, &t);
> +		if (ret)
> +			return ret;

if (t > UINT_MAX)
	return -EINVAL;

> +		*((unsigned int *) ptr) = t;
> +		return len;
> +	case attr_pointer_bool:
> +		if (!ptr)
> +			return 0;
> +		ret = kstrtoul(skip_spaces(buf), 0, &t);
> +		if (ret)
> +			return ret;

@t should be 0 or 1?

if (t != 0 && t != 1)
	return -EINVAL;

Thanks,

> +		*((bool *) ptr) = !!t;
> +		return len;
> +	}
> +
> +	return 0;
> +}
> +
> +static void erofs_sb_release(struct kobject *kobj)
> +{
> +	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
> +								s_kobj);
> +	complete(&sbi->s_kobj_unregister);
> +}
> +
> +static const struct sysfs_ops erofs_attr_ops = {
> +	.show	= erofs_attr_show,
> +	.store	= erofs_attr_store,
> +};
> +
> +static struct kobj_type erofs_sb_ktype = {
> +	.default_groups = erofs_groups,
> +	.sysfs_ops	= &erofs_attr_ops,
> +	.release	= erofs_sb_release,
> +};
> +
> +static struct kobj_type erofs_ktype = {
> +	.sysfs_ops	= &erofs_attr_ops,
> +};
> +
> +static struct kset erofs_root = {
> +	.kobj	= {.ktype = &erofs_ktype},
> +};
> +
> +static struct kobj_type erofs_feat_ktype = {
> +	.default_groups = erofs_feat_groups,
> +	.sysfs_ops	= &erofs_attr_ops,
> +};
> +
> +static struct kobject erofs_feat = {
> +	.kset	= &erofs_root,
> +};
> +
> +int erofs_register_sysfs(struct super_block *sb)
> +{
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	int err;
> +
> +	sbi->s_kobj.kset = &erofs_root;
> +	init_completion(&sbi->s_kobj_unregister);
> +	err = kobject_init_and_add(&sbi->s_kobj, &erofs_sb_ktype, NULL,
> +				"%s", sb->s_id);
> +	if (err)
> +		goto put_sb_kobj;
> +
> +	return 0;
> +
> +put_sb_kobj:
> +	kobject_put(&sbi->s_kobj);
> +	wait_for_completion(&sbi->s_kobj_unregister);
> +	return err;
> +}
> +
> +void erofs_unregister_sysfs(struct super_block *sb)
> +{
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +
> +	kobject_del(&sbi->s_kobj);
> +	kobject_put(&sbi->s_kobj);
> +	wait_for_completion(&sbi->s_kobj_unregister);
> +}
> +
> +int __init erofs_init_sysfs(void)
> +{
> +	int ret;
> +
> +	kobject_set_name(&erofs_root.kobj, "erofs");
> +	erofs_root.kobj.parent = fs_kobj;
> +	ret = kset_register(&erofs_root);
> +	if (ret)
> +		goto root_err;
> +
> +	ret = kobject_init_and_add(&erofs_feat, &erofs_feat_ktype,
> +				   NULL, "features");
> +	if (ret)
> +		goto feat_err;
> +
> +	return ret;
> +
> +feat_err:
> +	kobject_put(&erofs_feat);
> +	kset_unregister(&erofs_root);
> +root_err:
> +	return ret;
> +}
> +
> +void erofs_exit_sysfs(void)
> +{
> +	kobject_put(&erofs_feat);
> +	kset_unregister(&erofs_root);
> +}
> +
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] erofs: add sysfs node to control sync decompression strategy
  2021-11-09  7:45         ` [PATCH v2 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan via Linux-erofs
@ 2021-11-09 13:27           ` Chao Yu
  2021-11-09 15:38             ` [PATCH v3 1/2] erofs: add sysfs interface Huang Jianan
  0 siblings, 1 reply; 20+ messages in thread
From: Chao Yu @ 2021-11-09 13:27 UTC (permalink / raw)
  To: Huang Jianan, linux-erofs
  Cc: yh, zhangshiming, guoweichao, linux-kernel, guanyuwei

On 2021/11/9 15:45, Huang Jianan via Linux-erofs wrote:
> Although readpage is a synchronous path, there will be no additional
> kworker scheduling overhead in non-atomic contexts. So add a sysfs
> node to allow disable sync decompression.
> 
> Signed-off-by: Huang Jianan <huangjianan@oppo.com>
> ---
> changes since v1:
> - leave auto default
> - add a disable strategy for sync_decompress
> 
>   Documentation/ABI/testing/sysfs-fs-erofs |  9 +++++++++
>   fs/erofs/internal.h                      |  4 ++--
>   fs/erofs/super.c                         |  2 +-
>   fs/erofs/sysfs.c                         | 10 ++++++++++
>   fs/erofs/zdata.c                         | 19 ++++++++++++++-----
>   5 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-fs-erofs b/Documentation/ABI/testing/sysfs-fs-erofs
> index 86d0d0234473..c84f12004f02 100644
> --- a/Documentation/ABI/testing/sysfs-fs-erofs
> +++ b/Documentation/ABI/testing/sysfs-fs-erofs
> @@ -5,3 +5,12 @@ Description:	Shows all enabled kernel features.
>   		Supported features:
>   		lz4_0padding, compr_cfgs, big_pcluster, device_table,
>   		sb_chksum.
> +
> +What:		/sys/fs/erofs/<disk>/sync_decompress
> +Date:		November 2021
> +Contact:	"Huang Jianan" <huangjianan@oppo.com>
> +Description:	Control strategy of sync decompression
> +		- 0 (defalut, auto): enable for readpage, and enable for
> +				     readahead on atomic contexts only,
> +		- 1 (force on): enable for readpage and readahead.
> +		- 2 (disable): disable for all situations.

1 (force on)
2 (force off)

> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index d0cd712dc222..06a8bbdb378f 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -60,8 +60,8 @@ struct erofs_mount_opts {
>   #ifdef CONFIG_EROFS_FS_ZIP
>   	/* current strategy of how to use managed cache */
>   	unsigned char cache_strategy;
> -	/* strategy of sync decompression (false - auto, true - force on) */
> -	bool readahead_sync_decompress;
> +	/* strategy of sync decompression (0 - auto, 1 - force on, 2 - disable) */

Ditto,

> +	unsigned int sync_decompress;
>   
>   	/* threshold for decompression synchronously */
>   	unsigned int max_sync_decompress_pages;
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index abc1da5d1719..ea223d6c7985 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -423,7 +423,7 @@ static void erofs_default_options(struct erofs_fs_context *ctx)
>   #ifdef CONFIG_EROFS_FS_ZIP
>   	ctx->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
>   	ctx->opt.max_sync_decompress_pages = 3;
> -	ctx->opt.readahead_sync_decompress = false;
> +	ctx->opt.sync_decompress = 0;
>   #endif
>   #ifdef CONFIG_EROFS_FS_XATTR
>   	set_opt(&ctx->opt, XATTR_USER);
> diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
> index dd328d20c451..a8889b2b65f3 100644
> --- a/fs/erofs/sysfs.c
> +++ b/fs/erofs/sysfs.c
> @@ -16,6 +16,7 @@ enum {
>   
>   enum {
>   	struct_erofs_sb_info,
> +	struct_erofs_mount_opts,
>   };
>   
>   struct erofs_attr {
> @@ -55,7 +56,10 @@ static struct erofs_attr erofs_attr_##_name = {			\
>   
>   #define ATTR_LIST(name) (&erofs_attr_##name.attr)
>   
> +EROFS_ATTR_RW_UI(sync_decompress, erofs_mount_opts);
> +
>   static struct attribute *erofs_attrs[] = {
> +	ATTR_LIST(sync_decompress),
>   	NULL,
>   };
>   ATTRIBUTE_GROUPS(erofs);
> @@ -82,6 +86,8 @@ static unsigned char *__struct_ptr(struct erofs_sb_info *sbi,
>   {
>   	if (struct_type == struct_erofs_sb_info)
>   		return (unsigned char *)sbi + offset;
> +	if (struct_type == struct_erofs_mount_opts)
> +		return (unsigned char *)&sbi->opt + offset;
>   	return NULL;
>   }
>   
> @@ -126,6 +132,10 @@ static ssize_t erofs_attr_store(struct kobject *kobj, struct attribute *attr,
>   		ret = kstrtoul(skip_spaces(buf), 0, &t);
>   		if (ret)
>   			return ret;
> +		
> +		if (!strcmp(a->attr.name, "sync_decompress") && (t > 2))

if (!strcmp()) {
	if (t > 2)
		return -EINVAL;
	*((unsigned int *) ptr) = t;
	return len;
}

How about adding enum instead of raw numbers to improve readability?

Thanks,

> +			return -EINVAL;
> +
>   		*((unsigned int *) ptr) = t;
>   		return len;
>   	case attr_pointer_bool:
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index bcb1b91b234f..70ec51fa7131 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -794,7 +794,8 @@ static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
>   	/* Use workqueue and sync decompression for atomic contexts only */
>   	if (in_atomic() || irqs_disabled()) {
>   		queue_work(z_erofs_workqueue, &io->u.work);
> -		sbi->opt.readahead_sync_decompress = true;
> +		if (sbi->opt.sync_decompress == 0)
> +			sbi->opt.sync_decompress = 1;
>   		return;
>   	}
>   	z_erofs_decompressqueue_work(&io->u.work);
> @@ -1454,9 +1455,11 @@ static void z_erofs_pcluster_readmore(struct z_erofs_decompress_frontend *f,
>   static int z_erofs_readpage(struct file *file, struct page *page)
>   {
>   	struct inode *const inode = page->mapping->host;
> +	struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
>   	struct z_erofs_decompress_frontend f = DECOMPRESS_FRONTEND_INIT(inode);
>   	struct page *pagepool = NULL;
>   	int err;
> +	bool force_fg = true;
>   
>   	trace_erofs_readpage(page, false);
>   	f.headoffset = (erofs_off_t)page->index << PAGE_SHIFT;
> @@ -1468,8 +1471,11 @@ static int z_erofs_readpage(struct file *file, struct page *page)
>   
>   	(void)z_erofs_collector_end(&f.clt);
>   
> +	if (sbi->opt.sync_decompress == 2)
> +		force_fg = false;
> +
>   	/* if some compressed cluster ready, need submit them anyway */
> -	z_erofs_runqueue(inode->i_sb, &f, &pagepool, true);
> +	z_erofs_runqueue(inode->i_sb, &f, &pagepool, force_fg);
>   
>   	if (err)
>   		erofs_err(inode->i_sb, "failed to read, err [%d]", err);
> @@ -1488,6 +1494,7 @@ static void z_erofs_readahead(struct readahead_control *rac)
>   	struct z_erofs_decompress_frontend f = DECOMPRESS_FRONTEND_INIT(inode);
>   	struct page *pagepool = NULL, *head = NULL, *page;
>   	unsigned int nr_pages;
> +	bool force_fg = false;
>   
>   	f.readahead = true;
>   	f.headoffset = readahead_pos(rac);
> @@ -1519,9 +1526,11 @@ static void z_erofs_readahead(struct readahead_control *rac)
>   	z_erofs_pcluster_readmore(&f, rac, 0, &pagepool, false);
>   	(void)z_erofs_collector_end(&f.clt);
>   
> -	z_erofs_runqueue(inode->i_sb, &f, &pagepool,
> -			 sbi->opt.readahead_sync_decompress &&
> -			 nr_pages <= sbi->opt.max_sync_decompress_pages);
> +	if (sbi->opt.sync_decompress == 1)
> +		force_fg = true;
> +
> +	z_erofs_runqueue(inode->i_sb, &f, &pagepool, force_fg
> +			 && nr_pages <= sbi->opt.max_sync_decompress_pages);
>   	if (f.map.mpage)
>   		put_page(f.map.mpage);
>   	erofs_release_pages(&pagepool);
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 1/2] erofs: add sysfs interface
  2021-11-09 13:27           ` Chao Yu
@ 2021-11-09 15:38             ` Huang Jianan
  2021-11-09 15:38               ` [PATCH v3 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan
                                 ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Huang Jianan @ 2021-11-09 15:38 UTC (permalink / raw)
  To: linux-erofs, xiang, chao; +Cc: yh, guoweichao, zhangshiming, guanyuwei


Add sysfs interface to configure erofs related parameters later.

Signed-off-by: Huang Jianan <huangjianan@oppo.com>
---
since v2:
- check whether t in erofs_attr_store is illegal
- print raw value for bool entry

since v1:
- Add sysfs API documentation.
- Use sysfs_emit over snprintf.

 Documentation/ABI/testing/sysfs-fs-erofs |   7 +
 fs/erofs/Makefile                        |   2 +-
 fs/erofs/internal.h                      |  10 +
 fs/erofs/super.c                         |  12 ++
 fs/erofs/sysfs.c                         | 240 +++++++++++++++++++++++
 5 files changed, 270 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-fs-erofs
 create mode 100644 fs/erofs/sysfs.c

diff --git a/Documentation/ABI/testing/sysfs-fs-erofs b/Documentation/ABI/testing/sysfs-fs-erofs
new file mode 100644
index 000000000000..86d0d0234473
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-fs-erofs
@@ -0,0 +1,7 @@
+What:		/sys/fs/erofs/features/
+Date:		November 2021
+Contact:	"Huang Jianan" <huangjianan@oppo.com>
+Description:	Shows all enabled kernel features.
+		Supported features:
+		lz4_0padding, compr_cfgs, big_pcluster, device_table,
+		sb_chksum.
diff --git a/fs/erofs/Makefile b/fs/erofs/Makefile
index 756fe2d65272..8a3317e38e5a 100644
--- a/fs/erofs/Makefile
+++ b/fs/erofs/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-$(CONFIG_EROFS_FS) += erofs.o
-erofs-objs := super.o inode.o data.o namei.o dir.o utils.o pcpubuf.o
+erofs-objs := super.o inode.o data.o namei.o dir.o utils.o pcpubuf.o sysfs.o
 erofs-$(CONFIG_EROFS_FS_XATTR) += xattr.o
 erofs-$(CONFIG_EROFS_FS_ZIP) += decompressor.o zmap.o zdata.o
 erofs-$(CONFIG_EROFS_FS_ZIP_LZMA) += decompressor_lzma.o
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 3265688af7f9..d0cd712dc222 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -134,6 +134,10 @@ struct erofs_sb_info {
 	u8 volume_name[16];             /* volume name */
 	u32 feature_compat;
 	u32 feature_incompat;
+
+	/* sysfs support */
+	struct kobject s_kobj;		/* /sys/fs/erofs/<devname> */
+	struct completion s_kobj_unregister;
 };
 
 #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info)
@@ -498,6 +502,12 @@ int erofs_pcpubuf_growsize(unsigned int nrpages);
 void erofs_pcpubuf_init(void);
 void erofs_pcpubuf_exit(void);
 
+/* sysfs.c */
+int erofs_register_sysfs(struct super_block *sb);
+void erofs_unregister_sysfs(struct super_block *sb);
+int __init erofs_init_sysfs(void);
+void erofs_exit_sysfs(void);
+
 /* utils.c / zdata.c */
 struct page *erofs_allocpage(struct page **pagepool, gfp_t gfp);
 static inline void erofs_pagepool_add(struct page **pagepool,
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 6a969b1e0ee6..abc1da5d1719 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -695,6 +695,10 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (err)
 		return err;
 
+	err = erofs_register_sysfs(sb);
+	if (err)
+		return err;
+
 	erofs_info(sb, "mounted with root inode @ nid %llu.", ROOT_NID(sbi));
 	return 0;
 }
@@ -808,6 +812,7 @@ static void erofs_put_super(struct super_block *sb)
 
 	DBG_BUGON(!sbi);
 
+	erofs_unregister_sysfs(sb);
 	erofs_shrinker_unregister(sb);
 #ifdef CONFIG_EROFS_FS_ZIP
 	iput(sbi->managed_cache);
@@ -852,6 +857,10 @@ static int __init erofs_module_init(void)
 	if (err)
 		goto zip_err;
 
+	err = erofs_init_sysfs();
+	if (err)
+		goto sysfs_err;
+
 	err = register_filesystem(&erofs_fs_type);
 	if (err)
 		goto fs_err;
@@ -859,6 +868,8 @@ static int __init erofs_module_init(void)
 	return 0;
 
 fs_err:
+	erofs_exit_sysfs();
+sysfs_err:
 	z_erofs_exit_zip_subsystem();
 zip_err:
 	z_erofs_lzma_exit();
@@ -877,6 +888,7 @@ static void __exit erofs_module_exit(void)
 	/* Ensure all RCU free inodes / pclusters are safe to be destroyed. */
 	rcu_barrier();
 
+	erofs_exit_sysfs();
 	z_erofs_exit_zip_subsystem();
 	z_erofs_lzma_exit();
 	erofs_exit_shrinker();
diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
new file mode 100644
index 000000000000..cf88e083eea5
--- /dev/null
+++ b/fs/erofs/sysfs.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C), 2008-2021, OPPO Mobile Comm Corp., Ltd.
+ *             https://www.oppo.com/
+ */
+#include <linux/sysfs.h>
+#include <linux/kobject.h>
+
+#include "internal.h"
+
+enum {
+	attr_feature,
+	attr_pointer_ui,
+	attr_pointer_bool,
+};
+
+enum {
+	struct_erofs_sb_info,
+};
+
+struct erofs_attr {
+	struct attribute attr;
+	short attr_id;
+	int struct_type;
+	int offset;
+};
+
+#define EROFS_ATTR(_name, _mode, _id)					\
+static struct erofs_attr erofs_attr_##_name = {				\
+	.attr = {.name = __stringify(_name), .mode = _mode },		\
+	.attr_id = attr_##_id,						\
+}
+#define EROFS_ATTR_FUNC(_name, _mode)	EROFS_ATTR(_name, _mode, _name)
+#define EROFS_ATTR_FEATURE(_name)	EROFS_ATTR(_name, 0444, feature)
+
+#define EROFS_ATTR_OFFSET(_name, _mode, _id, _struct)	\
+static struct erofs_attr erofs_attr_##_name = {			\
+	.attr = {.name = __stringify(_name), .mode = _mode },	\
+	.attr_id = attr_##_id,					\
+	.struct_type = struct_##_struct,			\
+	.offset = offsetof(struct _struct, _name),\
+}
+
+#define EROFS_ATTR_RW(_name, _id, _struct)	\
+	EROFS_ATTR_OFFSET(_name, 0644, _id, _struct)
+
+#define EROFS_RO_ATTR(_name, _id, _struct)	\
+	EROFS_ATTR_OFFSET(_name, 0444, _id, _struct)
+
+#define EROFS_ATTR_RW_UI(_name, _struct)	\
+	EROFS_ATTR_RW(_name, pointer_ui, _struct)
+
+#define EROFS_ATTR_RW_BOOL(_name, _struct)	\
+	EROFS_ATTR_RW(_name, pointer_bool, _struct)
+
+#define ATTR_LIST(name) (&erofs_attr_##name.attr)
+
+static struct attribute *erofs_attrs[] = {
+	NULL,
+};
+ATTRIBUTE_GROUPS(erofs);
+
+/* Features this copy of erofs supports */
+EROFS_ATTR_FEATURE(lz4_0padding);
+EROFS_ATTR_FEATURE(compr_cfgs);
+EROFS_ATTR_FEATURE(big_pcluster);
+EROFS_ATTR_FEATURE(device_table);
+EROFS_ATTR_FEATURE(sb_chksum);
+
+static struct attribute *erofs_feat_attrs[] = {
+	ATTR_LIST(lz4_0padding),
+	ATTR_LIST(compr_cfgs),
+	ATTR_LIST(big_pcluster),
+	ATTR_LIST(device_table),
+	ATTR_LIST(sb_chksum),
+	NULL,
+};
+ATTRIBUTE_GROUPS(erofs_feat);
+
+static unsigned char *__struct_ptr(struct erofs_sb_info *sbi,
+					  int struct_type, int offset)
+{
+	if (struct_type == struct_erofs_sb_info)
+		return (unsigned char *)sbi + offset;
+	return NULL;
+}
+
+static ssize_t erofs_attr_show(struct kobject *kobj,
+				struct attribute *attr, char *buf)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+						s_kobj);
+	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
+	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
+
+	switch (a->attr_id) {
+	case attr_feature:
+		return sysfs_emit(buf, "supported\n");
+	case attr_pointer_ui:
+		if (!ptr)
+			return 0;
+		return sysfs_emit(buf, "%u\n", *(unsigned int *)ptr);
+	case attr_pointer_bool:
+		if (!ptr)
+			return 0;
+		return sysfs_emit(buf, "%d\n", *(bool *)ptr);
+	}
+
+	return 0;
+}
+
+static ssize_t erofs_attr_store(struct kobject *kobj, struct attribute *attr,
+						const char *buf, size_t len)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+						s_kobj);
+	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
+	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
+	unsigned long t;
+	int ret;
+
+	switch (a->attr_id) {
+	case attr_pointer_ui:
+		if (!ptr)
+			return 0;
+		ret = kstrtoul(skip_spaces(buf), 0, &t);
+		if (ret)
+			return ret;
+		if (t > UINT_MAX)
+			return -EINVAL;
+		*(unsigned int *)ptr = t;
+		return len;
+	case attr_pointer_bool:
+		if (!ptr)
+			return 0;
+		ret = kstrtoul(skip_spaces(buf), 0, &t);
+		if (ret)
+			return ret;
+		if (t != 0 && t != 1)
+			return -EINVAL;
+		*(bool *)ptr = !!t;
+		return len;
+	}
+
+	return 0;
+}
+
+static void erofs_sb_release(struct kobject *kobj)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+						 s_kobj);
+	complete(&sbi->s_kobj_unregister);
+}
+
+static const struct sysfs_ops erofs_attr_ops = {
+	.show	= erofs_attr_show,
+	.store	= erofs_attr_store,
+};
+
+static struct kobj_type erofs_sb_ktype = {
+	.default_groups = erofs_groups,
+	.sysfs_ops	= &erofs_attr_ops,
+	.release	= erofs_sb_release,
+};
+
+static struct kobj_type erofs_ktype = {
+	.sysfs_ops	= &erofs_attr_ops,
+};
+
+static struct kset erofs_root = {
+	.kobj	= {.ktype = &erofs_ktype},
+};
+
+static struct kobj_type erofs_feat_ktype = {
+	.default_groups = erofs_feat_groups,
+	.sysfs_ops	= &erofs_attr_ops,
+};
+
+static struct kobject erofs_feat = {
+	.kset	= &erofs_root,
+};
+
+int erofs_register_sysfs(struct super_block *sb)
+{
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	int err;
+
+	sbi->s_kobj.kset = &erofs_root;
+	init_completion(&sbi->s_kobj_unregister);
+	err = kobject_init_and_add(&sbi->s_kobj, &erofs_sb_ktype, NULL,
+				   "%s", sb->s_id);
+	if (err)
+		goto put_sb_kobj;
+
+	return 0;
+
+put_sb_kobj:
+	kobject_put(&sbi->s_kobj);
+	wait_for_completion(&sbi->s_kobj_unregister);
+	return err;
+}
+
+void erofs_unregister_sysfs(struct super_block *sb)
+{
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+
+	kobject_del(&sbi->s_kobj);
+	kobject_put(&sbi->s_kobj);
+	wait_for_completion(&sbi->s_kobj_unregister);
+}
+
+int __init erofs_init_sysfs(void)
+{
+	int ret;
+
+	kobject_set_name(&erofs_root.kobj, "erofs");
+	erofs_root.kobj.parent = fs_kobj;
+	ret = kset_register(&erofs_root);
+	if (ret)
+		goto root_err;
+
+	ret = kobject_init_and_add(&erofs_feat, &erofs_feat_ktype,
+				   NULL, "features");
+	if (ret)
+		goto feat_err;
+
+	return ret;
+
+feat_err:
+	kobject_put(&erofs_feat);
+	kset_unregister(&erofs_root);
+root_err:
+	return ret;
+}
+
+void erofs_exit_sysfs(void)
+{
+	kobject_put(&erofs_feat);
+	kset_unregister(&erofs_root);
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 2/2] erofs: add sysfs node to control sync decompression strategy
  2021-11-09 15:38             ` [PATCH v3 1/2] erofs: add sysfs interface Huang Jianan
@ 2021-11-09 15:38               ` Huang Jianan
  2021-11-10  3:09               ` [PATCH v3 1/2] erofs: add sysfs interface Chao Yu
  2021-11-10 13:48               ` [PATCH v4 " Huang Jianan
  2 siblings, 0 replies; 20+ messages in thread
From: Huang Jianan @ 2021-11-09 15:38 UTC (permalink / raw)
  To: linux-erofs, xiang, chao; +Cc: yh, guoweichao, zhangshiming, guanyuwei

Although readpage is a synchronous path, there will be no additional
kworker scheduling overhead in non-atomic contexts. So add a sysfs
node to allow disable sync decompression.

Signed-off-by: Huang Jianan <huangjianan@oppo.com>
---
since v2:
- use enum to indicate sync decompression strategy
- add missing CONFIG_EROFS_FS_ZIP ifdef

since v1:
- leave auto default
- add a disable strategy for sync_decompress

 Documentation/ABI/testing/sysfs-fs-erofs |  9 +++++++++
 fs/erofs/internal.h                      | 10 ++++++++--
 fs/erofs/super.c                         |  2 +-
 fs/erofs/sysfs.c                         | 15 +++++++++++++++
 fs/erofs/zdata.c                         | 16 ++++++++++++----
 5 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-erofs b/Documentation/ABI/testing/sysfs-fs-erofs
index 86d0d0234473..d301704de79b 100644
--- a/Documentation/ABI/testing/sysfs-fs-erofs
+++ b/Documentation/ABI/testing/sysfs-fs-erofs
@@ -5,3 +5,12 @@ Description:	Shows all enabled kernel features.
 		Supported features:
 		lz4_0padding, compr_cfgs, big_pcluster, device_table,
 		sb_chksum.
+
+What:		/sys/fs/erofs/<disk>/sync_decompress
+Date:		November 2021
+Contact:	"Huang Jianan" <huangjianan@oppo.com>
+Description:	Control strategy of sync decompression
+		- 0 (defalut, auto): enable for readpage, and enable for
+				     readahead on atomic contexts only,
+		- 1 (force on): enable for readpage and readahead.
+		- 2 (force off): disable for all situations.
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index d0cd712dc222..cd30d4f0bada 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -56,12 +56,18 @@ struct erofs_device_info {
 	u32 mapped_blkaddr;
 };
 
+enum {
+	EROFS_SYNC_DECOMPRESS_AUTO,
+	EROFS_SYNC_DECOMPRESS_FORCE_ON,
+	EROFS_SYNC_DECOMPRESS_FORCE_OFF
+};
+
 struct erofs_mount_opts {
 #ifdef CONFIG_EROFS_FS_ZIP
 	/* current strategy of how to use managed cache */
 	unsigned char cache_strategy;
-	/* strategy of sync decompression (false - auto, true - force on) */
-	bool readahead_sync_decompress;
+	/* strategy of sync decompression (0 - auto, 1 - force on, 2 - force off) */
+	unsigned int sync_decompress;
 
 	/* threshold for decompression synchronously */
 	unsigned int max_sync_decompress_pages;
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index abc1da5d1719..58f381f80205 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -423,7 +423,7 @@ static void erofs_default_options(struct erofs_fs_context *ctx)
 #ifdef CONFIG_EROFS_FS_ZIP
 	ctx->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
 	ctx->opt.max_sync_decompress_pages = 3;
-	ctx->opt.readahead_sync_decompress = false;
+	ctx->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_AUTO;
 #endif
 #ifdef CONFIG_EROFS_FS_XATTR
 	set_opt(&ctx->opt, XATTR_USER);
diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
index cf88e083eea5..821a73857d82 100644
--- a/fs/erofs/sysfs.c
+++ b/fs/erofs/sysfs.c
@@ -16,6 +16,7 @@ enum {
 
 enum {
 	struct_erofs_sb_info,
+	struct_erofs_mount_opts,
 };
 
 struct erofs_attr {
@@ -55,7 +56,14 @@ static struct erofs_attr erofs_attr_##_name = {			\
 
 #define ATTR_LIST(name) (&erofs_attr_##name.attr)
 
+#ifdef CONFIG_EROFS_FS_ZIP
+EROFS_ATTR_RW_UI(sync_decompress, erofs_mount_opts);
+#endif
+
 static struct attribute *erofs_attrs[] = {
+#ifdef CONFIG_EROFS_FS_ZIP
+	ATTR_LIST(sync_decompress),
+#endif
 	NULL,
 };
 ATTRIBUTE_GROUPS(erofs);
@@ -82,6 +90,8 @@ static unsigned char *__struct_ptr(struct erofs_sb_info *sbi,
 {
 	if (struct_type == struct_erofs_sb_info)
 		return (unsigned char *)sbi + offset;
+	if (struct_type == struct_erofs_mount_opts)
+		return (unsigned char *)&sbi->opt + offset;
 	return NULL;
 }
 
@@ -128,6 +138,11 @@ static ssize_t erofs_attr_store(struct kobject *kobj, struct attribute *attr,
 			return ret;
 		if (t > UINT_MAX)
 			return -EINVAL;
+#ifdef CONFIG_EROFS_FS_ZIP
+		if (!strcmp(a->attr.name, "sync_decompress") &&
+		    (t > EROFS_SYNC_DECOMPRESS_FORCE_OFF))
+			return -EINVAL;
+#endif
 		*(unsigned int *)ptr = t;
 		return len;
 	case attr_pointer_bool:
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index bcb1b91b234f..4c0a9fdd09ff 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -794,7 +794,7 @@ static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
 	/* Use workqueue and sync decompression for atomic contexts only */
 	if (in_atomic() || irqs_disabled()) {
 		queue_work(z_erofs_workqueue, &io->u.work);
-		sbi->opt.readahead_sync_decompress = true;
+		sbi->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_FORCE_ON;
 		return;
 	}
 	z_erofs_decompressqueue_work(&io->u.work);
@@ -1454,9 +1454,11 @@ static void z_erofs_pcluster_readmore(struct z_erofs_decompress_frontend *f,
 static int z_erofs_readpage(struct file *file, struct page *page)
 {
 	struct inode *const inode = page->mapping->host;
+	struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
 	struct z_erofs_decompress_frontend f = DECOMPRESS_FRONTEND_INIT(inode);
 	struct page *pagepool = NULL;
 	int err;
+	bool force_fg = true;
 
 	trace_erofs_readpage(page, false);
 	f.headoffset = (erofs_off_t)page->index << PAGE_SHIFT;
@@ -1468,8 +1470,11 @@ static int z_erofs_readpage(struct file *file, struct page *page)
 
 	(void)z_erofs_collector_end(&f.clt);
 
+	if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_FORCE_OFF)
+		force_fg = false;
+
 	/* if some compressed cluster ready, need submit them anyway */
-	z_erofs_runqueue(inode->i_sb, &f, &pagepool, true);
+	z_erofs_runqueue(inode->i_sb, &f, &pagepool, force_fg);
 
 	if (err)
 		erofs_err(inode->i_sb, "failed to read, err [%d]", err);
@@ -1488,6 +1493,7 @@ static void z_erofs_readahead(struct readahead_control *rac)
 	struct z_erofs_decompress_frontend f = DECOMPRESS_FRONTEND_INIT(inode);
 	struct page *pagepool = NULL, *head = NULL, *page;
 	unsigned int nr_pages;
+	bool force_fg = false;
 
 	f.readahead = true;
 	f.headoffset = readahead_pos(rac);
@@ -1519,8 +1525,10 @@ static void z_erofs_readahead(struct readahead_control *rac)
 	z_erofs_pcluster_readmore(&f, rac, 0, &pagepool, false);
 	(void)z_erofs_collector_end(&f.clt);
 
-	z_erofs_runqueue(inode->i_sb, &f, &pagepool,
-			 sbi->opt.readahead_sync_decompress &&
+	if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_FORCE_ON)
+		force_fg = true;
+
+	z_erofs_runqueue(inode->i_sb, &f, &pagepool, force_fg &&
 			 nr_pages <= sbi->opt.max_sync_decompress_pages);
 	if (f.map.mpage)
 		put_page(f.map.mpage);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 1/2] erofs: add sysfs interface
  2021-11-09 15:38             ` [PATCH v3 1/2] erofs: add sysfs interface Huang Jianan
  2021-11-09 15:38               ` [PATCH v3 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan
@ 2021-11-10  3:09               ` Chao Yu
  2021-11-10 13:48               ` [PATCH v4 " Huang Jianan
  2 siblings, 0 replies; 20+ messages in thread
From: Chao Yu @ 2021-11-10  3:09 UTC (permalink / raw)
  To: Huang Jianan, linux-erofs, xiang; +Cc: yh, guoweichao, zhangshiming, guanyuwei

On 2021/11/9 23:38, Huang Jianan wrote:
> 
> Add sysfs interface to configure erofs related parameters later.
> 
> Signed-off-by: Huang Jianan <huangjianan@oppo.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 1/2] erofs: add sysfs interface
  2021-11-09 15:38             ` [PATCH v3 1/2] erofs: add sysfs interface Huang Jianan
  2021-11-09 15:38               ` [PATCH v3 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan
  2021-11-10  3:09               ` [PATCH v3 1/2] erofs: add sysfs interface Chao Yu
@ 2021-11-10 13:48               ` Huang Jianan
  2021-11-10 13:48                 ` [PATCH v4 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan
  2 siblings, 1 reply; 20+ messages in thread
From: Huang Jianan @ 2021-11-10 13:48 UTC (permalink / raw)
  To: linux-erofs, xiang, chao; +Cc: yh, guoweichao, zhangshiming, guanyuwei

From: Huang Jianan <huangjianan@oppo.com>

Add sysfs interface to configure erofs related parameters later.

Signed-off-by: Huang Jianan <huangjianan@oppo.com>
Reviewed-by: Chao Yu <chao@kernel.org>
---
since v3:
- add description of sysfs in erofs documentation.

since v2:
- check whether t in erofs_attr_store is illegal.
- print raw value for bool entry.

since v1:
- Add sysfs API documentation.
- Use sysfs_emit over snprintf.

 Documentation/ABI/testing/sysfs-fs-erofs |   7 +
 Documentation/filesystems/erofs.rst      |   8 +
 fs/erofs/Makefile                        |   2 +-
 fs/erofs/internal.h                      |  10 +
 fs/erofs/super.c                         |  12 ++
 fs/erofs/sysfs.c                         | 240 +++++++++++++++++++++++
 6 files changed, 278 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-fs-erofs
 create mode 100644 fs/erofs/sysfs.c

diff --git a/Documentation/ABI/testing/sysfs-fs-erofs b/Documentation/ABI/testing/sysfs-fs-erofs
new file mode 100644
index 000000000000..86d0d0234473
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-fs-erofs
@@ -0,0 +1,7 @@
+What:		/sys/fs/erofs/features/
+Date:		November 2021
+Contact:	"Huang Jianan" <huangjianan@oppo.com>
+Description:	Shows all enabled kernel features.
+		Supported features:
+		lz4_0padding, compr_cfgs, big_pcluster, device_table,
+		sb_chksum.
diff --git a/Documentation/filesystems/erofs.rst b/Documentation/filesystems/erofs.rst
index 01df283c7d04..7119aa213be7 100644
--- a/Documentation/filesystems/erofs.rst
+++ b/Documentation/filesystems/erofs.rst
@@ -93,6 +93,14 @@ dax                    A legacy option which is an alias for ``dax=always``.
 device=%s              Specify a path to an extra device to be used together.
 ===================    =========================================================
 
+Sysfs Entries
+=============
+
+Information about mounted erofs file systems can be found in /sys/fs/erofs.
+Each mounted filesystem will have a directory in /sys/fs/erofs based on its
+device name (i.e., /sys/fs/erofs/sda).
+(see also Documentation/ABI/testing/sysfs-fs-erofs)
+
 On-disk details
 ===============
 
diff --git a/fs/erofs/Makefile b/fs/erofs/Makefile
index 756fe2d65272..8a3317e38e5a 100644
--- a/fs/erofs/Makefile
+++ b/fs/erofs/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-$(CONFIG_EROFS_FS) += erofs.o
-erofs-objs := super.o inode.o data.o namei.o dir.o utils.o pcpubuf.o
+erofs-objs := super.o inode.o data.o namei.o dir.o utils.o pcpubuf.o sysfs.o
 erofs-$(CONFIG_EROFS_FS_XATTR) += xattr.o
 erofs-$(CONFIG_EROFS_FS_ZIP) += decompressor.o zmap.o zdata.o
 erofs-$(CONFIG_EROFS_FS_ZIP_LZMA) += decompressor_lzma.o
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 3265688af7f9..d0cd712dc222 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -134,6 +134,10 @@ struct erofs_sb_info {
 	u8 volume_name[16];             /* volume name */
 	u32 feature_compat;
 	u32 feature_incompat;
+
+	/* sysfs support */
+	struct kobject s_kobj;		/* /sys/fs/erofs/<devname> */
+	struct completion s_kobj_unregister;
 };
 
 #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info)
@@ -498,6 +502,12 @@ int erofs_pcpubuf_growsize(unsigned int nrpages);
 void erofs_pcpubuf_init(void);
 void erofs_pcpubuf_exit(void);
 
+/* sysfs.c */
+int erofs_register_sysfs(struct super_block *sb);
+void erofs_unregister_sysfs(struct super_block *sb);
+int __init erofs_init_sysfs(void);
+void erofs_exit_sysfs(void);
+
 /* utils.c / zdata.c */
 struct page *erofs_allocpage(struct page **pagepool, gfp_t gfp);
 static inline void erofs_pagepool_add(struct page **pagepool,
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 6a969b1e0ee6..abc1da5d1719 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -695,6 +695,10 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (err)
 		return err;
 
+	err = erofs_register_sysfs(sb);
+	if (err)
+		return err;
+
 	erofs_info(sb, "mounted with root inode @ nid %llu.", ROOT_NID(sbi));
 	return 0;
 }
@@ -808,6 +812,7 @@ static void erofs_put_super(struct super_block *sb)
 
 	DBG_BUGON(!sbi);
 
+	erofs_unregister_sysfs(sb);
 	erofs_shrinker_unregister(sb);
 #ifdef CONFIG_EROFS_FS_ZIP
 	iput(sbi->managed_cache);
@@ -852,6 +857,10 @@ static int __init erofs_module_init(void)
 	if (err)
 		goto zip_err;
 
+	err = erofs_init_sysfs();
+	if (err)
+		goto sysfs_err;
+
 	err = register_filesystem(&erofs_fs_type);
 	if (err)
 		goto fs_err;
@@ -859,6 +868,8 @@ static int __init erofs_module_init(void)
 	return 0;
 
 fs_err:
+	erofs_exit_sysfs();
+sysfs_err:
 	z_erofs_exit_zip_subsystem();
 zip_err:
 	z_erofs_lzma_exit();
@@ -877,6 +888,7 @@ static void __exit erofs_module_exit(void)
 	/* Ensure all RCU free inodes / pclusters are safe to be destroyed. */
 	rcu_barrier();
 
+	erofs_exit_sysfs();
 	z_erofs_exit_zip_subsystem();
 	z_erofs_lzma_exit();
 	erofs_exit_shrinker();
diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
new file mode 100644
index 000000000000..cf88e083eea5
--- /dev/null
+++ b/fs/erofs/sysfs.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C), 2008-2021, OPPO Mobile Comm Corp., Ltd.
+ *             https://www.oppo.com/
+ */
+#include <linux/sysfs.h>
+#include <linux/kobject.h>
+
+#include "internal.h"
+
+enum {
+	attr_feature,
+	attr_pointer_ui,
+	attr_pointer_bool,
+};
+
+enum {
+	struct_erofs_sb_info,
+};
+
+struct erofs_attr {
+	struct attribute attr;
+	short attr_id;
+	int struct_type;
+	int offset;
+};
+
+#define EROFS_ATTR(_name, _mode, _id)					\
+static struct erofs_attr erofs_attr_##_name = {				\
+	.attr = {.name = __stringify(_name), .mode = _mode },		\
+	.attr_id = attr_##_id,						\
+}
+#define EROFS_ATTR_FUNC(_name, _mode)	EROFS_ATTR(_name, _mode, _name)
+#define EROFS_ATTR_FEATURE(_name)	EROFS_ATTR(_name, 0444, feature)
+
+#define EROFS_ATTR_OFFSET(_name, _mode, _id, _struct)	\
+static struct erofs_attr erofs_attr_##_name = {			\
+	.attr = {.name = __stringify(_name), .mode = _mode },	\
+	.attr_id = attr_##_id,					\
+	.struct_type = struct_##_struct,			\
+	.offset = offsetof(struct _struct, _name),\
+}
+
+#define EROFS_ATTR_RW(_name, _id, _struct)	\
+	EROFS_ATTR_OFFSET(_name, 0644, _id, _struct)
+
+#define EROFS_RO_ATTR(_name, _id, _struct)	\
+	EROFS_ATTR_OFFSET(_name, 0444, _id, _struct)
+
+#define EROFS_ATTR_RW_UI(_name, _struct)	\
+	EROFS_ATTR_RW(_name, pointer_ui, _struct)
+
+#define EROFS_ATTR_RW_BOOL(_name, _struct)	\
+	EROFS_ATTR_RW(_name, pointer_bool, _struct)
+
+#define ATTR_LIST(name) (&erofs_attr_##name.attr)
+
+static struct attribute *erofs_attrs[] = {
+	NULL,
+};
+ATTRIBUTE_GROUPS(erofs);
+
+/* Features this copy of erofs supports */
+EROFS_ATTR_FEATURE(lz4_0padding);
+EROFS_ATTR_FEATURE(compr_cfgs);
+EROFS_ATTR_FEATURE(big_pcluster);
+EROFS_ATTR_FEATURE(device_table);
+EROFS_ATTR_FEATURE(sb_chksum);
+
+static struct attribute *erofs_feat_attrs[] = {
+	ATTR_LIST(lz4_0padding),
+	ATTR_LIST(compr_cfgs),
+	ATTR_LIST(big_pcluster),
+	ATTR_LIST(device_table),
+	ATTR_LIST(sb_chksum),
+	NULL,
+};
+ATTRIBUTE_GROUPS(erofs_feat);
+
+static unsigned char *__struct_ptr(struct erofs_sb_info *sbi,
+					  int struct_type, int offset)
+{
+	if (struct_type == struct_erofs_sb_info)
+		return (unsigned char *)sbi + offset;
+	return NULL;
+}
+
+static ssize_t erofs_attr_show(struct kobject *kobj,
+				struct attribute *attr, char *buf)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+						s_kobj);
+	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
+	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
+
+	switch (a->attr_id) {
+	case attr_feature:
+		return sysfs_emit(buf, "supported\n");
+	case attr_pointer_ui:
+		if (!ptr)
+			return 0;
+		return sysfs_emit(buf, "%u\n", *(unsigned int *)ptr);
+	case attr_pointer_bool:
+		if (!ptr)
+			return 0;
+		return sysfs_emit(buf, "%d\n", *(bool *)ptr);
+	}
+
+	return 0;
+}
+
+static ssize_t erofs_attr_store(struct kobject *kobj, struct attribute *attr,
+						const char *buf, size_t len)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+						s_kobj);
+	struct erofs_attr *a = container_of(attr, struct erofs_attr, attr);
+	unsigned char *ptr = __struct_ptr(sbi, a->struct_type, a->offset);
+	unsigned long t;
+	int ret;
+
+	switch (a->attr_id) {
+	case attr_pointer_ui:
+		if (!ptr)
+			return 0;
+		ret = kstrtoul(skip_spaces(buf), 0, &t);
+		if (ret)
+			return ret;
+		if (t > UINT_MAX)
+			return -EINVAL;
+		*(unsigned int *)ptr = t;
+		return len;
+	case attr_pointer_bool:
+		if (!ptr)
+			return 0;
+		ret = kstrtoul(skip_spaces(buf), 0, &t);
+		if (ret)
+			return ret;
+		if (t != 0 && t != 1)
+			return -EINVAL;
+		*(bool *)ptr = !!t;
+		return len;
+	}
+
+	return 0;
+}
+
+static void erofs_sb_release(struct kobject *kobj)
+{
+	struct erofs_sb_info *sbi = container_of(kobj, struct erofs_sb_info,
+						 s_kobj);
+	complete(&sbi->s_kobj_unregister);
+}
+
+static const struct sysfs_ops erofs_attr_ops = {
+	.show	= erofs_attr_show,
+	.store	= erofs_attr_store,
+};
+
+static struct kobj_type erofs_sb_ktype = {
+	.default_groups = erofs_groups,
+	.sysfs_ops	= &erofs_attr_ops,
+	.release	= erofs_sb_release,
+};
+
+static struct kobj_type erofs_ktype = {
+	.sysfs_ops	= &erofs_attr_ops,
+};
+
+static struct kset erofs_root = {
+	.kobj	= {.ktype = &erofs_ktype},
+};
+
+static struct kobj_type erofs_feat_ktype = {
+	.default_groups = erofs_feat_groups,
+	.sysfs_ops	= &erofs_attr_ops,
+};
+
+static struct kobject erofs_feat = {
+	.kset	= &erofs_root,
+};
+
+int erofs_register_sysfs(struct super_block *sb)
+{
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	int err;
+
+	sbi->s_kobj.kset = &erofs_root;
+	init_completion(&sbi->s_kobj_unregister);
+	err = kobject_init_and_add(&sbi->s_kobj, &erofs_sb_ktype, NULL,
+				   "%s", sb->s_id);
+	if (err)
+		goto put_sb_kobj;
+
+	return 0;
+
+put_sb_kobj:
+	kobject_put(&sbi->s_kobj);
+	wait_for_completion(&sbi->s_kobj_unregister);
+	return err;
+}
+
+void erofs_unregister_sysfs(struct super_block *sb)
+{
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+
+	kobject_del(&sbi->s_kobj);
+	kobject_put(&sbi->s_kobj);
+	wait_for_completion(&sbi->s_kobj_unregister);
+}
+
+int __init erofs_init_sysfs(void)
+{
+	int ret;
+
+	kobject_set_name(&erofs_root.kobj, "erofs");
+	erofs_root.kobj.parent = fs_kobj;
+	ret = kset_register(&erofs_root);
+	if (ret)
+		goto root_err;
+
+	ret = kobject_init_and_add(&erofs_feat, &erofs_feat_ktype,
+				   NULL, "features");
+	if (ret)
+		goto feat_err;
+
+	return ret;
+
+feat_err:
+	kobject_put(&erofs_feat);
+	kset_unregister(&erofs_root);
+root_err:
+	return ret;
+}
+
+void erofs_exit_sysfs(void)
+{
+	kobject_put(&erofs_feat);
+	kset_unregister(&erofs_root);
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 2/2] erofs: add sysfs node to control sync decompression strategy
  2021-11-10 13:48               ` [PATCH v4 " Huang Jianan
@ 2021-11-10 13:48                 ` Huang Jianan
  2021-11-11  2:15                   ` Chao Yu
  0 siblings, 1 reply; 20+ messages in thread
From: Huang Jianan @ 2021-11-10 13:48 UTC (permalink / raw)
  To: linux-erofs, xiang, chao; +Cc: yh, guoweichao, zhangshiming, guanyuwei

From: Huang Jianan <huangjianan@oppo.com>

Although readpage is a synchronous path, there will be no additional
kworker scheduling overhead in non-atomic contexts. So add a sysfs
node to allow disable sync decompression.

Signed-off-by: Huang Jianan <huangjianan@oppo.com>
---
since v3:
- Clean up the sync decompressstrategy into a separate function.

since v2:
- Use enum to indicate sync decompression strategy.
- Add missing CONFIG_EROFS_FS_ZIP ifdef.

since v1:
- Leave auto default.
- Add a disable strategy for sync_decompress.

 Documentation/ABI/testing/sysfs-fs-erofs |  9 ++++++++
 fs/erofs/internal.h                      | 10 ++++++--
 fs/erofs/super.c                         |  2 +-
 fs/erofs/sysfs.c                         | 15 ++++++++++++
 fs/erofs/zdata.c                         | 29 ++++++++++++++++++++----
 5 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-erofs b/Documentation/ABI/testing/sysfs-fs-erofs
index 86d0d0234473..d301704de79b 100644
--- a/Documentation/ABI/testing/sysfs-fs-erofs
+++ b/Documentation/ABI/testing/sysfs-fs-erofs
@@ -5,3 +5,12 @@ Description:	Shows all enabled kernel features.
 		Supported features:
 		lz4_0padding, compr_cfgs, big_pcluster, device_table,
 		sb_chksum.
+
+What:		/sys/fs/erofs/<disk>/sync_decompress
+Date:		November 2021
+Contact:	"Huang Jianan" <huangjianan@oppo.com>
+Description:	Control strategy of sync decompression
+		- 0 (defalut, auto): enable for readpage, and enable for
+				     readahead on atomic contexts only,
+		- 1 (force on): enable for readpage and readahead.
+		- 2 (force off): disable for all situations.
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index d0cd712dc222..cd30d4f0bada 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -56,12 +56,18 @@ struct erofs_device_info {
 	u32 mapped_blkaddr;
 };
 
+enum {
+	EROFS_SYNC_DECOMPRESS_AUTO,
+	EROFS_SYNC_DECOMPRESS_FORCE_ON,
+	EROFS_SYNC_DECOMPRESS_FORCE_OFF
+};
+
 struct erofs_mount_opts {
 #ifdef CONFIG_EROFS_FS_ZIP
 	/* current strategy of how to use managed cache */
 	unsigned char cache_strategy;
-	/* strategy of sync decompression (false - auto, true - force on) */
-	bool readahead_sync_decompress;
+	/* strategy of sync decompression (0 - auto, 1 - force on, 2 - force off) */
+	unsigned int sync_decompress;
 
 	/* threshold for decompression synchronously */
 	unsigned int max_sync_decompress_pages;
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index abc1da5d1719..58f381f80205 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -423,7 +423,7 @@ static void erofs_default_options(struct erofs_fs_context *ctx)
 #ifdef CONFIG_EROFS_FS_ZIP
 	ctx->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
 	ctx->opt.max_sync_decompress_pages = 3;
-	ctx->opt.readahead_sync_decompress = false;
+	ctx->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_AUTO;
 #endif
 #ifdef CONFIG_EROFS_FS_XATTR
 	set_opt(&ctx->opt, XATTR_USER);
diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
index cf88e083eea5..821a73857d82 100644
--- a/fs/erofs/sysfs.c
+++ b/fs/erofs/sysfs.c
@@ -16,6 +16,7 @@ enum {
 
 enum {
 	struct_erofs_sb_info,
+	struct_erofs_mount_opts,
 };
 
 struct erofs_attr {
@@ -55,7 +56,14 @@ static struct erofs_attr erofs_attr_##_name = {			\
 
 #define ATTR_LIST(name) (&erofs_attr_##name.attr)
 
+#ifdef CONFIG_EROFS_FS_ZIP
+EROFS_ATTR_RW_UI(sync_decompress, erofs_mount_opts);
+#endif
+
 static struct attribute *erofs_attrs[] = {
+#ifdef CONFIG_EROFS_FS_ZIP
+	ATTR_LIST(sync_decompress),
+#endif
 	NULL,
 };
 ATTRIBUTE_GROUPS(erofs);
@@ -82,6 +90,8 @@ static unsigned char *__struct_ptr(struct erofs_sb_info *sbi,
 {
 	if (struct_type == struct_erofs_sb_info)
 		return (unsigned char *)sbi + offset;
+	if (struct_type == struct_erofs_mount_opts)
+		return (unsigned char *)&sbi->opt + offset;
 	return NULL;
 }
 
@@ -128,6 +138,11 @@ static ssize_t erofs_attr_store(struct kobject *kobj, struct attribute *attr,
 			return ret;
 		if (t > UINT_MAX)
 			return -EINVAL;
+#ifdef CONFIG_EROFS_FS_ZIP
+		if (!strcmp(a->attr.name, "sync_decompress") &&
+		    (t > EROFS_SYNC_DECOMPRESS_FORCE_OFF))
+			return -EINVAL;
+#endif
 		*(unsigned int *)ptr = t;
 		return len;
 	case attr_pointer_bool:
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index bcb1b91b234f..233c8a047c53 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -772,6 +772,26 @@ static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe,
 	goto out;
 }
 
+static void set_sync_decompress_policy(struct erofs_sb_info *sbi)
+{
+	/* enable sync decompression in readahead for atomic contexts */
+	if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_AUTO)
+		sbi->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_FORCE_ON;
+}
+
+static bool get_sync_decompress_policy(struct erofs_sb_info *sbi,
+				       unsigned int readahead_pages)
+{
+	/* auto: enable for readpage, disable for readahead */
+	if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_AUTO)
+		return readahead_pages == 0;
+
+	if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_FORCE_ON)
+		return readahead_pages <= sbi->opt.max_sync_decompress_pages;
+
+	return false;
+}
+
 static void z_erofs_decompressqueue_work(struct work_struct *work);
 static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
 				       bool sync, int bios)
@@ -794,7 +814,7 @@ static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
 	/* Use workqueue and sync decompression for atomic contexts only */
 	if (in_atomic() || irqs_disabled()) {
 		queue_work(z_erofs_workqueue, &io->u.work);
-		sbi->opt.readahead_sync_decompress = true;
+		set_sync_decompress_policy(sbi);
 		return;
 	}
 	z_erofs_decompressqueue_work(&io->u.work);
@@ -1454,6 +1474,7 @@ static void z_erofs_pcluster_readmore(struct z_erofs_decompress_frontend *f,
 static int z_erofs_readpage(struct file *file, struct page *page)
 {
 	struct inode *const inode = page->mapping->host;
+	struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
 	struct z_erofs_decompress_frontend f = DECOMPRESS_FRONTEND_INIT(inode);
 	struct page *pagepool = NULL;
 	int err;
@@ -1469,7 +1490,8 @@ static int z_erofs_readpage(struct file *file, struct page *page)
 	(void)z_erofs_collector_end(&f.clt);
 
 	/* if some compressed cluster ready, need submit them anyway */
-	z_erofs_runqueue(inode->i_sb, &f, &pagepool, true);
+	z_erofs_runqueue(inode->i_sb, &f, &pagepool,
+			 get_sync_decompress_policy(sbi, 0));
 
 	if (err)
 		erofs_err(inode->i_sb, "failed to read, err [%d]", err);
@@ -1520,8 +1542,7 @@ static void z_erofs_readahead(struct readahead_control *rac)
 	(void)z_erofs_collector_end(&f.clt);
 
 	z_erofs_runqueue(inode->i_sb, &f, &pagepool,
-			 sbi->opt.readahead_sync_decompress &&
-			 nr_pages <= sbi->opt.max_sync_decompress_pages);
+			 get_sync_decompress_policy(sbi, nr_pages));
 	if (f.map.mpage)
 		put_page(f.map.mpage);
 	erofs_release_pages(&pagepool);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 2/2] erofs: add sysfs node to control sync decompression strategy
  2021-11-10 13:48                 ` [PATCH v4 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan
@ 2021-11-11  2:15                   ` Chao Yu
  0 siblings, 0 replies; 20+ messages in thread
From: Chao Yu @ 2021-11-11  2:15 UTC (permalink / raw)
  To: Huang Jianan, linux-erofs, xiang; +Cc: yh, guoweichao, zhangshiming, guanyuwei

On 2021/11/10 21:48, Huang Jianan wrote:
> From: Huang Jianan <huangjianan@oppo.com>
> 
> Although readpage is a synchronous path, there will be no additional
> kworker scheduling overhead in non-atomic contexts. So add a sysfs
> node to allow disable sync decompression.
> 
> Signed-off-by: Huang Jianan <huangjianan@oppo.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy
  2021-11-09  2:54 ` [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan via Linux-erofs
  2021-11-09  3:16   ` Gao Xiang
@ 2021-11-11 20:17   ` kernel test robot
  2021-11-12  2:08     ` Huang Jianan via Linux-erofs
  1 sibling, 1 reply; 20+ messages in thread
From: kernel test robot @ 2021-11-11 20:17 UTC (permalink / raw)
  To: Huang Jianan, linux-erofs
  Cc: zhangshiming, kbuild-all, linux-kernel, yh, guanyuwei, guoweichao

[-- Attachment #1: Type: text/plain, Size: 3068 bytes --]

Hi Huang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on xiang-erofs/dev-test]
[also build test ERROR on next-20211111]
[cannot apply to v5.15]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Huang-Jianan/erofs-add-sysfs-interface/20211109-105542
base:   https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git dev-test
config: xtensa-buildonly-randconfig-r004-20211111 (attached as .config)
compiler: xtensa-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/d6bf9edf69e87ee0a9795421ff2e1a9b69a29ce8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Huang-Jianan/erofs-add-sysfs-interface/20211109-105542
        git checkout d6bf9edf69e87ee0a9795421ff2e1a9b69a29ce8
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=xtensa SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from <command-line>:
>> include/linux/compiler_types.h:140:41: error: 'struct erofs_mount_opts' has no member named 'readahead_sync_decompress'
     140 | #define __compiler_offsetof(a, b)       __builtin_offsetof(a, b)
         |                                         ^~~~~~~~~~~~~~~~~~
   include/linux/stddef.h:17:33: note: in expansion of macro '__compiler_offsetof'
      17 | #define offsetof(TYPE, MEMBER)  __compiler_offsetof(TYPE, MEMBER)
         |                                 ^~~~~~~~~~~~~~~~~~~
   fs/erofs/sysfs.c:42:19: note: in expansion of macro 'offsetof'
      42 |         .offset = offsetof(struct _struct, _name),\
         |                   ^~~~~~~~
   fs/erofs/sysfs.c:46:9: note: in expansion of macro 'EROFS_ATTR_OFFSET'
      46 |         EROFS_ATTR_OFFSET(_name, 0644, _id, _struct)
         |         ^~~~~~~~~~~~~~~~~
   fs/erofs/sysfs.c:55:9: note: in expansion of macro 'EROFS_RW_ATTR'
      55 |         EROFS_RW_ATTR(_name, pointer_bool, _struct)
         |         ^~~~~~~~~~~~~
   fs/erofs/sysfs.c:59:1: note: in expansion of macro 'EROFS_RW_ATTR_BOOL'
      59 | EROFS_RW_ATTR_BOOL(readahead_sync_decompress, erofs_mount_opts);
         | ^~~~~~~~~~~~~~~~~~


vim +140 include/linux/compiler_types.h

71391bdd2e9aab Xiaozhou Liu 2018-12-14  139  
71391bdd2e9aab Xiaozhou Liu 2018-12-14 @140  #define __compiler_offsetof(a, b)	__builtin_offsetof(a, b)
71391bdd2e9aab Xiaozhou Liu 2018-12-14  141  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38002 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy
  2021-11-11 20:17   ` [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy kernel test robot
@ 2021-11-12  2:08     ` Huang Jianan via Linux-erofs
  0 siblings, 0 replies; 20+ messages in thread
From: Huang Jianan via Linux-erofs @ 2021-11-12  2:08 UTC (permalink / raw)
  To: kernel test robot, linux-erofs
  Cc: zhangshiming, kbuild-all, linux-kernel, yh, guanyuwei, guoweichao

在 2021/11/12 4:17, kernel test robot 写道:
> Hi Huang,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on xiang-erofs/dev-test]
> [also build test ERROR on next-20211111]
> [cannot apply to v5.15]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit-scm.com%2Fdocs%2Fgit-format-patch&amp;data=04%7C01%7Chuangjianan%40oppo.com%7C0d3d672ac3e94a90cf9108d9a5505aae%7Cf1905eb1c35341c5951662b4a54b5ee6%7C0%7C0%7C637722589678393057%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=OVZ%2BPtlKYw1zTA3lxF2mFAqcHDs6FjweHQpTpS%2B6uog%3D&amp;reserved=0]
>
> url:    https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommits%2FHuang-Jianan%2Ferofs-add-sysfs-interface%2F20211109-105542&amp;data=04%7C01%7Chuangjianan%40oppo.com%7C0d3d672ac3e94a90cf9108d9a5505aae%7Cf1905eb1c35341c5951662b4a54b5ee6%7C0%7C0%7C637722589678403052%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=vvNfYf7RfWT813%2FG%2FOUvTPLOyYqlw1FXQZXjPuqoqsk%3D&amp;reserved=0
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git dev-test
> config: xtensa-buildonly-randconfig-r004-20211111 (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 11.2.0
> reproduce (this is a W=1 build):
>          wget https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fintel%2Flkp-tests%2Fmaster%2Fsbin%2Fmake.cross&amp;data=04%7C01%7Chuangjianan%40oppo.com%7C0d3d672ac3e94a90cf9108d9a5505aae%7Cf1905eb1c35341c5951662b4a54b5ee6%7C0%7C0%7C637722589678403052%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=SIkgxAB9wqCI7O0OjvYinNEpFtSD7vUO2YX%2FwQFC480%3D&amp;reserved=0 -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommit%2Fd6bf9edf69e87ee0a9795421ff2e1a9b69a29ce8&amp;data=04%7C01%7Chuangjianan%40oppo.com%7C0d3d672ac3e94a90cf9108d9a5505aae%7Cf1905eb1c35341c5951662b4a54b5ee6%7C0%7C0%7C637722589678403052%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=%2BHaZRNBm3bV0VbSMU0vlz6pokoI3tA8kNpVxCGF1wtc%3D&amp;reserved=0
>          git remote add linux-review https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux&amp;data=04%7C01%7Chuangjianan%40oppo.com%7C0d3d672ac3e94a90cf9108d9a5505aae%7Cf1905eb1c35341c5951662b4a54b5ee6%7C0%7C0%7C637722589678403052%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ICXqRmM2WczvCZQYR9hBUVQddexNDPdhSmr80YLCueY%3D&amp;reserved=0
>          git fetch --no-tags linux-review Huang-Jianan/erofs-add-sysfs-interface/20211109-105542
>          git checkout d6bf9edf69e87ee0a9795421ff2e1a9b69a29ce8
>          # save the attached .config to linux build tree
>          mkdir build_dir
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=xtensa SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>     In file included from <command-line>:
>>> include/linux/compiler_types.h:140:41: error: 'struct erofs_mount_opts' has no member named 'readahead_sync_decompress'
This problem has been fixed by adding macros in v2.
I will resend a final version of the patch in a clean chain later. 😁

Thanks,
Jianan
>       140 | #define __compiler_offsetof(a, b)       __builtin_offsetof(a, b)
>           |                                         ^~~~~~~~~~~~~~~~~~
>     include/linux/stddef.h:17:33: note: in expansion of macro '__compiler_offsetof'
>        17 | #define offsetof(TYPE, MEMBER)  __compiler_offsetof(TYPE, MEMBER)
>           |                                 ^~~~~~~~~~~~~~~~~~~
>     fs/erofs/sysfs.c:42:19: note: in expansion of macro 'offsetof'
>        42 |         .offset = offsetof(struct _struct, _name),\
>           |                   ^~~~~~~~
>     fs/erofs/sysfs.c:46:9: note: in expansion of macro 'EROFS_ATTR_OFFSET'
>        46 |         EROFS_ATTR_OFFSET(_name, 0644, _id, _struct)
>           |         ^~~~~~~~~~~~~~~~~
>     fs/erofs/sysfs.c:55:9: note: in expansion of macro 'EROFS_RW_ATTR'
>        55 |         EROFS_RW_ATTR(_name, pointer_bool, _struct)
>           |         ^~~~~~~~~~~~~
>     fs/erofs/sysfs.c:59:1: note: in expansion of macro 'EROFS_RW_ATTR_BOOL'
>        59 | EROFS_RW_ATTR_BOOL(readahead_sync_decompress, erofs_mount_opts);
>           | ^~~~~~~~~~~~~~~~~~
>
>
> vim +140 include/linux/compiler_types.h
>
> 71391bdd2e9aab Xiaozhou Liu 2018-12-14  139
> 71391bdd2e9aab Xiaozhou Liu 2018-12-14 @140  #define __compiler_offsetof(a, b)	__builtin_offsetof(a, b)
> 71391bdd2e9aab Xiaozhou Liu 2018-12-14  141
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01.org%2Fhyperkitty%2Flist%2Fkbuild-all%40lists.01.org&amp;data=04%7C01%7Chuangjianan%40oppo.com%7C0d3d672ac3e94a90cf9108d9a5505aae%7Cf1905eb1c35341c5951662b4a54b5ee6%7C0%7C0%7C637722589678403052%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=I8XIGfml9qKANF9lYjvZ1eRcBGmbE%2BPDSsaajTOgfXo%3D&amp;reserved=0


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-11-12  2:09 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-09  2:54 [PATCH 1/2] erofs: add sysfs interface Huang Jianan via Linux-erofs
2021-11-09  2:54 ` [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan via Linux-erofs
2021-11-09  3:16   ` Gao Xiang
2021-11-09  7:43     ` Huang Jianan via Linux-erofs
2021-11-09  7:45       ` [PATCH v2 1/2] erofs: add sysfs interface Huang Jianan via Linux-erofs
2021-11-09  7:45         ` [PATCH v2 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan via Linux-erofs
2021-11-09 13:27           ` Chao Yu
2021-11-09 15:38             ` [PATCH v3 1/2] erofs: add sysfs interface Huang Jianan
2021-11-09 15:38               ` [PATCH v3 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan
2021-11-10  3:09               ` [PATCH v3 1/2] erofs: add sysfs interface Chao Yu
2021-11-10 13:48               ` [PATCH v4 " Huang Jianan
2021-11-10 13:48                 ` [PATCH v4 2/2] erofs: add sysfs node to control sync decompression strategy Huang Jianan
2021-11-11  2:15                   ` Chao Yu
2021-11-09 13:14         ` [PATCH v2 1/2] erofs: add sysfs interface Chao Yu
2021-11-11 20:17   ` [PATCH 2/2] erofs: add sysfs node to control sync decompression strategy kernel test robot
2021-11-12  2:08     ` Huang Jianan via Linux-erofs
2021-11-09  3:10 ` [PATCH 1/2] erofs: add sysfs interface Gao Xiang
2021-11-09  7:29   ` Huang Jianan via Linux-erofs
2021-11-09  3:14 ` Joe Perches
2021-11-09  7:31   ` Huang Jianan via Linux-erofs

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).