On 3/25/22 9:41 PM, Gao Xiang wrote: > 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@xxxxxxxxxxxxxxxxx> >> --- >> 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? Sure. > > 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? OK. > >> +{ >> + 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? OK. > >> + goto err; > > kfree(ctx); > return ERR_PTR(ret); > > At least for now. Yeah, it's better. > >> + } >> + >> + return ctx; >> +err: >> + kfree(ctx); >> + return ERR_PTR(ret); >> +} >> + >> +void erofs_fscache_put(struct erofs_fscache *ctx) > > erofs_fscache_unregister? OK. > >> +{ >> + 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.. OK. -- Thanks, Jeffle