| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <29af5dbc-2bbe-d550-9fde-6c799a302b7c@bytedance.com>
Date: Wed, 8 Feb 2023 17:01:36 +0800
From: Jia Zhu <zhujia.zj@...edance.com>
To: Jingbo Xu <jefflexu@...ux.alibaba.com>, xiang@...nel.org,
chao@...nel.org, linux-erofs@...ts.ozlabs.org
Cc: huyue2@...lpad.com, linux-kernel@...r.kernel.org,
zhujia.zj@...edance.com
Subject: Re: [External] [PATCH 3/4] erofs: unify anonymous inodes for blob
在 2023/2/8 15:16, Jingbo Xu 写道:
> Currently there're two anonymous inodes (inode and anon_inode in struct
> erofs_fscache) for each blob. The former was introduced as the
> address_space of page cache for bootstrap.
>
> The latter was initially introduced as both the address_space of page
> cache and also a sentinel in the shared domain. Since now the management
> of cookies in share domain has been decoupled with the anonymous inode,
> there's no need to maintain an extra anonymous inode. Let's unify these
> two anonymous inodes.
>
> Besides, in non-share-domain mode only bootstrap will allocate anonymous
> inode. To simplify the implementation, always allocate anonymous inode
> for both bootstrap and data blobs. Similarly release anonymous inodes
> for data blobs when .put_super() is called, or we'll get "VFS: Busy
> inodes after unmount." warning.
>
> Also remove the redundant set_nlink() when initializing the anonymous
> inode, since i_nlink has already been initialized to 1 when the inode
> gets allocated.
>
> Signed-off-by: Jingbo Xu <jefflexu@...ux.alibaba.com>
> ---
> fs/erofs/fscache.c | 70 ++++++++++++++++-----------------------------
> fs/erofs/internal.h | 6 ++--
> fs/erofs/super.c | 2 ++
> 3 files changed, 28 insertions(+), 50 deletions(-)
>
> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
> index 2f5930e177cc..8353a5fe8c71 100644
> --- a/fs/erofs/fscache.c
> +++ b/fs/erofs/fscache.c
> @@ -424,12 +424,12 @@ static int erofs_fscache_register_domain(struct super_block *sb)
>
> static
> struct erofs_fscache *erofs_fscache_acquire_cookie(struct super_block *sb,
> - char *name,
> - unsigned int flags)
> + struct super_block *isb, char *name)
> {
> struct fscache_volume *volume = EROFS_SB(sb)->volume;
> struct erofs_fscache *ctx;
> struct fscache_cookie *cookie;
> + struct inode *inode;
> int ret;
>
> ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> @@ -445,33 +445,27 @@ struct erofs_fscache *erofs_fscache_acquire_cookie(struct super_block *sb,
> ret = -EINVAL;
> goto err;
> }
> -
> fscache_use_cookie(cookie, false);
> - ctx->cookie = cookie;
> -
> - if (flags & EROFS_REG_COOKIE_NEED_INODE) {
> - struct inode *const inode = new_inode(sb);
> -
> - if (!inode) {
> - erofs_err(sb, "failed to get anon inode for %s", name);
> - ret = -ENOMEM;
> - goto err_cookie;
> - }
> -
> - set_nlink(inode, 1);
> - inode->i_size = OFFSET_MAX;
> - inode->i_mapping->a_ops = &erofs_fscache_meta_aops;
> - mapping_set_gfp_mask(inode->i_mapping, GFP_NOFS);
> - inode->i_private = ctx;
>
> - ctx->inode = inode;
> + inode = new_inode(isb);
> + if (!inode) {
> + erofs_err(sb, "failed to get anon inode for %s", name);
> + ret = -ENOMEM;
> + goto err_cookie;
> }
>
> + inode->i_size = OFFSET_MAX;
> + inode->i_mapping->a_ops = &erofs_fscache_meta_aops;
> + mapping_set_gfp_mask(inode->i_mapping, GFP_NOFS);
> + inode->i_private = ctx;
> +
> + ctx->cookie = cookie;
> + ctx->inode = inode;
> return ctx;
>
> err_cookie:
> - fscache_unuse_cookie(ctx->cookie, NULL, NULL);
> - fscache_relinquish_cookie(ctx->cookie, false);
> + fscache_unuse_cookie(cookie, NULL, NULL);
> + fscache_relinquish_cookie(cookie, false);
> err:
> kfree(ctx);
> return ERR_PTR(ret);
> @@ -482,46 +476,31 @@ 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);
> }
>
> static
> struct erofs_fscache *erofs_fscache_domain_init_cookie(struct super_block *sb,
> - char *name,
> - unsigned int flags)
> + char *name)
> {
> - int err;
> - struct inode *inode;
> struct erofs_fscache *ctx;
> struct erofs_domain *domain = EROFS_SB(sb)->domain;
>
> - ctx = erofs_fscache_acquire_cookie(sb, name, flags);
> + ctx = erofs_fscache_acquire_cookie(sb, erofs_pseudo_mnt->mnt_sb, name);
> if (IS_ERR(ctx))
> return ctx;
>
> ctx->name = kstrdup(name, GFP_KERNEL);
> if (!ctx->name) {
> - err = -ENOMEM;
> - goto out;
> - }
> -
> - inode = new_inode(erofs_pseudo_mnt->mnt_sb);
> - if (!inode) {
> - err = -ENOMEM;
> - goto out;
> + erofs_fscache_relinquish_cookie(ctx);
> + return ERR_PTR(-ENOMEM);
> }
>
> + refcount_inc(&domain->ref);
> 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;
> -out:
> - erofs_fscache_relinquish_cookie(ctx);
> - return ERR_PTR(err);
> }
>
> static
> @@ -546,7 +525,7 @@ struct erofs_fscache *erofs_domain_register_cookie(struct super_block *sb,
> mutex_unlock(&erofs_domain_cookies_lock);
> return ctx;
> }
> - ctx = erofs_fscache_domain_init_cookie(sb, name, flags);
> + ctx = erofs_fscache_domain_init_cookie(sb, name);
> mutex_unlock(&erofs_domain_cookies_lock);
> return ctx;
> }
> @@ -557,7 +536,7 @@ struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
> {
> if (EROFS_SB(sb)->domain_id)
> return erofs_domain_register_cookie(sb, name, flags);
> - return erofs_fscache_acquire_cookie(sb, name, flags);
> + return erofs_fscache_acquire_cookie(sb, sb, name);
How about using <sb, flags> to indicate shared/non-shared domain
mode?
Besides, LGTM.
Reviewed-by: Jia Zhu <zhujia.zj@...edance.com>
> }
>
> void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
> @@ -585,7 +564,7 @@ int erofs_fscache_register_fs(struct super_block *sb)
> int ret;
> struct erofs_sb_info *sbi = EROFS_SB(sb);
> struct erofs_fscache *fscache;
> - unsigned int flags;
> + unsigned int flags = 0;
>
> if (sbi->domain_id)
> ret = erofs_fscache_register_domain(sb);
> @@ -604,7 +583,6 @@ int erofs_fscache_register_fs(struct super_block *sb)
> *
> * Acquired domain/volume will be relinquished in kill_sb() on error.
> */
> - flags = EROFS_REG_COOKIE_NEED_INODE;
> if (sbi->domain_id)
> flags |= EROFS_REG_COOKIE_NEED_NOEXIST;
> fscache = erofs_fscache_register_cookie(sb, sbi->fsid, flags);
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 8358cf5f731e..125e4aa8d295 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -107,8 +107,7 @@ struct erofs_domain {
>
> struct erofs_fscache {
> struct fscache_cookie *cookie;
> - struct inode *inode;
> - struct inode *anon_inode;
> + struct inode *inode; /* anonymous indoe for the blob */
>
> /* used for share domain mode */
> struct erofs_domain *domain;
> @@ -450,8 +449,7 @@ extern const struct file_operations erofs_dir_fops;
> extern const struct iomap_ops z_erofs_iomap_report_ops;
>
> /* flags for erofs_fscache_register_cookie() */
> -#define EROFS_REG_COOKIE_NEED_INODE 1
> -#define EROFS_REG_COOKIE_NEED_NOEXIST 2
> +#define EROFS_REG_COOKIE_NEED_NOEXIST 1
>
> void erofs_unmap_metabuf(struct erofs_buf *buf);
> void erofs_put_metabuf(struct erofs_buf *buf);
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 715efa94eed4..19b1ae79cec4 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -968,6 +968,8 @@ static void erofs_put_super(struct super_block *sb)
> iput(sbi->packed_inode);
> sbi->packed_inode = NULL;
> #endif
> + erofs_free_dev_context(sbi->devs);
> + sbi->devs = NULL;
> erofs_fscache_unregister_fs(sb);
> }
>
Powered by blists - more mailing lists