[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <23018405-bd62-45b6-d3d0-6f0acb5630f5@bytedance.com>
Date: Tue, 13 Sep 2022 20:59:47 +0800
From: Jia Zhu <zhujia.zj@...edance.com>
To: JeffleXu <jefflexu@...ux.alibaba.com>,
linux-erofs@...ts.ozlabs.org, xiang@...nel.org, chao@...nel.org
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
yinxin.x@...edance.com, huyue2@...lpad.com
Subject: Re: [External] Re: [PATCH V2 5/5] erofs: support fscache based shared
domain
在 2022/9/13 14:27, JeffleXu 写道:
>
>
> On 9/2/22 6:53 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@...edance.com>
>> ---
>> fs/erofs/fscache.c | 73 +++++++++++++++++++++++++++++++++++++++++++++
>> fs/erofs/internal.h | 12 ++++++++
>> fs/erofs/super.c | 10 +++++--
>> 3 files changed, 93 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
>> index 439dd3cc096a..c01845808ede 100644
>> --- a/fs/erofs/fscache.c
>> +++ b/fs/erofs/fscache.c
>> @@ -559,12 +559,27 @@ int erofs_fscache_register_cookie(struct super_block *sb,
>> void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache)
>> {
>> struct erofs_fscache *ctx = *fscache;
>> + struct erofs_domain *domain;
>>
>> if (!ctx)
>> return;
>> + domain = ctx->domain;
>> + if (domain) {
>> + mutex_lock(&domain->mutex);
>> + /* Cookie is still in use */
>> + if (atomic_read(&ctx->anon_inode->i_count) > 1) {
>> + iput(ctx->anon_inode);
>> + mutex_unlock(&domain->mutex);
>> + return;
>> + }
>> + iput(ctx->anon_inode);
>> + kfree(ctx->name);
>> + mutex_unlock(&domain->mutex);
>> + }
>>
>> fscache_unuse_cookie(ctx->cookie, NULL, NULL);
>> fscache_relinquish_cookie(ctx->cookie, false);
>> + erofs_fscache_domain_put(domain);
>> ctx->cookie = NULL;
>>
>> iput(ctx->inode);
>> @@ -609,3 +624,61 @@ void erofs_fscache_unregister_fs(struct super_block *sb)
>> sbi->volume = NULL;
>> sbi->domain = NULL;
>> }
>> +
>> +static int erofs_fscache_domain_init_cookie(struct super_block *sb,
>> + struct erofs_fscache **fscache, char *name, bool need_inode)
>> +{
>> + int ret;
>> + struct inode *inode;
>> + struct erofs_fscache *ctx;
>> + struct erofs_sb_info *sbi = EROFS_SB(sb);
>> + struct erofs_domain *domain = sbi->domain;
>> +
>> + ret = erofs_fscache_register_cookie(sb, &ctx, name, need_inode);
>> + if (ret)
>> + return ret;
>> +
>> + ctx->name = kstrdup(name, GFP_KERNEL);
>> + if (!ctx->name)
>> + return -ENOMEM;
>
> Shall we clean up the above registered cookie in the error path?
>
If this step fails, error will be transmitted to vfs_get_tree() and
erofs_kill_sb() will relinquish the cookie.
>> +
>> + inode = new_inode(erofs_pseudo_mnt->mnt_sb);
>> + if (!inode) {
>> + kfree(ctx->name);
>> + return -ENOMEM;
>> + }
>
> Ditto.
>
>> +
>> + ctx->domain = domain;
>> + ctx->anon_inode = inode;
>> + inode->i_private = ctx;
>> + erofs_fscache_domain_get(domain);
>> + *fscache = ctx;
>> + return 0;
>> +}
>> +
>> +int erofs_domain_register_cookie(struct super_block *sb,
>> + struct erofs_fscache **fscache, char *name, bool need_inode)
>> +{
>> + int err;
>> + 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(&domain->mutex);
>
> What is domain->mutex used for?
>
This lock is used to avoid race conditions between cookie's
traverse/del/insert in the inode list.
It seems to be possible to increase the granularity of the lock
after v2's change "Only initialize one pseudo fs to manage anonymous
inodes(cookies).".
>
>> + list_for_each_entry(inode, &psb->s_inodes, i_sb_list) {
>> + ctx = inode->i_private;
>> + if (!ctx)
>> + continue;
>> + if (!strcmp(ctx->name, name)) {
>> + *fscache = ctx;
>> + igrab(inode);
>> + mutex_unlock(&domain->mutex);
>> + return 0;
>> + }
>> + }
>> + err = erofs_fscache_domain_init_cookie(sb, fscache, name, need_inode);
>> + mutex_unlock(&domain->mutex);
>> + return err;
>> +}
>> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
>> index 2790c93ffb83..efa4f4ad77cc 100644
>> --- a/fs/erofs/internal.h
>> +++ b/fs/erofs/internal.h
>> @@ -110,6 +110,9 @@ struct erofs_domain {
>> struct erofs_fscache {
>> struct fscache_cookie *cookie;
>> struct inode *inode;
>> + struct inode *anon_inode;
>
> Why can't we reuse @inode for anon_inode?
>
We use pseudo sb's anonymous inodes list to manage the cookie.
Wouldn't it be a bit of messy if reuses erofs meta inode?
>
>> + struct erofs_domain *domain;
>> + char *name;
>> };
>>
>> struct erofs_sb_info {
>> @@ -625,6 +628,9 @@ int erofs_fscache_register_domain(struct super_block *sb);
>> int erofs_fscache_register_cookie(struct super_block *sb,
>> struct erofs_fscache **fscache,
>> char *name, bool need_inode);
>> +int erofs_domain_register_cookie(struct super_block *sb,
>> + struct erofs_fscache **fscache,
>> + char *name, bool need_inode);
>> void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache);
>>
>> extern const struct address_space_operations erofs_fscache_access_aops;
>> @@ -646,6 +652,12 @@ static inline int erofs_fscache_register_cookie(struct super_block *sb,
>> {
>> return -EOPNOTSUPP;
>> }
>> +static inline int erofs_domain_register_cookie(struct super_block *sb,
>> + struct erofs_fscache **fscache,
>> + char *name, bool need_inode)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>>
>> static inline void erofs_fscache_unregister_cookie(struct erofs_fscache **fscache)
>> {
>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>> index 667a78f0ee70..11c5ba84567c 100644
>> --- a/fs/erofs/super.c
>> +++ b/fs/erofs/super.c
>> @@ -245,8 +245,12 @@ 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 (sbi->opt.domain_id)
>> + ret = erofs_domain_register_cookie(sb, &dif->fscache, dif->path,
>> + false);
>> + else
>> + ret = erofs_fscache_register_cookie(sb, &dif->fscache, dif->path,
>> + false);
>> if (ret)
>> return ret;
>> } else {
>> @@ -726,6 +730,8 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>> err = erofs_fscache_register_domain(sb);
>> if (err)
>> return err;
>> + err = erofs_domain_register_cookie(sb, &sbi->s_fscache,
>> + sbi->opt.fsid, true);
>> } else {
>> err = erofs_fscache_register_fs(sb);
>> if (err)
>
Powered by blists - more mailing lists