Re: [PATCH v6 12/22] erofs: add cookie context helper functions

Gao Xiang
2022-03-25T13:41:54Z

Hi Jeffle,

On Fri, Mar 25, 2022 at 08:22:13PM +0800, Jeffle Xu wrote:
> Introduce "struct erofs_fscache" for managing cookie for backinig file,
> and helper functions for initializing and cleaning up this context
> structure.
>
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
> fs/erofs/fscache.c | 61 +++++++++++++++++++++++++++++++++++++++++++++
> fs/erofs/internal.h | 14 +++++++++++
> 2 files changed, 75 insertions(+)
>
> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
> index 08cf570a0810..73235fd43bf6 100644
> --- a/fs/erofs/fscache.c
> +++ b/fs/erofs/fscache.c
> @@ -7,6 +7,67 @@
>
> static struct fscache_volume *volume;
>
> +static int erofs_fscache_init_cookie(struct erofs_fscache *ctx, char *path)
> +{
> + struct fscache_cookie *cookie;
> +
> + cookie = fscache_acquire_cookie(volume, FSCACHE_ADV_WANT_CACHE_SIZE,
> + path, strlen(path),
> + NULL, 0, 0);

It'd be better to rearrange in one line?

path, strlen(path), NULL, 0, 0);

> + if (!cookie)
> + return -EINVAL;
> +
> + fscache_use_cookie(cookie, false);
> + ctx->cookie = cookie;
> + return 0;
> +}
> +
> +static inline void erofs_fscache_cleanup_cookie(struct erofs_fscache *ctx)
> +{
> + struct fscache_cookie *cookie = ctx->cookie;
> +
> + fscache_unuse_cookie(cookie, NULL, NULL);
> + fscache_relinquish_cookie(cookie, false);
> + ctx->cookie = NULL;
> +}
> +
> +/*
> + * erofs_fscache_get - create an fscache context for blob file
> + * @sb: superblock
> + * @path: name of blob file
> + *
> + * Return: fscache context on success, ERR_PTR() on failure.
> + */
> +struct erofs_fscache *erofs_fscache_get(struct super_block *sb, char *path)

erofs_fscache_register?

> +{
> + struct erofs_fscache *ctx;
> + int ret;
> +
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = erofs_fscache_init_cookie(ctx, path);

Can we fold it here? No need to introduce such simple wrapper..

> + if (ret) {
> + erofs_err(sb, "failed to init cookie");

It would be better to print the path?

> + goto err;

kfree(ctx);
return ERR_PTR(ret);

At least for now.

> + }
> +
> + return ctx;
> +err:
> + kfree(ctx);
> + return ERR_PTR(ret);
> +}
> +
> +void erofs_fscache_put(struct erofs_fscache *ctx)

erofs_fscache_unregister?

> +{
> + if (!ctx)
> + return;
> +
> + erofs_fscache_cleanup_cookie(ctx);

Fold it too, since such helper doesn't simplify code a lot but need
to take one more time to redirect..

Thanks,
Gao Xiang

> + kfree(ctx);
> +}
> +
> int __init erofs_init_fscache(void)
> {
> volume = fscache_acquire_volume("erofs", NULL, NULL, 0);
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 45b8b0dd8a27..d4f2b43cedae 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -96,6 +96,10 @@ struct erofs_sb_lz4_info {
> u16 max_pclusterblks;
> };
>
> +struct erofs_fscache {
> + struct fscache_cookie *cookie;
> +};
> +
> struct erofs_sb_info {
> struct erofs_mount_opts opt; /* options */
> #ifdef CONFIG_EROFS_FS_ZIP
> @@ -620,9 +624,19 @@ static inline int z_erofs_load_lzma_config(struct super_block *sb,
> #ifdef CONFIG_EROFS_FS_ONDEMAND
> int erofs_init_fscache(void);
> void erofs_exit_fscache(void);
> +
> +struct erofs_fscache *erofs_fscache_get(struct super_block *sb, char *path);
> +void erofs_fscache_put(struct erofs_fscache *ctx);
> #else
> static inline int erofs_init_fscache(void) { return 0; }
> static inline void erofs_exit_fscache(void) {}
> +
> +static inline struct erofs_fscache *erofs_fscache_get(struct super_block *sb,
> + char *path)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}
> +static inline void erofs_fscache_put(struct erofs_fscache *ctx) {}
> #endif
>
> #define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
> --
> 2.27.0