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

[Jia Zhu <zhujia.zj@xxxxxxxxxxxxx>]

在 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@xxxxxxxxxxxxx>
> > ---
> >   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 {