From: JeffleXu <jefflexu@linux.alibaba.com>
To: Jia Zhu <zhujia.zj@bytedance.com>,
linux-erofs@lists.ozlabs.org, xiang@kernel.org, chao@kernel.org
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
yinxin.x@bytedance.com, huyue2@coolpad.com
Subject: Re: [PATCH V3 2/6] erofs: code clean up for fscache
Date: Thu, 15 Sep 2022 10:26:35 +0800 [thread overview]
Message-ID: <6606b1d2-7612-2e9c-19f3-8790257219d3@linux.alibaba.com> (raw)
In-Reply-To: <20220914105041.42970-3-zhujia.zj@bytedance.com>
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
next prev parent reply other threads:[~2022-09-15 2:26 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-14 10:50 [PATCH V3 0/6] Introduce erofs shared domain Jia Zhu
2022-09-14 10:50 ` [PATCH V3 1/6] erofs: use kill_anon_super() to kill super in fscache mode Jia Zhu
2022-09-15 2:28 ` JeffleXu
2022-09-15 6:07 ` [External] " Jia Zhu
2022-09-14 10:50 ` [PATCH V3 2/6] erofs: code clean up for fscache Jia Zhu
2022-09-15 2:26 ` JeffleXu [this message]
2022-09-15 6:45 ` JeffleXu
2022-09-14 10:50 ` [PATCH V3 3/6] erofs: introduce 'domain_id' mount option Jia Zhu
2022-09-15 7:30 ` JeffleXu
2022-09-15 7:43 ` [External] " Jia Zhu
2022-09-14 10:50 ` [PATCH V3 4/6] erofs: introduce fscache-based domain Jia Zhu
2022-09-14 13:21 ` Gao Xiang
2022-09-15 3:29 ` JeffleXu
2022-09-15 8:00 ` [External] " Jia Zhu
2022-09-15 3:27 ` JeffleXu
2022-09-14 10:50 ` [PATCH V3 5/6] erofs: introduce a pseudo mnt to manage shared cookies Jia Zhu
2022-09-15 5:43 ` JeffleXu
2022-09-14 10:50 ` [PATCH V3 6/6] erofs: Support sharing cookies in the same domain Jia Zhu
2022-09-14 13:30 ` Gao Xiang
2022-09-15 6:53 ` JeffleXu
2022-09-15 7:26 ` [External] " Jia Zhu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6606b1d2-7612-2e9c-19f3-8790257219d3@linux.alibaba.com \
--to=jefflexu@linux.alibaba.com \
--cc=chao@kernel.org \
--cc=huyue2@coolpad.com \
--cc=linux-erofs@lists.ozlabs.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=xiang@kernel.org \
--cc=yinxin.x@bytedance.com \
--cc=zhujia.zj@bytedance.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®