[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d495d8d1-c37c-7e24-160b-efd7633ab20a@linux.alibaba.com>
Date: Thu, 9 Feb 2023 14:08:46 +0800
From: Gao Xiang <hsiangkao@...ux.alibaba.com>
To: Jingbo Xu <jefflexu@...ux.alibaba.com>, xiang@...nel.org,
chao@...nel.org, linux-erofs@...ts.ozlabs.org,
zhujia.zj@...edance.com
Cc: huyue2@...lpad.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/4] erofs: maintain cookies of share domain in
self-contained list
On 2023/2/9 13:18, Jingbo Xu wrote:
> We'd better not touch sb->s_inodes list and inode->i_count directly.
> Let's maintain cookies of share domain in a self-contained list in erofs.
>
> Besides, relinquish cookie with the mutex held. Otherwise if a cookie
> is registered when the old cookie with the same name in the same domain
> has been removed from the list but not relinquished yet, fscache may
> complain "Duplicate cookie detected".
>
> Signed-off-by: Jingbo Xu <jefflexu@...ux.alibaba.com>
> Reviewed-by: Jia Zhu <zhujia.zj@...edance.com>
> ---
> fs/erofs/fscache.c | 48 ++++++++++++++++++++++-----------------------
> fs/erofs/internal.h | 4 ++++
> 2 files changed, 27 insertions(+), 25 deletions(-)
>
> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
> index 03de4dc99302..2f5930e177cc 100644
> --- a/fs/erofs/fscache.c
> +++ b/fs/erofs/fscache.c
> @@ -7,8 +7,11 @@
> #include "internal.h"
>
> static DEFINE_MUTEX(erofs_domain_list_lock);
> -static DEFINE_MUTEX(erofs_domain_cookies_lock);
> static LIST_HEAD(erofs_domain_list);
> +
> +static DEFINE_MUTEX(erofs_domain_cookies_lock);
> +static LIST_HEAD(erofs_domain_cookies_list);
Redundant blank lines.
> +
> static struct vfsmount *erofs_pseudo_mnt;
>
> struct erofs_fscache_request {
> @@ -318,8 +321,6 @@ const struct address_space_operations erofs_fscache_access_aops = {
>
> static void erofs_fscache_domain_put(struct erofs_domain *domain)
> {
> - if (!domain)
> - return;
> mutex_lock(&erofs_domain_list_lock);
> if (refcount_dec_and_test(&domain->ref)) {
> list_del(&domain->list);
> @@ -434,6 +435,8 @@ struct erofs_fscache *erofs_fscache_acquire_cookie(struct super_block *sb,
> ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> if (!ctx)
> return ERR_PTR(-ENOMEM);
> + INIT_LIST_HEAD(&ctx->node);
> + refcount_set(&ctx->ref, 1);
>
> cookie = fscache_acquire_cookie(volume, FSCACHE_ADV_WANT_CACHE_SIZE,
> name, strlen(name), NULL, 0, 0);
> @@ -479,6 +482,7 @@ static void erofs_fscache_relinquish_cookie(struct erofs_fscache *ctx)
> fscache_unuse_cookie(ctx->cookie, NULL, NULL);
> fscache_relinquish_cookie(ctx->cookie, false);
> iput(ctx->inode);
> + iput(ctx->anon_inode);
> kfree(ctx->name);
> kfree(ctx);
> }
> @@ -511,6 +515,7 @@ struct erofs_fscache *erofs_fscache_domain_init_cookie(struct super_block *sb,
>
> ctx->domain = domain;
> ctx->anon_inode = inode;
> + list_add(&ctx->node, &erofs_domain_cookies_list);
> inode->i_private = ctx;
> refcount_inc(&domain->ref);
> return ctx;
> @@ -524,29 +529,23 @@ struct erofs_fscache *erofs_domain_register_cookie(struct super_block *sb,
> char *name,
> unsigned int flags)
> {
> - struct inode *inode;
> struct erofs_fscache *ctx;
> struct erofs_domain *domain = EROFS_SB(sb)->domain;
> - struct super_block *psb = erofs_pseudo_mnt->mnt_sb;
>
> mutex_lock(&erofs_domain_cookies_lock);
> - spin_lock(&psb->s_inode_list_lock);
> - list_for_each_entry(inode, &psb->s_inodes, i_sb_list) {
> - ctx = inode->i_private;
> - if (!ctx || ctx->domain != domain || strcmp(ctx->name, name))
> + list_for_each_entry(ctx, &erofs_domain_cookies_list, node) {
> + if (ctx->domain != domain || strcmp(ctx->name, name))
> continue;
> if (!(flags & EROFS_REG_COOKIE_NEED_NOEXIST)) {
> - igrab(inode);
> + refcount_inc(&ctx->ref);
> } else {
> erofs_err(sb, "%s already exists in domain %s", name,
> domain->domain_id);
> ctx = ERR_PTR(-EEXIST);
> }
> - spin_unlock(&psb->s_inode_list_lock);
> mutex_unlock(&erofs_domain_cookies_lock);
> return ctx;
> }
> - spin_unlock(&psb->s_inode_list_lock);
> ctx = erofs_fscache_domain_init_cookie(sb, name, flags);
> mutex_unlock(&erofs_domain_cookies_lock);
> return ctx;
> @@ -563,23 +562,22 @@ struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
>
> void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
> {
> - bool drop;
> - struct erofs_domain *domain;
> + struct erofs_domain *domain = NULL;
>
> if (!ctx)
> return;
> - domain = ctx->domain;
> - if (domain) {
> - 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;
> - }
> + if (!ctx->domain)
> + return erofs_fscache_relinquish_cookie(ctx);
>
> - erofs_fscache_relinquish_cookie(ctx);
> - erofs_fscache_domain_put(domain);
> + mutex_lock(&erofs_domain_cookies_lock);
> + if (refcount_dec_and_test(&ctx->ref)) {
> + domain = ctx->domain;
> + list_del(&ctx->node);
> + erofs_fscache_relinquish_cookie(ctx);
> + }
> + mutex_unlock(&erofs_domain_cookies_lock);
> + if (domain)
> + erofs_fscache_domain_put(domain);
> }
>
> int erofs_fscache_register_fs(struct super_block *sb)
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 48a2f33de15a..8358cf5f731e 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -109,7 +109,11 @@ struct erofs_fscache {
> struct fscache_cookie *cookie;
> struct inode *inode;
> struct inode *anon_inode;
> +
> + /* used for share domain mode */
> struct erofs_domain *domain;
> + struct list_head node;
> + refcount_t ref;
> char *name;
> };
>
Powered by blists - more mailing lists