[PATCH V3 0/6] Introduce erofs shared domain

[PATCH V3 0/6] Introduce erofs shared domain

[Jia Zhu]

Changes since V2:
1. Some code cleanups:
   1.1 Optimize the input parameters and return values of some functions.
   1.2 Only reserve a pair of function declarations for fscache related codes.
   1.3 Remove useless null check.
   1.4 Replace some ternary operators to make the code more intuitive.
2. Increase the granularity of @domain->mutex to a global lock.
3. Adjust patchset structure and order.

[Kernel Patchset]
===============
Git tree:
	https://github.com/userzj/linux.git zhujia/shared-domain-v3
Git web:
	https://github.com/userzj/linux/tree/zhujia/shared-domain-v3

[User Daemon for Quick Test]
============================
Git web:
	https://github.com/userzj/demand-read-cachefilesd/tree/shared-domain
More test cases will be added to:
	https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/log/?h=experimental-tests-fscache 

[E2E Container Demo for Quick Test]
===================================
[Issue]
	https://github.com/containerd/nydus-snapshotter/issues/161
[PR]
	https://github.com/containerd/nydus-snapshotter/pull/162

[Background]
============
In ondemand read mode, we use individual volume to present an erofs
mountpoint, cookies to present bootstrap and data blobs.

In which case, since cookies can't be shared between fscache volumes,
even if the data blobs between different mountpoints are exactly same,
they can't be shared.

[Introduction]
==============
Here we introduce erofs shared domain to resolve above mentioned case.
Several erofs filesystems can belong to one domain, and data blobs can
be shared among these erofs filesystems of same domain.

[Usage]
Users could specify 'domain_id' mount option to create or join into a
domain which reuses the same cookies(blobs).

[Design]
========
1. Use pseudo mnt to manage domain's lifecycle.
2. Use a linked list to maintain & traverse domains.
3. Use pseudo sb to create anonymous inode for recording cookie's info
   and manage cookies lifecycle.

[Flow Path]
===========
1. User specify a new 'domain_id' in mount option.
   1.1 Traverse domain list, compare domain_id with existing domain.[Miss]
   1.2 Create a new domain(volume), add it to domain list.
   1.3 Traverse pseudo sb's inode list, compare cookie name with
       existing cookies.[Miss]
   1.4 Alloc new anonymous inodes and cookies.

2. User specify an existing 'domain_id' in mount option and the data
   blob is existed in domain.
   2.1 Traverse domain list, compare domain_id with existing domain.[Hit]
   2.2 Reuse the domain and increase its refcnt.
   2.3 Traverse pseudo sb's inode list, compare cookie name with
   	   existing cookies.[Hit]
   2.4 Reuse the cookie and increase its refcnt.

RFC: https://lore.kernel.org/all/YxAlO%2FDHDrIAafR2@B-P7TQMD6M-0146.local/
V1: https://lore.kernel.org/all/20220902034748.60868-1-zhujia.zj@bytedance.com/
V2: https://lore.kernel.org/all/20220902105305.79687-1-zhujia.zj@bytedance.com/

Jia Zhu (6):
  erofs: use kill_anon_super() to kill super in fscache mode
  erofs: code clean up for fscache
  erofs: introduce 'domain_id' mount option
  erofs: introduce fscache-based domain
  erofs: introduce a pseudo mnt to manage shared cookies
  erofs: Support sharing cookies in the same domain

 fs/erofs/fscache.c  | 252 +++++++++++++++++++++++++++++++++++++++-----
 fs/erofs/internal.h |  30 ++++--
 fs/erofs/super.c    |  72 ++++++++++---
 fs/erofs/sysfs.c    |  19 +++-
 4 files changed, 321 insertions(+), 52 deletions(-)

-- 
2.20.1

[PATCH V3 1/6] erofs: use kill_anon_super() to kill super in fscache mode

[Jia Zhu]

Use kill_anon_super() instead of generic_shutdown_super() since the
mount() in erofs fscache mode uses get_tree_nodev() and associated
anon bdev needs to be freed.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
---
 fs/erofs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 3173debeaa5a..9716d355a63e 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -879,7 +879,7 @@ static void erofs_kill_sb(struct super_block *sb)
 	WARN_ON(sb->s_magic != EROFS_SUPER_MAGIC);
 
 	if (erofs_is_fscache_mode(sb))
-		generic_shutdown_super(sb);
+		kill_anon_super(sb);
 	else
 		kill_block_super(sb);
 
-- 
2.20.1

[PATCH V3 2/6] erofs: code clean up for fscache

[Jia Zhu]

Some cleanups. No logic changes.

Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
---
 fs/erofs/fscache.c  | 26 +++++++++++++++-----------
 fs/erofs/internal.h | 17 ++++++++---------
 fs/erofs/super.c    | 22 +++++++++-------------
 3 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
index 8e01d89c3319..4159cf781924 100644
--- a/fs/erofs/fscache.c
+++ b/fs/erofs/fscache.c
@@ -417,9 +417,8 @@ const struct address_space_operations erofs_fscache_access_aops = {
 	.readahead = erofs_fscache_readahead,
 };
 
-int erofs_fscache_register_cookie(struct super_block *sb,
-				  struct erofs_fscache **fscache,
-				  char *name, bool need_inode)
+struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
+						     char *name, bool need_inode)
 {
 	struct fscache_volume *volume = EROFS_SB(sb)->volume;
 	struct erofs_fscache *ctx;
@@ -428,7 +427,7 @@ int erofs_fscache_register_cookie(struct super_block *sb,
 
 	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	cookie = fscache_acquire_cookie(volume, FSCACHE_ADV_WANT_CACHE_SIZE,
 					name, strlen(name), NULL, 0, 0);
@@ -458,8 +457,7 @@ int erofs_fscache_register_cookie(struct super_block *sb,
 		ctx->inode = inode;
 	}
 
-	*fscache = ctx;
-	return 0;
+	return ctx;
 
 err_cookie:
 	fscache_unuse_cookie(ctx->cookie, NULL, NULL);
@@ -467,13 +465,11 @@ int erofs_fscache_register_cookie(struct super_block *sb,
 	ctx->cookie = NULL;
 err:
 	kfree(ctx);
-	return ret;
+	return ERR_PTR(ret);
 }
 
-void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache)
+void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
 {
-	struct erofs_fscache *ctx = *fscache;
-
 	if (!ctx)
 		return;
 
@@ -485,13 +481,13 @@ void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache)
 	ctx->inode = NULL;
 
 	kfree(ctx);
-	*fscache = NULL;
 }
 
 int erofs_fscache_register_fs(struct super_block *sb)
 {
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
 	struct fscache_volume *volume;
+	struct erofs_fscache *fscache;
 	char *name;
 	int ret = 0;
 
@@ -508,6 +504,12 @@ int erofs_fscache_register_fs(struct super_block *sb)
 
 	sbi->volume = volume;
 	kfree(name);
+
+	fscache = erofs_fscache_register_cookie(sb, sbi->opt.fsid, true);
+	if (IS_ERR(fscache))
+		return PTR_ERR(fscache);
+
+	sbi->s_fscache = fscache;
 	return ret;
 }
 
@@ -515,6 +517,8 @@ void erofs_fscache_unregister_fs(struct super_block *sb)
 {
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
 
+	erofs_fscache_unregister_cookie(sbi->s_fscache);
 	fscache_relinquish_volume(sbi->volume, NULL, false);
+	sbi->s_fscache = NULL;
 	sbi->volume = NULL;
 }
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index cfee49d33b95..aa71eb65e965 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -610,27 +610,26 @@ static inline int z_erofs_load_lzma_config(struct super_block *sb,
 int erofs_fscache_register_fs(struct super_block *sb);
 void erofs_fscache_unregister_fs(struct super_block *sb);
 
-int erofs_fscache_register_cookie(struct super_block *sb,
-				  struct erofs_fscache **fscache,
-				  char *name, bool need_inode);
-void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache);
+struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
+						     char *name, bool need_inode);
+void erofs_fscache_unregister_cookie(struct erofs_fscache *fscache);
 
 extern const struct address_space_operations erofs_fscache_access_aops;
 #else
 static inline int erofs_fscache_register_fs(struct super_block *sb)
 {
-	return 0;
+	return -EOPNOTSUPP;
 }
 static inline void erofs_fscache_unregister_fs(struct super_block *sb) {}
 
-static inline int erofs_fscache_register_cookie(struct super_block *sb,
-						struct erofs_fscache **fscache,
-						char *name, bool need_inode)
+static inline
+struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
+						     char *name, bool need_inode)
 {
 	return -EOPNOTSUPP;
 }
 
-static inline void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache)
+static inline void erofs_fscache_unregister_cookie(struct erofs_fscache *fscache)
 {
 }
 #endif
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 9716d355a63e..7aa57dcebf31 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -224,10 +224,10 @@ static int erofs_init_device(struct erofs_buf *buf, struct super_block *sb,
 			     struct erofs_device_info *dif, erofs_off_t *pos)
 {
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	struct erofs_fscache *fscache;
 	struct erofs_deviceslot *dis;
 	struct block_device *bdev;
 	void *ptr;
-	int ret;
 
 	ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*pos), EROFS_KMAP);
 	if (IS_ERR(ptr))
@@ -245,10 +245,10 @@ static int erofs_init_device(struct erofs_buf *buf, struct super_block *sb,
 	}
 
 	if (erofs_is_fscache_mode(sb)) {
-		ret = erofs_fscache_register_cookie(sb, &dif->fscache,
-				dif->path, false);
-		if (ret)
-			return ret;
+		fscache = erofs_fscache_register_cookie(sb, dif->path, false);
+		if (IS_ERR(fscache))
+			return PTR_ERR(fscache);
+		dif->fscache = fscache;
 	} else {
 		bdev = blkdev_get_by_path(dif->path, FMODE_READ | FMODE_EXCL,
 					  sb->s_type);
@@ -706,11 +706,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 		if (err)
 			return err;
 
-		err = erofs_fscache_register_cookie(sb, &sbi->s_fscache,
-						    sbi->opt.fsid, true);
-		if (err)
-			return err;
-
 		err = super_setup_bdi(sb);
 		if (err)
 			return err;
@@ -817,7 +812,8 @@ static int erofs_release_device_info(int id, void *ptr, void *data)
 	fs_put_dax(dif->dax_dev, NULL);
 	if (dif->bdev)
 		blkdev_put(dif->bdev, FMODE_READ | FMODE_EXCL);
-	erofs_fscache_unregister_cookie(&dif->fscache);
+	erofs_fscache_unregister_cookie(dif->fscache);
+	dif->fscache = NULL;
 	kfree(dif->path);
 	kfree(dif);
 	return 0;
@@ -889,7 +885,6 @@ static void erofs_kill_sb(struct super_block *sb)
 
 	erofs_free_dev_context(sbi->devs);
 	fs_put_dax(sbi->dax_dev, NULL);
-	erofs_fscache_unregister_cookie(&sbi->s_fscache);
 	erofs_fscache_unregister_fs(sb);
 	kfree(sbi->opt.fsid);
 	kfree(sbi);
@@ -909,7 +904,8 @@ static void erofs_put_super(struct super_block *sb)
 	iput(sbi->managed_cache);
 	sbi->managed_cache = NULL;
 #endif
-	erofs_fscache_unregister_cookie(&sbi->s_fscache);
+	erofs_fscache_unregister_cookie(sbi->s_fscache);
+	sbi->s_fscache = NULL;
 }
 
 static struct file_system_type erofs_fs_type = {
-- 
2.20.1

[PATCH V3 3/6] erofs: introduce 'domain_id' mount option

[Jia Zhu]

Introduce 'domain_id' mount option to enable shared domain sementics.
In which case, the related cookie is shared if two mountpoints in the
same domain have the same data blob. Users could specify the name of
domain by this mount option.

Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
---
 fs/erofs/internal.h |  1 +
 fs/erofs/super.c    | 17 +++++++++++++++++
 fs/erofs/sysfs.c    | 19 +++++++++++++++++--
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index aa71eb65e965..2d129c6b3027 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -76,6 +76,7 @@ struct erofs_mount_opts {
 #endif
 	unsigned int mount_opt;
 	char *fsid;
+	char *domain_id;
 };
 
 struct erofs_dev_context {
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 7aa57dcebf31..856758ee4869 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -440,6 +440,7 @@ enum {
 	Opt_dax_enum,
 	Opt_device,
 	Opt_fsid,
+	Opt_domain_id,
 	Opt_err
 };
 
@@ -465,6 +466,7 @@ static const struct fs_parameter_spec erofs_fs_parameters[] = {
 	fsparam_enum("dax",		Opt_dax_enum, erofs_dax_param_enums),
 	fsparam_string("device",	Opt_device),
 	fsparam_string("fsid",		Opt_fsid),
+	fsparam_string("domain_id",	Opt_domain_id),
 	{}
 };
 
@@ -568,6 +570,16 @@ static int erofs_fc_parse_param(struct fs_context *fc,
 			return -ENOMEM;
 #else
 		errorfc(fc, "fsid option not supported");
+#endif
+		break;
+	case Opt_domain_id:
+#ifdef CONFIG_EROFS_FS_ONDEMAND
+		kfree(ctx->opt.domain_id);
+		ctx->opt.domain_id = kstrdup(param->string, GFP_KERNEL);
+		if (!ctx->opt.domain_id)
+			return -ENOMEM;
+#else
+		errorfc(fc, "domain_id option not supported");
 #endif
 		break;
 	default:
@@ -695,6 +707,7 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_fs_info = sbi;
 	sbi->opt = ctx->opt;
 	ctx->opt.fsid = NULL;
+	ctx->opt.domain_id = NULL;
 	sbi->devs = ctx->devs;
 	ctx->devs = NULL;
 
@@ -834,6 +847,7 @@ static void erofs_fc_free(struct fs_context *fc)
 
 	erofs_free_dev_context(ctx->devs);
 	kfree(ctx->opt.fsid);
+	kfree(ctx->opt.domain_id);
 	kfree(ctx);
 }
 
@@ -887,6 +901,7 @@ static void erofs_kill_sb(struct super_block *sb)
 	fs_put_dax(sbi->dax_dev, NULL);
 	erofs_fscache_unregister_fs(sb);
 	kfree(sbi->opt.fsid);
+	kfree(sbi->opt.domain_id);
 	kfree(sbi);
 	sb->s_fs_info = NULL;
 }
@@ -1040,6 +1055,8 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
 #ifdef CONFIG_EROFS_FS_ONDEMAND
 	if (opt->fsid)
 		seq_printf(seq, ",fsid=%s", opt->fsid);
+	if (opt->domain_id)
+		seq_printf(seq, ",domain_id=%s", opt->domain_id);
 #endif
 	return 0;
 }
diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
index c1383e508bbe..341fb43ad587 100644
--- a/fs/erofs/sysfs.c
+++ b/fs/erofs/sysfs.c
@@ -201,12 +201,27 @@ static struct kobject erofs_feat = {
 int erofs_register_sysfs(struct super_block *sb)
 {
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	char *name;
+	char *str = NULL;
 	int err;
 
+	if (erofs_is_fscache_mode(sb)) {
+		if (sbi->opt.domain_id) {
+			str = kasprintf(GFP_KERNEL, "%s,%s", sbi->opt.domain_id,
+					sbi->opt.fsid);
+			if (!str)
+				return -ENOMEM;
+			name = str;
+		} else {
+			name = sbi->opt.fsid;
+		}
+	} else {
+		name = sb->s_id;
+	}
 	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",
-			erofs_is_fscache_mode(sb) ? sbi->opt.fsid : sb->s_id);
+	err = kobject_init_and_add(&sbi->s_kobj, &erofs_sb_ktype, NULL, "%s", name);
+	kfree(str);
 	if (err)
 		goto put_sb_kobj;
 	return 0;
-- 
2.20.1

[PATCH V3 4/6] erofs: introduce fscache-based domain

[Jia Zhu]

A new fscache-based shared domain mode is going to be introduced for
erofs. In which case, same data blobs in same domain will be shared
and reused to reduce on-disk space usage.

As the first step, we use pseudo mnt to manage and maintain domain's
lifecycle.

The implementation of sharing blobs will be introduced in subsequent
patches.

Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
---
 fs/erofs/fscache.c  | 134 ++++++++++++++++++++++++++++++++++++++------
 fs/erofs/internal.h |   9 +++
 2 files changed, 127 insertions(+), 16 deletions(-)

diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
index 4159cf781924..b2100dc67cde 100644
--- a/fs/erofs/fscache.c
+++ b/fs/erofs/fscache.c
@@ -1,10 +1,14 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (C) 2022, Alibaba Cloud
+ * Copyright (C) 2022, Bytedance Inc. All rights reserved.
  */
 #include <linux/fscache.h>
 #include "internal.h"
 
+static DEFINE_MUTEX(erofs_domain_list_lock);
+static LIST_HEAD(erofs_domain_list);
+
 static struct netfs_io_request *erofs_fscache_alloc_request(struct address_space *mapping,
 					     loff_t start, size_t len)
 {
@@ -417,6 +421,106 @@ const struct address_space_operations erofs_fscache_access_aops = {
 	.readahead = erofs_fscache_readahead,
 };
 
+static
+struct erofs_domain *erofs_fscache_domain_get(struct erofs_domain *domain)
+{
+	refcount_inc(&domain->ref);
+	return domain;
+}
+
+static void erofs_fscache_domain_put(struct erofs_domain *domain)
+{
+	if (!domain)
+		return;
+	if (refcount_dec_and_test(&domain->ref)) {
+		fscache_relinquish_volume(domain->volume, NULL, false);
+		mutex_lock(&erofs_domain_list_lock);
+		list_del(&domain->list);
+		mutex_unlock(&erofs_domain_list_lock);
+		kfree(domain->domain_id);
+		kfree(domain);
+	}
+}
+
+static int erofs_fscache_register_volume(struct super_block *sb)
+{
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	char *domain_id = sbi->opt.domain_id;
+	struct fscache_volume *volume;
+	char *name;
+	int ret = 0;
+
+	if (domain_id)
+		name = kasprintf(GFP_KERNEL, "erofs,%s", domain_id);
+	else
+		name = kasprintf(GFP_KERNEL, "erofs,%s", sbi->opt.fsid);
+	if (!name)
+		return -ENOMEM;
+
+	volume = fscache_acquire_volume(name, NULL, NULL, 0);
+	if (IS_ERR_OR_NULL(volume)) {
+		erofs_err(sb, "failed to register volume for %s", name);
+		ret = volume ? PTR_ERR(volume) : -EOPNOTSUPP;
+		volume = NULL;
+	}
+
+	sbi->volume = volume;
+	kfree(name);
+	return ret;
+}
+
+static int erofs_fscache_init_domain(struct super_block *sb)
+{
+	int err;
+	struct erofs_domain *domain;
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+
+	domain = kzalloc(sizeof(struct erofs_domain), GFP_KERNEL);
+	if (!domain)
+		return -ENOMEM;
+
+	domain->domain_id = kstrdup(sbi->opt.domain_id, GFP_KERNEL);
+	if (!domain->domain_id) {
+		kfree(domain);
+		return -ENOMEM;
+	}
+	sbi->domain = domain;
+	err = erofs_fscache_register_volume(sb);
+	if (err)
+		goto out;
+
+	domain->volume = sbi->volume;
+	refcount_set(&domain->ref, 1);
+	mutex_init(&domain->mutex);
+	list_add(&domain->list, &erofs_domain_list);
+	return 0;
+out:
+	kfree(domain->domain_id);
+	kfree(domain);
+	sbi->domain = NULL;
+	return err;
+}
+
+static int erofs_fscache_register_domain(struct super_block *sb)
+{
+	int err;
+	struct erofs_domain *domain;
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+
+	mutex_lock(&erofs_domain_list_lock);
+	list_for_each_entry(domain, &erofs_domain_list, list) {
+		if (!strcmp(domain->domain_id, sbi->opt.domain_id)) {
+			sbi->domain = erofs_fscache_domain_get(domain);
+			sbi->volume = domain->volume;
+			mutex_unlock(&erofs_domain_list_lock);
+			return 0;
+		}
+	}
+	err = erofs_fscache_init_domain(sb);
+	mutex_unlock(&erofs_domain_list_lock);
+	return err;
+}
+
 struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
 						     char *name, bool need_inode)
 {
@@ -486,24 +590,16 @@ void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
 int erofs_fscache_register_fs(struct super_block *sb)
 {
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
-	struct fscache_volume *volume;
 	struct erofs_fscache *fscache;
-	char *name;
-	int ret = 0;
+	int ret;
 
-	name = kasprintf(GFP_KERNEL, "erofs,%s", sbi->opt.fsid);
-	if (!name)
-		return -ENOMEM;
+	if (sbi->opt.domain_id)
+		ret = erofs_fscache_register_domain(sb);
+	else
+		ret = erofs_fscache_register_volume(sb);
 
-	volume = fscache_acquire_volume(name, NULL, NULL, 0);
-	if (IS_ERR_OR_NULL(volume)) {
-		erofs_err(sb, "failed to register volume for %s", name);
-		ret = volume ? PTR_ERR(volume) : -EOPNOTSUPP;
-		volume = NULL;
-	}
-
-	sbi->volume = volume;
-	kfree(name);
+	if (ret)
+		return ret;
 
 	fscache = erofs_fscache_register_cookie(sb, sbi->opt.fsid, true);
 	if (IS_ERR(fscache))
@@ -518,7 +614,13 @@ void erofs_fscache_unregister_fs(struct super_block *sb)
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
 
 	erofs_fscache_unregister_cookie(sbi->s_fscache);
-	fscache_relinquish_volume(sbi->volume, NULL, false);
 	sbi->s_fscache = NULL;
+
+	if (sbi->domain)
+		erofs_fscache_domain_put(sbi->domain);
+	else
+		fscache_relinquish_volume(sbi->volume, NULL, false);
+
 	sbi->volume = NULL;
+	sbi->domain = NULL;
 }
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 2d129c6b3027..5ce6889d6f1d 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -99,6 +99,14 @@ struct erofs_sb_lz4_info {
 	u16 max_pclusterblks;
 };
 
+struct erofs_domain {
+	refcount_t ref;
+	struct mutex mutex;
+	struct list_head list;
+	struct fscache_volume *volume;
+	char *domain_id;
+};
+
 struct erofs_fscache {
 	struct fscache_cookie *cookie;
 	struct inode *inode;
@@ -158,6 +166,7 @@ struct erofs_sb_info {
 	/* fscache support */
 	struct fscache_volume *volume;
 	struct erofs_fscache *s_fscache;
+	struct erofs_domain *domain;
 };
 
 #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info)
-- 
2.20.1

[PATCH V3 5/6] erofs: introduce a pseudo mnt to manage shared cookies

[Jia Zhu]

Use a pseudo mnt to manage shared cookies.

Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
---
 fs/erofs/fscache.c  | 13 +++++++++++++
 fs/erofs/internal.h |  1 +
 fs/erofs/super.c    | 31 +++++++++++++++++++++++++++++--
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
index b2100dc67cde..4e0a441afb7d 100644
--- a/fs/erofs/fscache.c
+++ b/fs/erofs/fscache.c
@@ -8,6 +8,7 @@
 
 static DEFINE_MUTEX(erofs_domain_list_lock);
 static LIST_HEAD(erofs_domain_list);
+static struct vfsmount *erofs_pseudo_mnt;
 
 static struct netfs_io_request *erofs_fscache_alloc_request(struct address_space *mapping,
 					     loff_t start, size_t len)
@@ -436,6 +437,10 @@ static void erofs_fscache_domain_put(struct erofs_domain *domain)
 		fscache_relinquish_volume(domain->volume, NULL, false);
 		mutex_lock(&erofs_domain_list_lock);
 		list_del(&domain->list);
+		if (list_empty(&erofs_domain_list)) {
+			kern_unmount(erofs_pseudo_mnt);
+			erofs_pseudo_mnt = NULL;
+		}
 		mutex_unlock(&erofs_domain_list_lock);
 		kfree(domain->domain_id);
 		kfree(domain);
@@ -489,6 +494,14 @@ static int erofs_fscache_init_domain(struct super_block *sb)
 	if (err)
 		goto out;
 
+	if (!erofs_pseudo_mnt) {
+		erofs_pseudo_mnt = kern_mount(&erofs_fs_type);
+		if (IS_ERR(erofs_pseudo_mnt)) {
+			err = PTR_ERR(erofs_pseudo_mnt);
+			goto out;
+		}
+	}
+
 	domain->volume = sbi->volume;
 	refcount_set(&domain->ref, 1);
 	mutex_init(&domain->mutex);
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 5ce6889d6f1d..4dd0b545755a 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -403,6 +403,7 @@ struct page *erofs_grab_cache_page_nowait(struct address_space *mapping,
 }
 
 extern const struct super_operations erofs_sops;
+extern struct file_system_type erofs_fs_type;
 
 extern const struct address_space_operations erofs_raw_access_aops;
 extern const struct address_space_operations z_erofs_aops;
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 856758ee4869..ced1d2fd6e4b 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -688,6 +688,13 @@ static const struct export_operations erofs_export_ops = {
 	.get_parent = erofs_get_parent,
 };
 
+static int erofs_fc_fill_pseudo_super(struct super_block *sb, struct fs_context *fc)
+{
+	static const struct tree_descr empty_descr = {""};
+
+	return simple_fill_super(sb, EROFS_SUPER_MAGIC, &empty_descr);
+}
+
 static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct inode *inode;
@@ -789,6 +796,11 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 	return 0;
 }
 
+static int erofs_fc_anon_get_tree(struct fs_context *fc)
+{
+	return get_tree_nodev(fc, erofs_fc_fill_pseudo_super);
+}
+
 static int erofs_fc_get_tree(struct fs_context *fc)
 {
 	struct erofs_fs_context *ctx = fc->fs_private;
@@ -858,10 +870,20 @@ static const struct fs_context_operations erofs_context_ops = {
 	.free		= erofs_fc_free,
 };
 
+static const struct fs_context_operations erofs_anon_context_ops = {
+	.get_tree       = erofs_fc_anon_get_tree,
+};
+
 static int erofs_init_fs_context(struct fs_context *fc)
 {
-	struct erofs_fs_context *ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	struct erofs_fs_context *ctx;
+
+	if (fc->sb_flags & SB_KERNMOUNT) {
+		fc->ops = &erofs_anon_context_ops;
+		return 0;
+	}
 
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
 	ctx->devs = kzalloc(sizeof(struct erofs_dev_context), GFP_KERNEL);
@@ -888,6 +910,11 @@ static void erofs_kill_sb(struct super_block *sb)
 
 	WARN_ON(sb->s_magic != EROFS_SUPER_MAGIC);
 
+	if (sb->s_flags & SB_KERNMOUNT) {
+		kill_litter_super(sb);
+		return;
+	}
+
 	if (erofs_is_fscache_mode(sb))
 		kill_anon_super(sb);
 	else
@@ -923,7 +950,7 @@ static void erofs_put_super(struct super_block *sb)
 	sbi->s_fscache = NULL;
 }
 
-static struct file_system_type erofs_fs_type = {
+struct file_system_type erofs_fs_type = {
 	.owner          = THIS_MODULE,
 	.name           = "erofs",
 	.init_fs_context = erofs_init_fs_context,
-- 
2.20.1

[PATCH V3 6/6] erofs: Support sharing cookies in the same domain

[Jia Zhu]

Several erofs filesystems can belong to one domain, and data blobs can
be shared among these erofs filesystems of same domain.

Users could specify domain_id mount option to create or join into a
domain.

Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
---
 fs/erofs/fscache.c  | 89 +++++++++++++++++++++++++++++++++++++++++++--
 fs/erofs/internal.h |  4 +-
 2 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
index 4e0a441afb7d..e9ae1ee963e2 100644
--- a/fs/erofs/fscache.c
+++ b/fs/erofs/fscache.c
@@ -7,6 +7,7 @@
 #include "internal.h"
 
 static DEFINE_MUTEX(erofs_domain_list_lock);
+static DEFINE_MUTEX(erofs_domain_cookies_lock);
 static LIST_HEAD(erofs_domain_list);
 static struct vfsmount *erofs_pseudo_mnt;
 
@@ -504,7 +505,6 @@ static int erofs_fscache_init_domain(struct super_block *sb)
 
 	domain->volume = sbi->volume;
 	refcount_set(&domain->ref, 1);
-	mutex_init(&domain->mutex);
 	list_add(&domain->list, &erofs_domain_list);
 	return 0;
 out:
@@ -534,8 +534,8 @@ static int erofs_fscache_register_domain(struct super_block *sb)
 	return err;
 }
 
-struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
-						     char *name, bool need_inode)
+struct erofs_fscache *erofs_fscache_acquire_cookie(struct super_block *sb,
+						    char *name, bool need_inode)
 {
 	struct fscache_volume *volume = EROFS_SB(sb)->volume;
 	struct erofs_fscache *ctx;
@@ -585,13 +585,96 @@ struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
 	return ERR_PTR(ret);
 }
 
+static
+struct erofs_fscache *erofs_fscache_domain_init_cookie(struct super_block *sb,
+							char *name, bool need_inode)
+{
+	struct inode *inode;
+	struct erofs_fscache *ctx;
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	struct erofs_domain *domain = sbi->domain;
+
+	ctx = erofs_fscache_acquire_cookie(sb, name, need_inode);
+	if (IS_ERR(ctx))
+		return ctx;
+
+	ctx->name = kstrdup(name, GFP_KERNEL);
+	if (!ctx->name)
+		return ERR_PTR(-ENOMEM);
+
+	inode = new_inode(erofs_pseudo_mnt->mnt_sb);
+	if (!inode) {
+		kfree(ctx->name);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	ctx->domain = domain;
+	ctx->anon_inode = inode;
+	inode->i_private = ctx;
+	erofs_fscache_domain_get(domain);
+	return ctx;
+}
+
+static
+struct erofs_fscache *erofs_domain_register_cookie(struct super_block *sb,
+						    char *name, bool need_inode)
+{
+	struct inode *inode;
+	struct erofs_fscache *ctx;
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	struct erofs_domain *domain = sbi->domain;
+	struct super_block *psb = erofs_pseudo_mnt->mnt_sb;
+
+	mutex_lock(&erofs_domain_cookies_lock);
+	list_for_each_entry(inode, &psb->s_inodes, i_sb_list) {
+		ctx = inode->i_private;
+		if (!ctx)
+			continue;
+		if (ctx->domain == domain && !strcmp(ctx->name, name)) {
+			igrab(inode);
+			mutex_unlock(&erofs_domain_cookies_lock);
+			return ctx;
+		}
+	}
+	ctx = erofs_fscache_domain_init_cookie(sb, name, need_inode);
+	mutex_unlock(&erofs_domain_cookies_lock);
+	return ctx;
+}
+
+struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
+						     char *name, bool need_inode)
+{
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+
+	if (sbi->opt.domain_id)
+		return erofs_domain_register_cookie(sb, name, need_inode);
+	else
+		return erofs_fscache_acquire_cookie(sb, name, need_inode);
+}
+
 void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
 {
+	struct erofs_domain *domain;
+
 	if (!ctx)
 		return;
+	domain = ctx->domain;
+	if (domain) {
+		mutex_lock(&erofs_domain_cookies_lock);
+		/* Cookie is still in use */
+		if (atomic_read(&ctx->anon_inode->i_count) > 1) {
+			iput(ctx->anon_inode);
+			mutex_unlock(&erofs_domain_cookies_lock);
+			return;
+		}
+		iput(ctx->anon_inode);
+		kfree(ctx->name);
+		mutex_unlock(&erofs_domain_cookies_lock);
+	}
 
 	fscache_unuse_cookie(ctx->cookie, NULL, NULL);
 	fscache_relinquish_cookie(ctx->cookie, false);
+	erofs_fscache_domain_put(domain);
 	ctx->cookie = NULL;
 
 	iput(ctx->inode);
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 4dd0b545755a..8a6f94b27a23 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -101,7 +101,6 @@ struct erofs_sb_lz4_info {
 
 struct erofs_domain {
 	refcount_t ref;
-	struct mutex mutex;
 	struct list_head list;
 	struct fscache_volume *volume;
 	char *domain_id;
@@ -110,6 +109,9 @@ struct erofs_domain {
 struct erofs_fscache {
 	struct fscache_cookie *cookie;
 	struct inode *inode;
+	struct inode *anon_inode;
+	struct erofs_domain *domain;
+	char *name;
 };
 
 struct erofs_sb_info {
-- 
2.20.1

Re: [PATCH V3 4/6] erofs: introduce fscache-based domain

[Gao Xiang]

On Wed, Sep 14, 2022 at 06:50:39PM +0800, Jia Zhu wrote:
> A new fscache-based shared domain mode is going to be introduced for
> erofs. In which case, same data blobs in same domain will be shared
> and reused to reduce on-disk space usage.
> 
> As the first step, we use pseudo mnt to manage and maintain domain's
> lifecycle.
> 
> The implementation of sharing blobs will be introduced in subsequent
> patches.
> 
> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
> ---
>  fs/erofs/fscache.c  | 134 ++++++++++++++++++++++++++++++++++++++------
>  fs/erofs/internal.h |   9 +++
>  2 files changed, 127 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
> index 4159cf781924..b2100dc67cde 100644
> --- a/fs/erofs/fscache.c
> +++ b/fs/erofs/fscache.c
> @@ -1,10 +1,14 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (C) 2022, Alibaba Cloud
> + * Copyright (C) 2022, Bytedance Inc. All rights reserved.
>   */
>  #include <linux/fscache.h>
>  #include "internal.h"
>  
> +static DEFINE_MUTEX(erofs_domain_list_lock);
> +static LIST_HEAD(erofs_domain_list);
> +
>  static struct netfs_io_request *erofs_fscache_alloc_request(struct address_space *mapping,
>  					     loff_t start, size_t len)
>  {
> @@ -417,6 +421,106 @@ const struct address_space_operations erofs_fscache_access_aops = {
>  	.readahead = erofs_fscache_readahead,
>  };
>  
> +static
> +struct erofs_domain *erofs_fscache_domain_get(struct erofs_domain *domain)
> +{
> +	refcount_inc(&domain->ref);
> +	return domain;
> +}

We can just open-code that.

> +
> +static void erofs_fscache_domain_put(struct erofs_domain *domain)
> +{
> +	if (!domain)
> +		return;
> +	if (refcount_dec_and_test(&domain->ref)) {
> +		fscache_relinquish_volume(domain->volume, NULL, false);
> +		mutex_lock(&erofs_domain_list_lock);
> +		list_del(&domain->list);
> +		mutex_unlock(&erofs_domain_list_lock);
> +		kfree(domain->domain_id);
> +		kfree(domain);
> +	}
> +}
> +
> +static int erofs_fscache_register_volume(struct super_block *sb)
> +{
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	char *domain_id = sbi->opt.domain_id;
> +	struct fscache_volume *volume;
> +	char *name;
> +	int ret = 0;
> +
> +	if (domain_id)
> +		name = kasprintf(GFP_KERNEL, "erofs,%s", domain_id);
> +	else
> +		name = kasprintf(GFP_KERNEL, "erofs,%s", sbi->opt.fsid);

I think we could just use

	name = kasprintf(GFP_KERNEL, "erofs,%s",
			 domain_id ? domain_id : sbi->opt.fsid);

here.

> +	if (!name)
> +		return -ENOMEM;
> +
> +	volume = fscache_acquire_volume(name, NULL, NULL, 0);
> +	if (IS_ERR_OR_NULL(volume)) {
> +		erofs_err(sb, "failed to register volume for %s", name);
> +		ret = volume ? PTR_ERR(volume) : -EOPNOTSUPP;
> +		volume = NULL;
> +	}
> +
> +	sbi->volume = volume;
> +	kfree(name);
> +	return ret;
> +}
> +
> +static int erofs_fscache_init_domain(struct super_block *sb)
> +{
> +	int err;
> +	struct erofs_domain *domain;
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +
> +	domain = kzalloc(sizeof(struct erofs_domain), GFP_KERNEL);
> +	if (!domain)
> +		return -ENOMEM;
> +
> +	domain->domain_id = kstrdup(sbi->opt.domain_id, GFP_KERNEL);
> +	if (!domain->domain_id) {
> +		kfree(domain);
> +		return -ENOMEM;
> +	}
> +	sbi->domain = domain;
> +	err = erofs_fscache_register_volume(sb);
> +	if (err)
> +		goto out;
> +
> +	domain->volume = sbi->volume;
> +	refcount_set(&domain->ref, 1);
> +	mutex_init(&domain->mutex);
> +	list_add(&domain->list, &erofs_domain_list);
> +	return 0;
> +out:
> +	kfree(domain->domain_id);
> +	kfree(domain);
> +	sbi->domain = NULL;
> +	return err;
> +}
> +
> +static int erofs_fscache_register_domain(struct super_block *sb)
> +{
> +	int err;
> +	struct erofs_domain *domain;
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +
> +	mutex_lock(&erofs_domain_list_lock);
> +	list_for_each_entry(domain, &erofs_domain_list, list) {
> +		if (!strcmp(domain->domain_id, sbi->opt.domain_id)) {
> +			sbi->domain = erofs_fscache_domain_get(domain);
> +			sbi->volume = domain->volume;
> +			mutex_unlock(&erofs_domain_list_lock);
> +			return 0;
> +		}
> +	}
> +	err = erofs_fscache_init_domain(sb);

why introduce erofs_fscache_init_domain?

> +	mutex_unlock(&erofs_domain_list_lock);
> +	return err;
> +}
> +
>  struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
>  						     char *name, bool need_inode)
>  {
> @@ -486,24 +590,16 @@ void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
>  int erofs_fscache_register_fs(struct super_block *sb)
>  {
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
> -	struct fscache_volume *volume;
>  	struct erofs_fscache *fscache;
> -	char *name;
> -	int ret = 0;
> +	int ret;
>  
> -	name = kasprintf(GFP_KERNEL, "erofs,%s", sbi->opt.fsid);
> -	if (!name)
> -		return -ENOMEM;
> +	if (sbi->opt.domain_id)
> +		ret = erofs_fscache_register_domain(sb);
> +	else
> +		ret = erofs_fscache_register_volume(sb);
>  
> -	volume = fscache_acquire_volume(name, NULL, NULL, 0);
> -	if (IS_ERR_OR_NULL(volume)) {
> -		erofs_err(sb, "failed to register volume for %s", name);
> -		ret = volume ? PTR_ERR(volume) : -EOPNOTSUPP;
> -		volume = NULL;
> -	}
> -
> -	sbi->volume = volume;
> -	kfree(name);
> +	if (ret)
> +		return ret;
>  
>  	fscache = erofs_fscache_register_cookie(sb, sbi->opt.fsid, true);
>  	if (IS_ERR(fscache))
> @@ -518,7 +614,13 @@ void erofs_fscache_unregister_fs(struct super_block *sb)
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
>  
>  	erofs_fscache_unregister_cookie(sbi->s_fscache);
> -	fscache_relinquish_volume(sbi->volume, NULL, false);
>  	sbi->s_fscache = NULL;
> +
> +	if (sbi->domain)
> +		erofs_fscache_domain_put(sbi->domain);
> +	else
> +		fscache_relinquish_volume(sbi->volume, NULL, false);
> +

How about using one helper and pass in sb directly instead?

Thanks,
Gao Xiang

Re: [PATCH V3 6/6] erofs: Support sharing cookies in the same domain

[Gao Xiang]

On Wed, Sep 14, 2022 at 06:50:41PM +0800, Jia Zhu wrote:
> Several erofs filesystems can belong to one domain, and data blobs can
> be shared among these erofs filesystems of same domain.
> 
> Users could specify domain_id mount option to create or join into a
> domain.
> 
> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
> ---
>  fs/erofs/fscache.c  | 89 +++++++++++++++++++++++++++++++++++++++++++--
>  fs/erofs/internal.h |  4 +-
>  2 files changed, 89 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
> index 4e0a441afb7d..e9ae1ee963e2 100644
> --- a/fs/erofs/fscache.c
> +++ b/fs/erofs/fscache.c
> @@ -7,6 +7,7 @@
>  #include "internal.h"
>  
>  static DEFINE_MUTEX(erofs_domain_list_lock);
> +static DEFINE_MUTEX(erofs_domain_cookies_lock);
>  static LIST_HEAD(erofs_domain_list);
>  static struct vfsmount *erofs_pseudo_mnt;
>  
> @@ -504,7 +505,6 @@ static int erofs_fscache_init_domain(struct super_block *sb)
>  
>  	domain->volume = sbi->volume;
>  	refcount_set(&domain->ref, 1);
> -	mutex_init(&domain->mutex);
>  	list_add(&domain->list, &erofs_domain_list);
>  	return 0;
>  out:
> @@ -534,8 +534,8 @@ static int erofs_fscache_register_domain(struct super_block *sb)
>  	return err;
>  }
>  
> -struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
> -						     char *name, bool need_inode)
> +struct erofs_fscache *erofs_fscache_acquire_cookie(struct super_block *sb,
> +						    char *name, bool need_inode)
>  {
>  	struct fscache_volume *volume = EROFS_SB(sb)->volume;
>  	struct erofs_fscache *ctx;
> @@ -585,13 +585,96 @@ struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
>  	return ERR_PTR(ret);
>  }
>  
> +static
> +struct erofs_fscache *erofs_fscache_domain_init_cookie(struct super_block *sb,
> +							char *name, bool need_inode)
> +{
> +	struct inode *inode;
> +	struct erofs_fscache *ctx;
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	struct erofs_domain *domain = sbi->domain;

	struct erofs_domain *domain = EROFS_SB(sb)->domain;

> +
> +	ctx = erofs_fscache_acquire_cookie(sb, name, need_inode);
> +	if (IS_ERR(ctx))
> +		return ctx;
> +
> +	ctx->name = kstrdup(name, GFP_KERNEL);
> +	if (!ctx->name)
> +		return ERR_PTR(-ENOMEM);
> +
> +	inode = new_inode(erofs_pseudo_mnt->mnt_sb);
> +	if (!inode) {
> +		kfree(ctx->name);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	ctx->domain = domain;
> +	ctx->anon_inode = inode;
> +	inode->i_private = ctx;
> +	erofs_fscache_domain_get(domain);
> +	return ctx;
> +}
> +
> +static
> +struct erofs_fscache *erofs_domain_register_cookie(struct super_block *sb,
> +						    char *name, bool need_inode)
> +{
> +	struct inode *inode;
> +	struct erofs_fscache *ctx;
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	struct erofs_domain *domain = sbi->domain;

	struct erofs_domain *domain = EROFS_SB(sb)->domain;

> +	struct super_block *psb = erofs_pseudo_mnt->mnt_sb;
> +
> +	mutex_lock(&erofs_domain_cookies_lock);
> +	list_for_each_entry(inode, &psb->s_inodes, i_sb_list) {
> +		ctx = inode->i_private;
> +		if (!ctx)
> +			continue;
> +		if (ctx->domain == domain && !strcmp(ctx->name, name)) {
> +			igrab(inode);
> +			mutex_unlock(&erofs_domain_cookies_lock);
> +			return ctx;
> +		}

		if (!ctx || ctx->domain != domain ||
		    strcmp(ctx->name, name))
			continue;
		igrab(inode);
		mutex_unlock(&erofs_domain_cookies_lock);
		return ctx;

> +	}
> +	ctx = erofs_fscache_domain_init_cookie(sb, name, need_inode);
> +	mutex_unlock(&erofs_domain_cookies_lock);
> +	return ctx;
> +}
> +
> +struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
> +						     char *name, bool need_inode)
> +{
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +
> +	if (sbi->opt.domain_id)
> +		return erofs_domain_register_cookie(sb, name, need_inode);
> +	else
> +		return erofs_fscache_acquire_cookie(sb, name, need_inode);

	if (EROFS_SB(sb)->opt.domain_id)
		return erofs_domain_register_cookie(sb, name, need_inode);
	Return erofs_fscache_acquire_cookie(sb, name, need_inode);

Thanks,
Gao Xiang

Re: [PATCH V3 2/6] erofs: code clean up for fscache

[JeffleXu]

On 9/14/22 6:50 PM, Jia Zhu wrote:
> Some cleanups. No logic changes.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
> ---
>  fs/erofs/fscache.c  | 26 +++++++++++++++-----------
>  fs/erofs/internal.h | 17 ++++++++---------
>  fs/erofs/super.c    | 22 +++++++++-------------
>  3 files changed, 32 insertions(+), 33 deletions(-)
> 
> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
> index 8e01d89c3319..4159cf781924 100644
> --- a/fs/erofs/fscache.c
> +++ b/fs/erofs/fscache.c
> @@ -417,9 +417,8 @@ const struct address_space_operations erofs_fscache_access_aops = {
>  	.readahead = erofs_fscache_readahead,
>  };
>  
> -int erofs_fscache_register_cookie(struct super_block *sb,
> -				  struct erofs_fscache **fscache,
> -				  char *name, bool need_inode)
> +struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
> +						     char *name, bool need_inode)
>  {
>  	struct fscache_volume *volume = EROFS_SB(sb)->volume;
>  	struct erofs_fscache *ctx;
> @@ -428,7 +427,7 @@ int erofs_fscache_register_cookie(struct super_block *sb,
>  
>  	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>  	if (!ctx)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
>  	cookie = fscache_acquire_cookie(volume, FSCACHE_ADV_WANT_CACHE_SIZE,
>  					name, strlen(name), NULL, 0, 0);
> @@ -458,8 +457,7 @@ int erofs_fscache_register_cookie(struct super_block *sb,
>  		ctx->inode = inode;
>  	}
>  
> -	*fscache = ctx;
> -	return 0;
> +	return ctx;
>  
>  err_cookie:
>  	fscache_unuse_cookie(ctx->cookie, NULL, NULL);
> @@ -467,13 +465,11 @@ int erofs_fscache_register_cookie(struct super_block *sb,
>  	ctx->cookie = NULL;
>  err:
>  	kfree(ctx);
> -	return ret;
> +	return ERR_PTR(ret);
>  }
>  
> -void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache)
> +void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
>  {
> -	struct erofs_fscache *ctx = *fscache;
> -
>  	if (!ctx)
>  		return;
>  
> @@ -485,13 +481,13 @@ void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache)
>  	ctx->inode = NULL;
>  
>  	kfree(ctx);
> -	*fscache = NULL;
>  }
>  
>  int erofs_fscache_register_fs(struct super_block *sb)
>  {
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
>  	struct fscache_volume *volume;
> +	struct erofs_fscache *fscache;
>  	char *name;
>  	int ret = 0;
>  
> @@ -508,6 +504,12 @@ int erofs_fscache_register_fs(struct super_block *sb)
>  
>  	sbi->volume = volume;
>  	kfree(name);

If the above fscache_acquire_volume() fails, we'd better return directly
without going on registering cookie.

> +
> +	fscache = erofs_fscache_register_cookie(sb, sbi->opt.fsid, true);
> +	if (IS_ERR(fscache))
> +		return PTR_ERR(fscache);

We'd better add some comment like "the registered volume will be cleaned
up in .kill_sb() in error case".


> +
> +	sbi->s_fscache = fscache;
>  	return ret;
>  }
>  
> @@ -515,6 +517,8 @@ void erofs_fscache_unregister_fs(struct super_block *sb)
>  {
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
>  
> +	erofs_fscache_unregister_cookie(sbi->s_fscache);
>  	fscache_relinquish_volume(sbi->volume, NULL, false);
> +	sbi->s_fscache = NULL;
>  	sbi->volume = NULL;
>  }
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index cfee49d33b95..aa71eb65e965 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -610,27 +610,26 @@ static inline int z_erofs_load_lzma_config(struct super_block *sb,
>  int erofs_fscache_register_fs(struct super_block *sb);
>  void erofs_fscache_unregister_fs(struct super_block *sb);
>  
> -int erofs_fscache_register_cookie(struct super_block *sb,
> -				  struct erofs_fscache **fscache,
> -				  char *name, bool need_inode);
> -void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache);
> +struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
> +						     char *name, bool need_inode);
> +void erofs_fscache_unregister_cookie(struct erofs_fscache *fscache);
>  
>  extern const struct address_space_operations erofs_fscache_access_aops;
>  #else
>  static inline int erofs_fscache_register_fs(struct super_block *sb)
>  {
> -	return 0;
> +	return -EOPNOTSUPP;
>  }
>  static inline void erofs_fscache_unregister_fs(struct super_block *sb) {}
>  
> -static inline int erofs_fscache_register_cookie(struct super_block *sb,
> -						struct erofs_fscache **fscache,
> -						char *name, bool need_inode)
> +static inline
> +struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
> +						     char *name, bool need_inode)
>  {
>  	return -EOPNOTSUPP;
>  }
>  
> -static inline void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache)
> +static inline void erofs_fscache_unregister_cookie(struct erofs_fscache *fscache)
>  {
>  }
>  #endif
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 9716d355a63e..7aa57dcebf31 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -224,10 +224,10 @@ static int erofs_init_device(struct erofs_buf *buf, struct super_block *sb,
>  			     struct erofs_device_info *dif, erofs_off_t *pos)
>  {
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	struct erofs_fscache *fscache;
>  	struct erofs_deviceslot *dis;
>  	struct block_device *bdev;
>  	void *ptr;
> -	int ret;
>  
>  	ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*pos), EROFS_KMAP);
>  	if (IS_ERR(ptr))
> @@ -245,10 +245,10 @@ static int erofs_init_device(struct erofs_buf *buf, struct super_block *sb,
>  	}
>  
>  	if (erofs_is_fscache_mode(sb)) {
> -		ret = erofs_fscache_register_cookie(sb, &dif->fscache,
> -				dif->path, false);
> -		if (ret)
> -			return ret;
> +		fscache = erofs_fscache_register_cookie(sb, dif->path, false);
> +		if (IS_ERR(fscache))
> +			return PTR_ERR(fscache);
> +		dif->fscache = fscache;
>  	} else {
>  		bdev = blkdev_get_by_path(dif->path, FMODE_READ | FMODE_EXCL,
>  					  sb->s_type);
> @@ -706,11 +706,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>  		if (err)
>  			return err;
>  
> -		err = erofs_fscache_register_cookie(sb, &sbi->s_fscache,
> -						    sbi->opt.fsid, true);
> -		if (err)
> -			return err;
> -
>  		err = super_setup_bdi(sb);
>  		if (err)
>  			return err;
> @@ -817,7 +812,8 @@ static int erofs_release_device_info(int id, void *ptr, void *data)
>  	fs_put_dax(dif->dax_dev, NULL);
>  	if (dif->bdev)
>  		blkdev_put(dif->bdev, FMODE_READ | FMODE_EXCL);
> -	erofs_fscache_unregister_cookie(&dif->fscache);
> +	erofs_fscache_unregister_cookie(dif->fscache);
> +	dif->fscache = NULL;
>  	kfree(dif->path);
>  	kfree(dif);
>  	return 0;
> @@ -889,7 +885,6 @@ static void erofs_kill_sb(struct super_block *sb)
>  
>  	erofs_free_dev_context(sbi->devs);
>  	fs_put_dax(sbi->dax_dev, NULL);
> -	erofs_fscache_unregister_cookie(&sbi->s_fscache);
>  	erofs_fscache_unregister_fs(sb);
>  	kfree(sbi->opt.fsid);
>  	kfree(sbi);
> @@ -909,7 +904,8 @@ static void erofs_put_super(struct super_block *sb)
>  	iput(sbi->managed_cache);
>  	sbi->managed_cache = NULL;
>  #endif
> -	erofs_fscache_unregister_cookie(&sbi->s_fscache);
> +	erofs_fscache_unregister_cookie(sbi->s_fscache);
> +	sbi->s_fscache = NULL;

How about calling erofs_fscache_unregister_fs() here? It shall be okay
to also unregister volume here.

>  }
>  
>  static struct file_system_type erofs_fs_type = {

-- 
Thanks,
Jingbo

Re: [PATCH V3 1/6] erofs: use kill_anon_super() to kill super in fscache mode

[JeffleXu]

On 9/14/22 6:50 PM, Jia Zhu wrote:
> Use kill_anon_super() instead of generic_shutdown_super() since the
> mount() in erofs fscache mode uses get_tree_nodev() and associated
> anon bdev needs to be freed.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>

Thanks. You're welcome to use "Suggested-by" in this case. The same with
patch 2.

> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
> ---
>  fs/erofs/super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 3173debeaa5a..9716d355a63e 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -879,7 +879,7 @@ static void erofs_kill_sb(struct super_block *sb)
>  	WARN_ON(sb->s_magic != EROFS_SUPER_MAGIC);
>  
>  	if (erofs_is_fscache_mode(sb))
> -		generic_shutdown_super(sb);
> +		kill_anon_super(sb);
>  	else
>  		kill_block_super(sb);
>  

Reviewed-by: Jingbo Xu <jefflexu@linux.alibaba.com>

-- 
Thanks,
Jingbo

Re: [PATCH V3 4/6] erofs: introduce fscache-based domain

[JeffleXu]

On 9/14/22 6:50 PM, Jia Zhu wrote:
> A new fscache-based shared domain mode is going to be introduced for
> erofs. In which case, same data blobs in same domain will be shared
> and reused to reduce on-disk space usage.
> 
> As the first step, we use pseudo mnt to manage and maintain domain's
> lifecycle.

The commit message needs to be updated, since the pseudo mnt is not
introduced yet in this patch.

> 
> The implementation of sharing blobs will be introduced in subsequent
> patches.
> 
> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
> ---
>  fs/erofs/fscache.c  | 134 ++++++++++++++++++++++++++++++++++++++------
>  fs/erofs/internal.h |   9 +++
>  2 files changed, 127 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
> index 4159cf781924..b2100dc67cde 100644
> --- a/fs/erofs/fscache.c
> +++ b/fs/erofs/fscache.c
> @@ -1,10 +1,14 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (C) 2022, Alibaba Cloud
> + * Copyright (C) 2022, Bytedance Inc. All rights reserved.
>   */
>  #include <linux/fscache.h>
>  #include "internal.h"
>  
> +static DEFINE_MUTEX(erofs_domain_list_lock);
> +static LIST_HEAD(erofs_domain_list);
> +
>  static struct netfs_io_request *erofs_fscache_alloc_request(struct address_space *mapping,
>  					     loff_t start, size_t len)
>  {
> @@ -417,6 +421,106 @@ const struct address_space_operations erofs_fscache_access_aops = {
>  	.readahead = erofs_fscache_readahead,
>  };
>  
> +static
> +struct erofs_domain *erofs_fscache_domain_get(struct erofs_domain *domain)
> +{
> +	refcount_inc(&domain->ref);

refcount_inc_not_zero() is prefered here.

Considering the following time sequence:

CPU1				CPU2
------				------
erofs_fscache_domain_put
  refcount decreased to 0
  				erofs_fscache_register_domain
				  mutex_lock
				  erofs_fscache_domain_get
				    inc refcount to 1
				  mutex_unlock
  mutex_lock
  remove the domain from list
  mutex_unlock



> +	return domain;
> +}
> +
> +static void erofs_fscache_domain_put(struct erofs_domain *domain)
> +{
> +	if (!domain)
> +		return;
> +	if (refcount_dec_and_test(&domain->ref)) {
> +		fscache_relinquish_volume(domain->volume, NULL, false);
> +		mutex_lock(&erofs_domain_list_lock);
> +		list_del(&domain->list);
> +		mutex_unlock(&erofs_domain_list_lock);

We need to delete the domain from the list first, and then calling
fscache_relinquish_volume(), so that others won't race with this.


> +		kfree(domain->domain_id);
> +		kfree(domain);
> +	}
> +}
> +
> +static int erofs_fscache_register_volume(struct super_block *sb)
> +{
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	char *domain_id = sbi->opt.domain_id;
> +	struct fscache_volume *volume;
> +	char *name;
> +	int ret = 0;
> +
> +	if (domain_id)
> +		name = kasprintf(GFP_KERNEL, "erofs,%s", domain_id);
> +	else
> +		name = kasprintf(GFP_KERNEL, "erofs,%s", sbi->opt.fsid);
> +	if (!name)
> +		return -ENOMEM;
> +
> +	volume = fscache_acquire_volume(name, NULL, NULL, 0);
> +	if (IS_ERR_OR_NULL(volume)) {
> +		erofs_err(sb, "failed to register volume for %s", name);
> +		ret = volume ? PTR_ERR(volume) : -EOPNOTSUPP;
> +		volume = NULL;
> +	}
> +
> +	sbi->volume = volume;
> +	kfree(name);
> +	return ret;
> +}
> +
> +static int erofs_fscache_init_domain(struct super_block *sb)
> +{
> +	int err;
> +	struct erofs_domain *domain;
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +
> +	domain = kzalloc(sizeof(struct erofs_domain), GFP_KERNEL);
> +	if (!domain)
> +		return -ENOMEM;
> +
> +	domain->domain_id = kstrdup(sbi->opt.domain_id, GFP_KERNEL);
> +	if (!domain->domain_id) {
> +		kfree(domain);
> +		return -ENOMEM;
> +	}
> +	sbi->domain = domain;

Why bothering setting sbi->domain here? Can't we set sbi->domain finnaly
when the domain has been fully initialized?


> +	err = erofs_fscache_register_volume(sb);
> +	if (err)
> +		goto out;
> +
> +	domain->volume = sbi->volume;
> +	refcount_set(&domain->ref, 1);
> +	mutex_init(&domain->mutex);
> +	list_add(&domain->list, &erofs_domain_list);
> +	return 0;
> +out:
> +	kfree(domain->domain_id);
> +	kfree(domain);
> +	sbi->domain = NULL;
> +	return err;
> +}
> +
> +static int erofs_fscache_register_domain(struct super_block *sb)
> +{
> +	int err;
> +	struct erofs_domain *domain;
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +
> +	mutex_lock(&erofs_domain_list_lock);
> +	list_for_each_entry(domain, &erofs_domain_list, list) {
> +		if (!strcmp(domain->domain_id, sbi->opt.domain_id)) {
> +			sbi->domain = erofs_fscache_domain_get(domain);
> +			sbi->volume = domain->volume;
> +			mutex_unlock(&erofs_domain_list_lock);
> +			return 0;
> +		}
> +	}
> +	err = erofs_fscache_init_domain(sb);
> +	mutex_unlock(&erofs_domain_list_lock);
> +	return err;
> +}
> +
>  struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
>  						     char *name, bool need_inode)
>  {
> @@ -486,24 +590,16 @@ void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
>  int erofs_fscache_register_fs(struct super_block *sb)
>  {
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
> -	struct fscache_volume *volume;
>  	struct erofs_fscache *fscache;
> -	char *name;
> -	int ret = 0;
> +	int ret;
>  
> -	name = kasprintf(GFP_KERNEL, "erofs,%s", sbi->opt.fsid);
> -	if (!name)
> -		return -ENOMEM;
> +	if (sbi->opt.domain_id)
> +		ret = erofs_fscache_register_domain(sb);
> +	else
> +		ret = erofs_fscache_register_volume(sb);
>  
> -	volume = fscache_acquire_volume(name, NULL, NULL, 0);
> -	if (IS_ERR_OR_NULL(volume)) {
> -		erofs_err(sb, "failed to register volume for %s", name);
> -		ret = volume ? PTR_ERR(volume) : -EOPNOTSUPP;
> -		volume = NULL;
> -	}
> -
> -	sbi->volume = volume;
> -	kfree(name);
> +	if (ret)
> +		return ret;
>  
>  	fscache = erofs_fscache_register_cookie(sb, sbi->opt.fsid, true);
>  	if (IS_ERR(fscache))
> @@ -518,7 +614,13 @@ void erofs_fscache_unregister_fs(struct super_block *sb)
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
>  
>  	erofs_fscache_unregister_cookie(sbi->s_fscache);
> -	fscache_relinquish_volume(sbi->volume, NULL, false);
>  	sbi->s_fscache = NULL;
> +
> +	if (sbi->domain)
> +		erofs_fscache_domain_put(sbi->domain);
> +	else
> +		fscache_relinquish_volume(sbi->volume, NULL, false);
> +
>  	sbi->volume = NULL;
> +	sbi->domain = NULL;
>  }
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 2d129c6b3027..5ce6889d6f1d 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -99,6 +99,14 @@ struct erofs_sb_lz4_info {
>  	u16 max_pclusterblks;
>  };
>  
> +struct erofs_domain {
> +	refcount_t ref;
> +	struct mutex mutex;
> +	struct list_head list;
> +	struct fscache_volume *volume;
> +	char *domain_id;
> +};
> +
>  struct erofs_fscache {
>  	struct fscache_cookie *cookie;
>  	struct inode *inode;
> @@ -158,6 +166,7 @@ struct erofs_sb_info {
>  	/* fscache support */
>  	struct fscache_volume *volume;
>  	struct erofs_fscache *s_fscache;
> +	struct erofs_domain *domain;
>  };
>  
>  #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info)

-- 
Thanks,
Jingbo

Re: [PATCH V3 4/6] erofs: introduce fscache-based domain

[JeffleXu]

On 9/14/22 9:21 PM, Gao Xiang wrote:
>> @@ -518,7 +614,13 @@ void erofs_fscache_unregister_fs(struct super_block *sb)
>>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
>>  
>>  	erofs_fscache_unregister_cookie(sbi->s_fscache);
>> -	fscache_relinquish_volume(sbi->volume, NULL, false);
>>  	sbi->s_fscache = NULL;
>> +
>> +	if (sbi->domain)
>> +		erofs_fscache_domain_put(sbi->domain);
>> +	else
>> +		fscache_relinquish_volume(sbi->volume, NULL, false);
>> +
> 
> How about using one helper and pass in sb directly instead?
> 

Then this helper has only one caller...


-- 
Thanks,
Jingbo

Re: [PATCH V3 5/6] erofs: introduce a pseudo mnt to manage shared cookies

[JeffleXu]

On 9/14/22 6:50 PM, Jia Zhu wrote:
> Use a pseudo mnt to manage shared cookies.
> 
> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
> ---
>  fs/erofs/fscache.c  | 13 +++++++++++++
>  fs/erofs/internal.h |  1 +
>  fs/erofs/super.c    | 31 +++++++++++++++++++++++++++++--
>  3 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
> index b2100dc67cde..4e0a441afb7d 100644
> --- a/fs/erofs/fscache.c
> +++ b/fs/erofs/fscache.c
> @@ -8,6 +8,7 @@
>  
>  static DEFINE_MUTEX(erofs_domain_list_lock);
>  static LIST_HEAD(erofs_domain_list);
> +static struct vfsmount *erofs_pseudo_mnt;
>  
>  static struct netfs_io_request *erofs_fscache_alloc_request(struct address_space *mapping,
>  					     loff_t start, size_t len)
> @@ -436,6 +437,10 @@ static void erofs_fscache_domain_put(struct erofs_domain *domain)
>  		fscache_relinquish_volume(domain->volume, NULL, false);
>  		mutex_lock(&erofs_domain_list_lock);
>  		list_del(&domain->list);
> +		if (list_empty(&erofs_domain_list)) {
> +			kern_unmount(erofs_pseudo_mnt);
> +			erofs_pseudo_mnt = NULL;
> +		}
>  		mutex_unlock(&erofs_domain_list_lock);
>  		kfree(domain->domain_id);
>  		kfree(domain);
> @@ -489,6 +494,14 @@ static int erofs_fscache_init_domain(struct super_block *sb)
>  	if (err)
>  		goto out;
>  
> +	if (!erofs_pseudo_mnt) {
> +		erofs_pseudo_mnt = kern_mount(&erofs_fs_type);
> +		if (IS_ERR(erofs_pseudo_mnt)) {
> +			err = PTR_ERR(erofs_pseudo_mnt);
> +			goto out;

Comment like "sbi->volume will be cleaned up in .kill_sb() in the error
path" is needed here. But personally I prefer the function is
self-maintained, i.e. the error path is handled locally, which is more
intuitive. The same with the error path handling I had pointed in patch 2.

> +		}
> +	}
> +
>  	domain->volume = sbi->volume;
>  	refcount_set(&domain->ref, 1);
>  	mutex_init(&domain->mutex);
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 5ce6889d6f1d..4dd0b545755a 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -403,6 +403,7 @@ struct page *erofs_grab_cache_page_nowait(struct address_space *mapping,
>  }
>  
>  extern const struct super_operations erofs_sops;
> +extern struct file_system_type erofs_fs_type;
>  
>  extern const struct address_space_operations erofs_raw_access_aops;
>  extern const struct address_space_operations z_erofs_aops;
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 856758ee4869..ced1d2fd6e4b 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -688,6 +688,13 @@ static const struct export_operations erofs_export_ops = {
>  	.get_parent = erofs_get_parent,
>  };
>  
> +static int erofs_fc_fill_pseudo_super(struct super_block *sb, struct fs_context *fc)
> +{
> +	static const struct tree_descr empty_descr = {""};
> +
> +	return simple_fill_super(sb, EROFS_SUPER_MAGIC, &empty_descr);
> +}
> +
>  static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>  {
>  	struct inode *inode;
> @@ -789,6 +796,11 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>  	return 0;
>  }
>  
> +static int erofs_fc_anon_get_tree(struct fs_context *fc)
> +{
> +	return get_tree_nodev(fc, erofs_fc_fill_pseudo_super);
> +}
> +
>  static int erofs_fc_get_tree(struct fs_context *fc)
>  {
>  	struct erofs_fs_context *ctx = fc->fs_private;
> @@ -858,10 +870,20 @@ static const struct fs_context_operations erofs_context_ops = {
>  	.free		= erofs_fc_free,
>  };
>  
> +static const struct fs_context_operations erofs_anon_context_ops = {
> +	.get_tree       = erofs_fc_anon_get_tree,
> +};
> +
>  static int erofs_init_fs_context(struct fs_context *fc)
>  {
> -	struct erofs_fs_context *ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> +	struct erofs_fs_context *ctx;
> +
> +	if (fc->sb_flags & SB_KERNMOUNT) {
> +		fc->ops = &erofs_anon_context_ops;
> +		return 0;
> +	}
>  
> +	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>  	if (!ctx)
>  		return -ENOMEM;
>  	ctx->devs = kzalloc(sizeof(struct erofs_dev_context), GFP_KERNEL);
> @@ -888,6 +910,11 @@ static void erofs_kill_sb(struct super_block *sb)
>  
>  	WARN_ON(sb->s_magic != EROFS_SUPER_MAGIC);
>  
> +	if (sb->s_flags & SB_KERNMOUNT) {
> +		kill_litter_super(sb);
> +		return;
> +	}
> +
>  	if (erofs_is_fscache_mode(sb))
>  		kill_anon_super(sb);
>  	else
> @@ -923,7 +950,7 @@ static void erofs_put_super(struct super_block *sb)
>  	sbi->s_fscache = NULL;
>  }
>  
> -static struct file_system_type erofs_fs_type = {
> +struct file_system_type erofs_fs_type = {
>  	.owner          = THIS_MODULE,
>  	.name           = "erofs",
>  	.init_fs_context = erofs_init_fs_context,

-- 
Thanks,
Jingbo

Re: [External] Re: [PATCH V3 1/6] erofs: use kill_anon_super() to kill super in fscache mode

[Jia Zhu]

在 2022/9/15 10:28, JeffleXu 写道:
> 
> 
> On 9/14/22 6:50 PM, Jia Zhu wrote:
>> Use kill_anon_super() instead of generic_shutdown_super() since the
>> mount() in erofs fscache mode uses get_tree_nodev() and associated
>> anon bdev needs to be freed.
>>
>> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> 
> Thanks. You're welcome to use "Suggested-by" in this case. The same with
> patch 2.
> 
OK, thanks for your suggestion and review.
>> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
>> ---
>>   fs/erofs/super.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>> index 3173debeaa5a..9716d355a63e 100644
>> --- a/fs/erofs/super.c
>> +++ b/fs/erofs/super.c
>> @@ -879,7 +879,7 @@ static void erofs_kill_sb(struct super_block *sb)
>>   	WARN_ON(sb->s_magic != EROFS_SUPER_MAGIC);
>>   
>>   	if (erofs_is_fscache_mode(sb))
>> -		generic_shutdown_super(sb);
>> +		kill_anon_super(sb);
>>   	else
>>   		kill_block_super(sb);
>>   
> 
> Reviewed-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> 

Re: [PATCH V3 2/6] erofs: code clean up for fscache

[JeffleXu]

On 9/14/22 6:50 PM, Jia Zhu wrote:
>  
> @@ -485,13 +481,13 @@ void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache)
>  	ctx->inode = NULL;

-	ctx->cookie = NULL;
-	ctx->inode = NULL;

I think the above two lines can also be deleted. After all @ctx will be
freed soon.

>  
>  	kfree(ctx);
> -	*fscache = NULL;
>  }
>  


-- 
Thanks,
Jingbo

Re: [PATCH V3 6/6] erofs: Support sharing cookies in the same domain

[JeffleXu]

On 9/14/22 6:50 PM, Jia Zhu wrote:
> Several erofs filesystems can belong to one domain, and data blobs can
> be shared among these erofs filesystems of same domain.
> 
> Users could specify domain_id mount option to create or join into a
> domain.
> 
> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
> ---
>  fs/erofs/fscache.c  | 89 +++++++++++++++++++++++++++++++++++++++++++--
>  fs/erofs/internal.h |  4 +-
>  2 files changed, 89 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
> index 4e0a441afb7d..e9ae1ee963e2 100644
> --- a/fs/erofs/fscache.c
> +++ b/fs/erofs/fscache.c
> @@ -7,6 +7,7 @@
>  #include "internal.h"
>  
>  static DEFINE_MUTEX(erofs_domain_list_lock);
> +static DEFINE_MUTEX(erofs_domain_cookies_lock);
>  static LIST_HEAD(erofs_domain_list);
>  static struct vfsmount *erofs_pseudo_mnt;
>  
> @@ -504,7 +505,6 @@ static int erofs_fscache_init_domain(struct super_block *sb)
>  
>  	domain->volume = sbi->volume;
>  	refcount_set(&domain->ref, 1);
> -	mutex_init(&domain->mutex);

This needs to be folded into patch 4.


>  	list_add(&domain->list, &erofs_domain_list);
>  	return 0;
>  out:
> @@ -534,8 +534,8 @@ static int erofs_fscache_register_domain(struct super_block *sb)
>  	return err;
>  }
>  
> -struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
> -						     char *name, bool need_inode)
> +struct erofs_fscache *erofs_fscache_acquire_cookie(struct super_block *sb,
> +						    char *name, bool need_inode)
>  {
>  	struct fscache_volume *volume = EROFS_SB(sb)->volume;
>  	struct erofs_fscache *ctx;
> @@ -585,13 +585,96 @@ struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
>  	return ERR_PTR(ret);
>  }
>  
> +static
> +struct erofs_fscache *erofs_fscache_domain_init_cookie(struct super_block *sb,
> +							char *name, bool need_inode)
> +{
> +	struct inode *inode;
> +	struct erofs_fscache *ctx;
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	struct erofs_domain *domain = sbi->domain;
> +
> +	ctx = erofs_fscache_acquire_cookie(sb, name, need_inode);
> +	if (IS_ERR(ctx))
> +		return ctx;
> +
> +	ctx->name = kstrdup(name, GFP_KERNEL);
> +	if (!ctx->name)
> +		return ERR_PTR(-ENOMEM);

The previously registered erofs_fscache needs to be cleaned up in the
error path.

> +
> +	inode = new_inode(erofs_pseudo_mnt->mnt_sb);
> +	if (!inode) {
> +		kfree(ctx->name);
> +		return ERR_PTR(-ENOMEM);
> +	}

Ditto.

> +
> +	ctx->domain = domain;
> +	ctx->anon_inode = inode;
> +	inode->i_private = ctx;
> +	erofs_fscache_domain_get(domain);
> +	return ctx;
> +}
> +
> +static
> +struct erofs_fscache *erofs_domain_register_cookie(struct super_block *sb,
> +						    char *name, bool need_inode)
> +{
> +	struct inode *inode;
> +	struct erofs_fscache *ctx;
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	struct erofs_domain *domain = sbi->domain;
> +	struct super_block *psb = erofs_pseudo_mnt->mnt_sb;
> +
> +	mutex_lock(&erofs_domain_cookies_lock);
> +	list_for_each_entry(inode, &psb->s_inodes, i_sb_list) {
> +		ctx = inode->i_private;
> +		if (!ctx)
> +			continue;
> +		if (ctx->domain == domain && !strcmp(ctx->name, name)) {
> +			igrab(inode);
> +			mutex_unlock(&erofs_domain_cookies_lock);
> +			return ctx;
> +		}
> +	}
> +	ctx = erofs_fscache_domain_init_cookie(sb, name, need_inode);
> +	mutex_unlock(&erofs_domain_cookies_lock);
> +	return ctx;
> +}
> +
> +struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
> +						     char *name, bool need_inode)
> +{
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +
> +	if (sbi->opt.domain_id)
> +		return erofs_domain_register_cookie(sb, name, need_inode);
> +	else
> +		return erofs_fscache_acquire_cookie(sb, name, need_inode);
> +}
> +
>  void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
>  {
> +	struct erofs_domain *domain;
> +
>  	if (!ctx)
>  		return;
> +	domain = ctx->domain;
> +	if (domain) {
> +		mutex_lock(&erofs_domain_cookies_lock);
> +		/* Cookie is still in use */
> +		if (atomic_read(&ctx->anon_inode->i_count) > 1) {
> +			iput(ctx->anon_inode);
> +			mutex_unlock(&erofs_domain_cookies_lock);
> +			return;
> +		}
> +		iput(ctx->anon_inode);
> +		kfree(ctx->name);
> +		mutex_unlock(&erofs_domain_cookies_lock);

		mutex_lock(&erofs_domain_cookies_lock);
		drop = atomic_read(&ctx->anon_inode->i_count) == 1;
		iput(ctx->anon_inode);
		mutex_unlock(&erofs_domain_cookies_lock);

		if (!drop)
			return;
> +	}
>  >  	fscache_unuse_cookie(ctx->cookie, NULL, NULL);
>  	fscache_relinquish_cookie(ctx->cookie, false);
> +	erofs_fscache_domain_put(domain);
>  	ctx->cookie = NULL;

	fscache_unuse_cookie(ctx->cookie, NULL, NULL);
	fscache_relinquish_cookie(ctx->cookie, false);
	erofs_fscache_domain_put(domain);
	kfree(ctx->name);

>  
>  	iput(ctx->inode);
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 4dd0b545755a..8a6f94b27a23 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -101,7 +101,6 @@ struct erofs_sb_lz4_info {
>  
>  struct erofs_domain {
>  	refcount_t ref;
> -	struct mutex mutex;
>  	struct list_head list;
>  	struct fscache_volume *volume;
>  	char *domain_id;
> @@ -110,6 +109,9 @@ struct erofs_domain {
>  struct erofs_fscache {
>  	struct fscache_cookie *cookie;
>  	struct inode *inode;
> +	struct inode *anon_inode;
> +	struct erofs_domain *domain;
> +	char *name;
>  };
>  
>  struct erofs_sb_info {

-- 
Thanks,
Jingbo

Re: [External] Re: [PATCH V3 6/6] erofs: Support sharing cookies in the same domain

[Jia Zhu]

在 2022/9/15 14:53, JeffleXu 写道:
> 
> 
> On 9/14/22 6:50 PM, Jia Zhu wrote:
>> Several erofs filesystems can belong to one domain, and data blobs can
>> be shared among these erofs filesystems of same domain.
>>
>> Users could specify domain_id mount option to create or join into a
>> domain.
>>
>> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
>> ---
>>   fs/erofs/fscache.c  | 89 +++++++++++++++++++++++++++++++++++++++++++--
>>   fs/erofs/internal.h |  4 +-
>>   2 files changed, 89 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
>> index 4e0a441afb7d..e9ae1ee963e2 100644
>> --- a/fs/erofs/fscache.c
>> +++ b/fs/erofs/fscache.c
>> @@ -7,6 +7,7 @@
>>   #include "internal.h"
>>   
>>   static DEFINE_MUTEX(erofs_domain_list_lock);
>> +static DEFINE_MUTEX(erofs_domain_cookies_lock);
>>   static LIST_HEAD(erofs_domain_list);
>>   static struct vfsmount *erofs_pseudo_mnt;
>>   
>> @@ -504,7 +505,6 @@ static int erofs_fscache_init_domain(struct super_block *sb)
>>   
>>   	domain->volume = sbi->volume;
>>   	refcount_set(&domain->ref, 1);
>> -	mutex_init(&domain->mutex);
> 
> This needs to be folded into patch 4.
Thanks.
> 
> 
>>   	list_add(&domain->list, &erofs_domain_list);
>>   	return 0;
>>   out:
>> @@ -534,8 +534,8 @@ static int erofs_fscache_register_domain(struct super_block *sb)
>>   	return err;
>>   }
>>   
>> -struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
>> -						     char *name, bool need_inode)
>> +struct erofs_fscache *erofs_fscache_acquire_cookie(struct super_block *sb,
>> +						    char *name, bool need_inode)
>>   {
>>   	struct fscache_volume *volume = EROFS_SB(sb)->volume;
>>   	struct erofs_fscache *ctx;
>> @@ -585,13 +585,96 @@ struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
>>   	return ERR_PTR(ret);
>>   }
>>   
>> +static
>> +struct erofs_fscache *erofs_fscache_domain_init_cookie(struct super_block *sb,
>> +							char *name, bool need_inode)
>> +{
>> +	struct inode *inode;
>> +	struct erofs_fscache *ctx;
>> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
>> +	struct erofs_domain *domain = sbi->domain;
>> +
>> +	ctx = erofs_fscache_acquire_cookie(sb, name, need_inode);
>> +	if (IS_ERR(ctx))
>> +		return ctx;
>> +
>> +	ctx->name = kstrdup(name, GFP_KERNEL);
>> +	if (!ctx->name)
>> +		return ERR_PTR(-ENOMEM);
> 
> The previously registered erofs_fscache needs to be cleaned up in the
> error path.
Thanks for catching this. I'll fix it in next version.
> 
>> +
>> +	inode = new_inode(erofs_pseudo_mnt->mnt_sb);
>> +	if (!inode) {
>> +		kfree(ctx->name);
>> +		return ERR_PTR(-ENOMEM);
>> +	}
> 
> Ditto.
> 
>> +
>> +	ctx->domain = domain;
>> +	ctx->anon_inode = inode;
>> +	inode->i_private = ctx;
>> +	erofs_fscache_domain_get(domain);
>> +	return ctx;
>> +}
>> +
>> +static
>> +struct erofs_fscache *erofs_domain_register_cookie(struct super_block *sb,
>> +						    char *name, bool need_inode)
>> +{
>> +	struct inode *inode;
>> +	struct erofs_fscache *ctx;
>> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
>> +	struct erofs_domain *domain = sbi->domain;
>> +	struct super_block *psb = erofs_pseudo_mnt->mnt_sb;
>> +
>> +	mutex_lock(&erofs_domain_cookies_lock);
>> +	list_for_each_entry(inode, &psb->s_inodes, i_sb_list) {
>> +		ctx = inode->i_private;
>> +		if (!ctx)
>> +			continue;
>> +		if (ctx->domain == domain && !strcmp(ctx->name, name)) {
>> +			igrab(inode);
>> +			mutex_unlock(&erofs_domain_cookies_lock);
>> +			return ctx;
>> +		}
>> +	}
>> +	ctx = erofs_fscache_domain_init_cookie(sb, name, need_inode);
>> +	mutex_unlock(&erofs_domain_cookies_lock);
>> +	return ctx;
>> +}
>> +
>> +struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
>> +						     char *name, bool need_inode)
>> +{
>> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
>> +
>> +	if (sbi->opt.domain_id)
>> +		return erofs_domain_register_cookie(sb, name, need_inode);
>> +	else
>> +		return erofs_fscache_acquire_cookie(sb, name, need_inode);
>> +}
>> +
>>   void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
>>   {
>> +	struct erofs_domain *domain;
>> +
>>   	if (!ctx)
>>   		return;
>> +	domain = ctx->domain;
>> +	if (domain) {
>> +		mutex_lock(&erofs_domain_cookies_lock);
>> +		/* Cookie is still in use */
>> +		if (atomic_read(&ctx->anon_inode->i_count) > 1) {
>> +			iput(ctx->anon_inode);
>> +			mutex_unlock(&erofs_domain_cookies_lock);
>> +			return;
>> +		}
>> +		iput(ctx->anon_inode);
>> +		kfree(ctx->name);
>> +		mutex_unlock(&erofs_domain_cookies_lock);
> 
> 		mutex_lock(&erofs_domain_cookies_lock);
> 		drop = atomic_read(&ctx->anon_inode->i_count) == 1;
> 		iput(ctx->anon_inode);
> 		mutex_unlock(&erofs_domain_cookies_lock);
> 
> 		if (!drop)
> 			return;
This code style is more intuitive, I'll revise it, thanks.
>> +	}
>>   >  	fscache_unuse_cookie(ctx->cookie, NULL, NULL);
>>   	fscache_relinquish_cookie(ctx->cookie, false);
>> +	erofs_fscache_domain_put(domain);
>>   	ctx->cookie = NULL;
> 
> 	fscache_unuse_cookie(ctx->cookie, NULL, NULL);
> 	fscache_relinquish_cookie(ctx->cookie, false);
> 	erofs_fscache_domain_put(domain);
> 	kfree(ctx->name);
> 
>>   
>>   	iput(ctx->inode);
>> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
>> index 4dd0b545755a..8a6f94b27a23 100644
>> --- a/fs/erofs/internal.h
>> +++ b/fs/erofs/internal.h
>> @@ -101,7 +101,6 @@ struct erofs_sb_lz4_info {
>>   
>>   struct erofs_domain {
>>   	refcount_t ref;
>> -	struct mutex mutex;
>>   	struct list_head list;
>>   	struct fscache_volume *volume;
>>   	char *domain_id;
>> @@ -110,6 +109,9 @@ struct erofs_domain {
>>   struct erofs_fscache {
>>   	struct fscache_cookie *cookie;
>>   	struct inode *inode;
>> +	struct inode *anon_inode;
>> +	struct erofs_domain *domain;
>> +	char *name;
>>   };
>>   
>>   struct erofs_sb_info {
> 

Re: [PATCH V3 3/6] erofs: introduce 'domain_id' mount option

[JeffleXu]

On 9/14/22 6:50 PM, Jia Zhu wrote:
> Introduce 'domain_id' mount option to enable shared domain sementics.
> In which case, the related cookie is shared if two mountpoints in the
> same domain have the same data blob. Users could specify the name of
> domain by this mount option.
> 
> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>

Could you please move this patch to the end of this patch set, so that
once the "domain_id" mount option is visible to users, this feature
really works?


> ---
>  fs/erofs/internal.h |  1 +
>  fs/erofs/super.c    | 17 +++++++++++++++++
>  fs/erofs/sysfs.c    | 19 +++++++++++++++++--
>  3 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index aa71eb65e965..2d129c6b3027 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -76,6 +76,7 @@ struct erofs_mount_opts {
>  #endif
>  	unsigned int mount_opt;
>  	char *fsid;
> +	char *domain_id;
>  };

Indeed we can add @domain_id field into struct erofs_mount_opts in prep
for the following implementation of the domain_id feature. IOW, the
above change can be folded into patch 4, just like what [1] does.

[1]
https://github.com/torvalds/linux/commit/c6be2bd0a5dd91f98d6b5d2df2c79bc32993352c#diff-eee5fb30f4e83505af808386e84c953266d2fd2e76b6e66cb94cf6e849881240R77


>  
>  struct erofs_dev_context {
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 7aa57dcebf31..856758ee4869 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -440,6 +440,7 @@ enum {
>  	Opt_dax_enum,
>  	Opt_device,
>  	Opt_fsid,
> +	Opt_domain_id,
>  	Opt_err
>  };
>  
> @@ -465,6 +466,7 @@ static const struct fs_parameter_spec erofs_fs_parameters[] = {
>  	fsparam_enum("dax",		Opt_dax_enum, erofs_dax_param_enums),
>  	fsparam_string("device",	Opt_device),
>  	fsparam_string("fsid",		Opt_fsid),
> +	fsparam_string("domain_id",	Opt_domain_id),
>  	{}
>  };
>  
> @@ -568,6 +570,16 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>  			return -ENOMEM;
>  #else
>  		errorfc(fc, "fsid option not supported");
> +#endif
> +		break;
> +	case Opt_domain_id:
> +#ifdef CONFIG_EROFS_FS_ONDEMAND
> +		kfree(ctx->opt.domain_id);
> +		ctx->opt.domain_id = kstrdup(param->string, GFP_KERNEL);
> +		if (!ctx->opt.domain_id)
> +			return -ENOMEM;
> +#else
> +		errorfc(fc, "domain_id option not supported");
>  #endif
>  		break;
>  	default:
> @@ -695,6 +707,7 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>  	sb->s_fs_info = sbi;
>  	sbi->opt = ctx->opt;
>  	ctx->opt.fsid = NULL;
> +	ctx->opt.domain_id = NULL;
>  	sbi->devs = ctx->devs;
>  	ctx->devs = NULL;
>  
> @@ -834,6 +847,7 @@ static void erofs_fc_free(struct fs_context *fc)
>  
>  	erofs_free_dev_context(ctx->devs);
>  	kfree(ctx->opt.fsid);
> +	kfree(ctx->opt.domain_id);
>  	kfree(ctx);
>  }
>  
> @@ -887,6 +901,7 @@ static void erofs_kill_sb(struct super_block *sb)
>  	fs_put_dax(sbi->dax_dev, NULL);
>  	erofs_fscache_unregister_fs(sb);
>  	kfree(sbi->opt.fsid);
> +	kfree(sbi->opt.domain_id);
>  	kfree(sbi);
>  	sb->s_fs_info = NULL;
>  }
> @@ -1040,6 +1055,8 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
>  #ifdef CONFIG_EROFS_FS_ONDEMAND
>  	if (opt->fsid)
>  		seq_printf(seq, ",fsid=%s", opt->fsid);
> +	if (opt->domain_id)
> +		seq_printf(seq, ",domain_id=%s", opt->domain_id);
>  #endif
>  	return 0;
>  }
> diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
> index c1383e508bbe..341fb43ad587 100644
> --- a/fs/erofs/sysfs.c
> +++ b/fs/erofs/sysfs.c
> @@ -201,12 +201,27 @@ static struct kobject erofs_feat = {
>  int erofs_register_sysfs(struct super_block *sb)
>  {
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	char *name;
> +	char *str = NULL;
>  	int err;
>  
> +	if (erofs_is_fscache_mode(sb)) {
> +		if (sbi->opt.domain_id) {
> +			str = kasprintf(GFP_KERNEL, "%s,%s", sbi->opt.domain_id,
> +					sbi->opt.fsid);
> +			if (!str)
> +				return -ENOMEM;
> +			name = str;
> +		} else {
> +			name = sbi->opt.fsid;
> +		}
> +	} else {
> +		name = sb->s_id;
> +	}
>  	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",
> -			erofs_is_fscache_mode(sb) ? sbi->opt.fsid : sb->s_id);
> +	err = kobject_init_and_add(&sbi->s_kobj, &erofs_sb_ktype, NULL, "%s", name);
> +	kfree(str);
>  	if (err)
>  		goto put_sb_kobj;
>  	return 0;

-- 
Thanks,
Jingbo

Re: [External] Re: [PATCH V3 3/6] erofs: introduce 'domain_id' mount option

[Jia Zhu]

在 2022/9/15 15:30, JeffleXu 写道:
> 
> 
> On 9/14/22 6:50 PM, Jia Zhu wrote:
>> Introduce 'domain_id' mount option to enable shared domain sementics.
>> In which case, the related cookie is shared if two mountpoints in the
>> same domain have the same data blob. Users could specify the name of
>> domain by this mount option.
>>
>> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
> 
> Could you please move this patch to the end of this patch set, so that
> once the "domain_id" mount option is visible to users, this feature
> really works?
> 
Thanks for your example. I'll revise it.
> 
>> ---
>>   fs/erofs/internal.h |  1 +
>>   fs/erofs/super.c    | 17 +++++++++++++++++
>>   fs/erofs/sysfs.c    | 19 +++++++++++++++++--
>>   3 files changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
>> index aa71eb65e965..2d129c6b3027 100644
>> --- a/fs/erofs/internal.h
>> +++ b/fs/erofs/internal.h
>> @@ -76,6 +76,7 @@ struct erofs_mount_opts {
>>   #endif
>>   	unsigned int mount_opt;
>>   	char *fsid;
>> +	char *domain_id;
>>   };
> 
> Indeed we can add @domain_id field into struct erofs_mount_opts in prep
> for the following implementation of the domain_id feature. IOW, the
> above change can be folded into patch 4, just like what [1] does.
> 
> [1]
> https://github.com/torvalds/linux/commit/c6be2bd0a5dd91f98d6b5d2df2c79bc32993352c#diff-eee5fb30f4e83505af808386e84c953266d2fd2e76b6e66cb94cf6e849881240R77
> 
> 
>>   
>>   struct erofs_dev_context {
>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>> index 7aa57dcebf31..856758ee4869 100644
>> --- a/fs/erofs/super.c
>> +++ b/fs/erofs/super.c
>> @@ -440,6 +440,7 @@ enum {
>>   	Opt_dax_enum,
>>   	Opt_device,
>>   	Opt_fsid,
>> +	Opt_domain_id,
>>   	Opt_err
>>   };
>>   
>> @@ -465,6 +466,7 @@ static const struct fs_parameter_spec erofs_fs_parameters[] = {
>>   	fsparam_enum("dax",		Opt_dax_enum, erofs_dax_param_enums),
>>   	fsparam_string("device",	Opt_device),
>>   	fsparam_string("fsid",		Opt_fsid),
>> +	fsparam_string("domain_id",	Opt_domain_id),
>>   	{}
>>   };
>>   
>> @@ -568,6 +570,16 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>>   			return -ENOMEM;
>>   #else
>>   		errorfc(fc, "fsid option not supported");
>> +#endif
>> +		break;
>> +	case Opt_domain_id:
>> +#ifdef CONFIG_EROFS_FS_ONDEMAND
>> +		kfree(ctx->opt.domain_id);
>> +		ctx->opt.domain_id = kstrdup(param->string, GFP_KERNEL);
>> +		if (!ctx->opt.domain_id)
>> +			return -ENOMEM;
>> +#else
>> +		errorfc(fc, "domain_id option not supported");
>>   #endif
>>   		break;
>>   	default:
>> @@ -695,6 +707,7 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>>   	sb->s_fs_info = sbi;
>>   	sbi->opt = ctx->opt;
>>   	ctx->opt.fsid = NULL;
>> +	ctx->opt.domain_id = NULL;
>>   	sbi->devs = ctx->devs;
>>   	ctx->devs = NULL;
>>   
>> @@ -834,6 +847,7 @@ static void erofs_fc_free(struct fs_context *fc)
>>   
>>   	erofs_free_dev_context(ctx->devs);
>>   	kfree(ctx->opt.fsid);
>> +	kfree(ctx->opt.domain_id);
>>   	kfree(ctx);
>>   }
>>   
>> @@ -887,6 +901,7 @@ static void erofs_kill_sb(struct super_block *sb)
>>   	fs_put_dax(sbi->dax_dev, NULL);
>>   	erofs_fscache_unregister_fs(sb);
>>   	kfree(sbi->opt.fsid);
>> +	kfree(sbi->opt.domain_id);
>>   	kfree(sbi);
>>   	sb->s_fs_info = NULL;
>>   }
>> @@ -1040,6 +1055,8 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
>>   #ifdef CONFIG_EROFS_FS_ONDEMAND
>>   	if (opt->fsid)
>>   		seq_printf(seq, ",fsid=%s", opt->fsid);
>> +	if (opt->domain_id)
>> +		seq_printf(seq, ",domain_id=%s", opt->domain_id);
>>   #endif
>>   	return 0;
>>   }
>> diff --git a/fs/erofs/sysfs.c b/fs/erofs/sysfs.c
>> index c1383e508bbe..341fb43ad587 100644
>> --- a/fs/erofs/sysfs.c
>> +++ b/fs/erofs/sysfs.c
>> @@ -201,12 +201,27 @@ static struct kobject erofs_feat = {
>>   int erofs_register_sysfs(struct super_block *sb)
>>   {
>>   	struct erofs_sb_info *sbi = EROFS_SB(sb);
>> +	char *name;
>> +	char *str = NULL;
>>   	int err;
>>   
>> +	if (erofs_is_fscache_mode(sb)) {
>> +		if (sbi->opt.domain_id) {
>> +			str = kasprintf(GFP_KERNEL, "%s,%s", sbi->opt.domain_id,
>> +					sbi->opt.fsid);
>> +			if (!str)
>> +				return -ENOMEM;
>> +			name = str;
>> +		} else {
>> +			name = sbi->opt.fsid;
>> +		}
>> +	} else {
>> +		name = sb->s_id;
>> +	}
>>   	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",
>> -			erofs_is_fscache_mode(sb) ? sbi->opt.fsid : sb->s_id);
>> +	err = kobject_init_and_add(&sbi->s_kobj, &erofs_sb_ktype, NULL, "%s", name);
>> +	kfree(str);
>>   	if (err)
>>   		goto put_sb_kobj;
>>   	return 0;
> 

Re: [External] Re: [PATCH V3 4/6] erofs: introduce fscache-based domain

[Jia Zhu]

在 2022/9/14 21:21, Gao Xiang 写道:
> On Wed, Sep 14, 2022 at 06:50:39PM +0800, Jia Zhu wrote:
>> A new fscache-based shared domain mode is going to be introduced for
>> erofs. In which case, same data blobs in same domain will be shared
>> and reused to reduce on-disk space usage.
>>
>> As the first step, we use pseudo mnt to manage and maintain domain's
>> lifecycle.
>>
>> The implementation of sharing blobs will be introduced in subsequent
>> patches.
>>
>> Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
>> ---
>>   fs/erofs/fscache.c  | 134 ++++++++++++++++++++++++++++++++++++++------
>>   fs/erofs/internal.h |   9 +++
>>   2 files changed, 127 insertions(+), 16 deletions(-)
>>
>> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
>> index 4159cf781924..b2100dc67cde 100644
>> --- a/fs/erofs/fscache.c
>> +++ b/fs/erofs/fscache.c
>> @@ -1,10 +1,14 @@
>>   // SPDX-License-Identifier: GPL-2.0-or-later
>>   /*
>>    * Copyright (C) 2022, Alibaba Cloud
>> + * Copyright (C) 2022, Bytedance Inc. All rights reserved.
>>    */
>>   #include <linux/fscache.h>
>>   #include "internal.h"
>>   
>> +static DEFINE_MUTEX(erofs_domain_list_lock);
>> +static LIST_HEAD(erofs_domain_list);
>> +
>>   static struct netfs_io_request *erofs_fscache_alloc_request(struct address_space *mapping,
>>   					     loff_t start, size_t len)
>>   {
>> @@ -417,6 +421,106 @@ const struct address_space_operations erofs_fscache_access_aops = {
>>   	.readahead = erofs_fscache_readahead,
>>   };
>>   
>> +static
>> +struct erofs_domain *erofs_fscache_domain_get(struct erofs_domain *domain)
>> +{
>> +	refcount_inc(&domain->ref);
>> +	return domain;
>> +}
> 
> We can just open-code that.
Thanks, I'll revise it.
> 
>> +
>> +static void erofs_fscache_domain_put(struct erofs_domain *domain)
>> +{
>> +	if (!domain)
>> +		return;
>> +	if (refcount_dec_and_test(&domain->ref)) {
>> +		fscache_relinquish_volume(domain->volume, NULL, false);
>> +		mutex_lock(&erofs_domain_list_lock);
>> +		list_del(&domain->list);
>> +		mutex_unlock(&erofs_domain_list_lock);
>> +		kfree(domain->domain_id);
>> +		kfree(domain);
>> +	}
>> +}
>> +
>> +static int erofs_fscache_register_volume(struct super_block *sb)
>> +{
>> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
>> +	char *domain_id = sbi->opt.domain_id;
>> +	struct fscache_volume *volume;
>> +	char *name;
>> +	int ret = 0;
>> +
>> +	if (domain_id)
>> +		name = kasprintf(GFP_KERNEL, "erofs,%s", domain_id);
>> +	else
>> +		name = kasprintf(GFP_KERNEL, "erofs,%s", sbi->opt.fsid);
> 
> I think we could just use
> 
> 	name = kasprintf(GFP_KERNEL, "erofs,%s",
> 			 domain_id ? domain_id : sbi->opt.fsid);
> 
> here.
Thanks.
> 
>> +	if (!name)
>> +		return -ENOMEM;
>> +
>> +	volume = fscache_acquire_volume(name, NULL, NULL, 0);
>> +	if (IS_ERR_OR_NULL(volume)) {
>> +		erofs_err(sb, "failed to register volume for %s", name);
>> +		ret = volume ? PTR_ERR(volume) : -EOPNOTSUPP;
>> +		volume = NULL;
>> +	}
>> +
>> +	sbi->volume = volume;
>> +	kfree(name);
>> +	return ret;
>> +}
>> +
>> +static int erofs_fscache_init_domain(struct super_block *sb)
>> +{
>> +	int err;
>> +	struct erofs_domain *domain;
>> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
>> +
>> +	domain = kzalloc(sizeof(struct erofs_domain), GFP_KERNEL);
>> +	if (!domain)
>> +		return -ENOMEM;
>> +
>> +	domain->domain_id = kstrdup(sbi->opt.domain_id, GFP_KERNEL);
>> +	if (!domain->domain_id) {
>> +		kfree(domain);
>> +		return -ENOMEM;
>> +	}
>> +	sbi->domain = domain;
>> +	err = erofs_fscache_register_volume(sb);
>> +	if (err)
>> +		goto out;
>> +
>> +	domain->volume = sbi->volume;
>> +	refcount_set(&domain->ref, 1);
>> +	mutex_init(&domain->mutex);
>> +	list_add(&domain->list, &erofs_domain_list);
>> +	return 0;
>> +out:
>> +	kfree(domain->domain_id);
>> +	kfree(domain);
>> +	sbi->domain = NULL;
>> +	return err;
>> +}
>> +
>> +static int erofs_fscache_register_domain(struct super_block *sb)
>> +{
>> +	int err;
>> +	struct erofs_domain *domain;
>> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
>> +
>> +	mutex_lock(&erofs_domain_list_lock);
>> +	list_for_each_entry(domain, &erofs_domain_list, list) {
>> +		if (!strcmp(domain->domain_id, sbi->opt.domain_id)) {
>> +			sbi->domain = erofs_fscache_domain_get(domain);
>> +			sbi->volume = domain->volume;
>> +			mutex_unlock(&erofs_domain_list_lock);
>> +			return 0;
>> +		}
>> +	}
>> +	err = erofs_fscache_init_domain(sb);
> 
> why introduce erofs_fscache_init_domain?
> 
To make error handling path simpler, avoid lots of 'goto' and 'unlock'
>> +	mutex_unlock(&erofs_domain_list_lock);
>> +	return err;
>> +}
>> +
>>   struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
>>   						     char *name, bool need_inode)
>>   {
>> @@ -486,24 +590,16 @@ void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
>>   int erofs_fscache_register_fs(struct super_block *sb)
>>   {
>>   	struct erofs_sb_info *sbi = EROFS_SB(sb);
>> -	struct fscache_volume *volume;
>>   	struct erofs_fscache *fscache;
>> -	char *name;
>> -	int ret = 0;
>> +	int ret;
>>   
>> -	name = kasprintf(GFP_KERNEL, "erofs,%s", sbi->opt.fsid);
>> -	if (!name)
>> -		return -ENOMEM;
>> +	if (sbi->opt.domain_id)
>> +		ret = erofs_fscache_register_domain(sb);
>> +	else
>> +		ret = erofs_fscache_register_volume(sb);
>>   
>> -	volume = fscache_acquire_volume(name, NULL, NULL, 0);
>> -	if (IS_ERR_OR_NULL(volume)) {
>> -		erofs_err(sb, "failed to register volume for %s", name);
>> -		ret = volume ? PTR_ERR(volume) : -EOPNOTSUPP;
>> -		volume = NULL;
>> -	}
>> -
>> -	sbi->volume = volume;
>> -	kfree(name);
>> +	if (ret)
>> +		return ret;
>>   
>>   	fscache = erofs_fscache_register_cookie(sb, sbi->opt.fsid, true);
>>   	if (IS_ERR(fscache))
>> @@ -518,7 +614,13 @@ void erofs_fscache_unregister_fs(struct super_block *sb)
>>   	struct erofs_sb_info *sbi = EROFS_SB(sb);
>>   
>>   	erofs_fscache_unregister_cookie(sbi->s_fscache);
>> -	fscache_relinquish_volume(sbi->volume, NULL, false);
>>   	sbi->s_fscache = NULL;
>> +
>> +	if (sbi->domain)
>> +		erofs_fscache_domain_put(sbi->domain);
>> +	else
>> +		fscache_relinquish_volume(sbi->volume, NULL, false);
>> +
> 
> How about using one helper and pass in sb directly instead?
Thanks for your suggestion, I'll revise it in next version.
> 
> Thanks,
> Gao Xiang