* [PATCH -next] erofs: get rid of erofs_fs_context
@ 2024-04-19 3:14 Baokun Li via Linux-erofs
2024-04-19 10:13 ` Christian Brauner
0 siblings, 1 reply; 3+ messages in thread
From: Baokun Li via Linux-erofs @ 2024-04-19 3:14 UTC (permalink / raw)
To: linux-erofs; +Cc: brauner, yangerkun, linux-kernel, huyue2, viro
Instead of allocating the erofs_sb_info in get_tree() allocate it during
init_fs_context() and after this erofs_fs_context is no longer needed,
so replace ctx with sbi, no functional changes.
Suggested-by: Jingbo Xu <jefflexu@linux.alibaba.com>
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
Hi Gao Xiang,
Hi Jingbo,
I noticed that Christian's original patch has been merged into the next
branch, so I'm sending this patch out separately.
Regards,
Baokun
fs/erofs/internal.h | 7 ---
fs/erofs/super.c | 112 ++++++++++++++++++--------------------------
2 files changed, 46 insertions(+), 73 deletions(-)
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 116c1d5d1932..53ebba952a2f 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -84,13 +84,6 @@ struct erofs_dev_context {
bool flatdev;
};
-struct erofs_fs_context {
- struct erofs_mount_opts opt;
- struct erofs_dev_context *devs;
- char *fsid;
- char *domain_id;
-};
-
/* all filesystem-wide lz4 configurations */
struct erofs_sb_lz4_info {
/* # of pages needed for EROFS lz4 rolling decompression */
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 4fc34b984e51..7172271290b9 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -370,18 +370,18 @@ static int erofs_read_superblock(struct super_block *sb)
return ret;
}
-static void erofs_default_options(struct erofs_fs_context *ctx)
+static void erofs_default_options(struct erofs_sb_info *sbi)
{
#ifdef CONFIG_EROFS_FS_ZIP
- ctx->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
- ctx->opt.max_sync_decompress_pages = 3;
- ctx->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_AUTO;
+ sbi->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
+ sbi->opt.max_sync_decompress_pages = 3;
+ sbi->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_AUTO;
#endif
#ifdef CONFIG_EROFS_FS_XATTR
- set_opt(&ctx->opt, XATTR_USER);
+ set_opt(&sbi->opt, XATTR_USER);
#endif
#ifdef CONFIG_EROFS_FS_POSIX_ACL
- set_opt(&ctx->opt, POSIX_ACL);
+ set_opt(&sbi->opt, POSIX_ACL);
#endif
}
@@ -426,16 +426,16 @@ static const struct fs_parameter_spec erofs_fs_parameters[] = {
static bool erofs_fc_set_dax_mode(struct fs_context *fc, unsigned int mode)
{
#ifdef CONFIG_FS_DAX
- struct erofs_fs_context *ctx = fc->fs_private;
+ struct erofs_sb_info *sbi = fc->s_fs_info;
switch (mode) {
case EROFS_MOUNT_DAX_ALWAYS:
- set_opt(&ctx->opt, DAX_ALWAYS);
- clear_opt(&ctx->opt, DAX_NEVER);
+ set_opt(&sbi->opt, DAX_ALWAYS);
+ clear_opt(&sbi->opt, DAX_NEVER);
return true;
case EROFS_MOUNT_DAX_NEVER:
- set_opt(&ctx->opt, DAX_NEVER);
- clear_opt(&ctx->opt, DAX_ALWAYS);
+ set_opt(&sbi->opt, DAX_NEVER);
+ clear_opt(&sbi->opt, DAX_ALWAYS);
return true;
default:
DBG_BUGON(1);
@@ -450,7 +450,7 @@ static bool erofs_fc_set_dax_mode(struct fs_context *fc, unsigned int mode)
static int erofs_fc_parse_param(struct fs_context *fc,
struct fs_parameter *param)
{
- struct erofs_fs_context *ctx = fc->fs_private;
+ struct erofs_sb_info *sbi = fc->s_fs_info;
struct fs_parse_result result;
struct erofs_device_info *dif;
int opt, ret;
@@ -463,9 +463,9 @@ static int erofs_fc_parse_param(struct fs_context *fc,
case Opt_user_xattr:
#ifdef CONFIG_EROFS_FS_XATTR
if (result.boolean)
- set_opt(&ctx->opt, XATTR_USER);
+ set_opt(&sbi->opt, XATTR_USER);
else
- clear_opt(&ctx->opt, XATTR_USER);
+ clear_opt(&sbi->opt, XATTR_USER);
#else
errorfc(fc, "{,no}user_xattr options not supported");
#endif
@@ -473,16 +473,16 @@ static int erofs_fc_parse_param(struct fs_context *fc,
case Opt_acl:
#ifdef CONFIG_EROFS_FS_POSIX_ACL
if (result.boolean)
- set_opt(&ctx->opt, POSIX_ACL);
+ set_opt(&sbi->opt, POSIX_ACL);
else
- clear_opt(&ctx->opt, POSIX_ACL);
+ clear_opt(&sbi->opt, POSIX_ACL);
#else
errorfc(fc, "{,no}acl options not supported");
#endif
break;
case Opt_cache_strategy:
#ifdef CONFIG_EROFS_FS_ZIP
- ctx->opt.cache_strategy = result.uint_32;
+ sbi->opt.cache_strategy = result.uint_32;
#else
errorfc(fc, "compression not supported, cache_strategy ignored");
#endif
@@ -504,27 +504,27 @@ static int erofs_fc_parse_param(struct fs_context *fc,
kfree(dif);
return -ENOMEM;
}
- down_write(&ctx->devs->rwsem);
- ret = idr_alloc(&ctx->devs->tree, dif, 0, 0, GFP_KERNEL);
- up_write(&ctx->devs->rwsem);
+ down_write(&sbi->devs->rwsem);
+ ret = idr_alloc(&sbi->devs->tree, dif, 0, 0, GFP_KERNEL);
+ up_write(&sbi->devs->rwsem);
if (ret < 0) {
kfree(dif->path);
kfree(dif);
return ret;
}
- ++ctx->devs->extra_devices;
+ ++sbi->devs->extra_devices;
break;
#ifdef CONFIG_EROFS_FS_ONDEMAND
case Opt_fsid:
- kfree(ctx->fsid);
- ctx->fsid = kstrdup(param->string, GFP_KERNEL);
- if (!ctx->fsid)
+ kfree(sbi->fsid);
+ sbi->fsid = kstrdup(param->string, GFP_KERNEL);
+ if (!sbi->fsid)
return -ENOMEM;
break;
case Opt_domain_id:
- kfree(ctx->domain_id);
- ctx->domain_id = kstrdup(param->string, GFP_KERNEL);
- if (!ctx->domain_id)
+ kfree(sbi->domain_id);
+ sbi->domain_id = kstrdup(param->string, GFP_KERNEL);
+ if (!sbi->domain_id)
return -ENOMEM;
break;
#else
@@ -582,7 +582,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
{
struct inode *inode;
struct erofs_sb_info *sbi = EROFS_SB(sb);
- struct erofs_fs_context *ctx = fc->fs_private;
int err;
sb->s_magic = EROFS_SUPER_MAGIC;
@@ -590,14 +589,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
sb->s_maxbytes = MAX_LFS_FILESIZE;
sb->s_op = &erofs_sops;
- sb->s_fs_info = sbi;
- sbi->opt = ctx->opt;
- sbi->devs = ctx->devs;
- ctx->devs = NULL;
- ctx->fsid = NULL;
- sbi->domain_id = ctx->domain_id;
- ctx->domain_id = NULL;
-
sbi->blkszbits = PAGE_SHIFT;
if (erofs_is_fscache_mode(sb)) {
sb->s_blocksize = PAGE_SIZE;
@@ -701,15 +692,8 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
static int erofs_fc_get_tree(struct fs_context *fc)
{
- struct erofs_fs_context *ctx = fc->fs_private;
- struct erofs_sb_info *sbi;
-
- sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
- if (!sbi)
- return -ENOMEM;
+ struct erofs_sb_info *sbi = fc->s_fs_info;
- fc->s_fs_info = sbi;
- sbi->fsid = ctx->fsid;
if (IS_ENABLED(CONFIG_EROFS_FS_ONDEMAND) && sbi->fsid)
return get_tree_nodev(fc, erofs_fc_fill_super);
@@ -720,19 +704,19 @@ static int erofs_fc_reconfigure(struct fs_context *fc)
{
struct super_block *sb = fc->root->d_sb;
struct erofs_sb_info *sbi = EROFS_SB(sb);
- struct erofs_fs_context *ctx = fc->fs_private;
+ struct erofs_sb_info *new_sbi = fc->s_fs_info;
DBG_BUGON(!sb_rdonly(sb));
- if (ctx->fsid || ctx->domain_id)
+ if (new_sbi->fsid || new_sbi->domain_id)
erofs_info(sb, "ignoring reconfiguration for fsid|domain_id.");
- if (test_opt(&ctx->opt, POSIX_ACL))
+ if (test_opt(&new_sbi->opt, POSIX_ACL))
fc->sb_flags |= SB_POSIXACL;
else
fc->sb_flags &= ~SB_POSIXACL;
- sbi->opt = ctx->opt;
+ sbi->opt = new_sbi->opt;
fc->sb_flags |= SB_RDONLY;
return 0;
@@ -763,16 +747,12 @@ static void erofs_free_dev_context(struct erofs_dev_context *devs)
static void erofs_fc_free(struct fs_context *fc)
{
- struct erofs_fs_context *ctx = fc->fs_private;
struct erofs_sb_info *sbi = fc->s_fs_info;
- erofs_free_dev_context(ctx->devs);
- kfree(ctx->fsid);
- kfree(ctx->domain_id);
- kfree(ctx);
-
- if (sbi)
- kfree(sbi);
+ erofs_free_dev_context(sbi->devs);
+ kfree(sbi->fsid);
+ kfree(sbi->domain_id);
+ kfree(sbi);
}
static const struct fs_context_operations erofs_context_ops = {
@@ -784,22 +764,22 @@ static const struct fs_context_operations erofs_context_ops = {
static int erofs_init_fs_context(struct fs_context *fc)
{
- struct erofs_fs_context *ctx;
+ struct erofs_sb_info *sbi;
- ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
- if (!ctx)
+ sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
+ if (!sbi)
return -ENOMEM;
- ctx->devs = kzalloc(sizeof(struct erofs_dev_context), GFP_KERNEL);
- if (!ctx->devs) {
- kfree(ctx);
+ sbi->devs = kzalloc(sizeof(struct erofs_dev_context), GFP_KERNEL);
+ if (!sbi->devs) {
+ kfree(sbi);
return -ENOMEM;
}
- fc->fs_private = ctx;
+ fc->fs_private = sbi;
- idr_init(&ctx->devs->tree);
- init_rwsem(&ctx->devs->rwsem);
- erofs_default_options(ctx);
+ idr_init(&sbi->devs->tree);
+ init_rwsem(&sbi->devs->rwsem);
+ erofs_default_options(sbi);
fc->ops = &erofs_context_ops;
return 0;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH -next] erofs: get rid of erofs_fs_context
2024-04-19 3:14 [PATCH -next] erofs: get rid of erofs_fs_context Baokun Li via Linux-erofs
@ 2024-04-19 10:13 ` Christian Brauner
2024-04-19 11:20 ` Baokun Li via Linux-erofs
0 siblings, 1 reply; 3+ messages in thread
From: Christian Brauner @ 2024-04-19 10:13 UTC (permalink / raw)
To: Baokun Li; +Cc: yangerkun, linux-kernel, huyue2, viro, linux-erofs
On Fri, Apr 19, 2024 at 11:14:59AM +0800, Baokun Li wrote:
> Instead of allocating the erofs_sb_info in get_tree() allocate it during
> init_fs_context() and after this erofs_fs_context is no longer needed,
> so replace ctx with sbi, no functional changes.
>
> Suggested-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
> ---
> Hi Gao Xiang,
> Hi Jingbo,
>
> I noticed that Christian's original patch has been merged into the next
> branch, so I'm sending this patch out separately.
An an accident on my part as I left it in the vfs.fixes branch. I expect
that the erofs tree will pick it up.
>
> Regards,
> Baokun
>
> fs/erofs/internal.h | 7 ---
> fs/erofs/super.c | 112 ++++++++++++++++++--------------------------
> 2 files changed, 46 insertions(+), 73 deletions(-)
>
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 116c1d5d1932..53ebba952a2f 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -84,13 +84,6 @@ struct erofs_dev_context {
> bool flatdev;
> };
>
> -struct erofs_fs_context {
> - struct erofs_mount_opts opt;
> - struct erofs_dev_context *devs;
> - char *fsid;
> - char *domain_id;
> -};
> -
> /* all filesystem-wide lz4 configurations */
> struct erofs_sb_lz4_info {
> /* # of pages needed for EROFS lz4 rolling decompression */
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 4fc34b984e51..7172271290b9 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -370,18 +370,18 @@ static int erofs_read_superblock(struct super_block *sb)
> return ret;
> }
>
> -static void erofs_default_options(struct erofs_fs_context *ctx)
> +static void erofs_default_options(struct erofs_sb_info *sbi)
> {
> #ifdef CONFIG_EROFS_FS_ZIP
> - ctx->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
> - ctx->opt.max_sync_decompress_pages = 3;
> - ctx->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_AUTO;
> + sbi->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
> + sbi->opt.max_sync_decompress_pages = 3;
> + sbi->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_AUTO;
> #endif
> #ifdef CONFIG_EROFS_FS_XATTR
> - set_opt(&ctx->opt, XATTR_USER);
> + set_opt(&sbi->opt, XATTR_USER);
> #endif
> #ifdef CONFIG_EROFS_FS_POSIX_ACL
> - set_opt(&ctx->opt, POSIX_ACL);
> + set_opt(&sbi->opt, POSIX_ACL);
> #endif
> }
>
> @@ -426,16 +426,16 @@ static const struct fs_parameter_spec erofs_fs_parameters[] = {
> static bool erofs_fc_set_dax_mode(struct fs_context *fc, unsigned int mode)
> {
> #ifdef CONFIG_FS_DAX
> - struct erofs_fs_context *ctx = fc->fs_private;
> + struct erofs_sb_info *sbi = fc->s_fs_info;
>
> switch (mode) {
> case EROFS_MOUNT_DAX_ALWAYS:
> - set_opt(&ctx->opt, DAX_ALWAYS);
> - clear_opt(&ctx->opt, DAX_NEVER);
> + set_opt(&sbi->opt, DAX_ALWAYS);
> + clear_opt(&sbi->opt, DAX_NEVER);
> return true;
> case EROFS_MOUNT_DAX_NEVER:
> - set_opt(&ctx->opt, DAX_NEVER);
> - clear_opt(&ctx->opt, DAX_ALWAYS);
> + set_opt(&sbi->opt, DAX_NEVER);
> + clear_opt(&sbi->opt, DAX_ALWAYS);
> return true;
> default:
> DBG_BUGON(1);
> @@ -450,7 +450,7 @@ static bool erofs_fc_set_dax_mode(struct fs_context *fc, unsigned int mode)
> static int erofs_fc_parse_param(struct fs_context *fc,
> struct fs_parameter *param)
> {
> - struct erofs_fs_context *ctx = fc->fs_private;
> + struct erofs_sb_info *sbi = fc->s_fs_info;
> struct fs_parse_result result;
> struct erofs_device_info *dif;
> int opt, ret;
> @@ -463,9 +463,9 @@ static int erofs_fc_parse_param(struct fs_context *fc,
> case Opt_user_xattr:
> #ifdef CONFIG_EROFS_FS_XATTR
> if (result.boolean)
> - set_opt(&ctx->opt, XATTR_USER);
> + set_opt(&sbi->opt, XATTR_USER);
> else
> - clear_opt(&ctx->opt, XATTR_USER);
> + clear_opt(&sbi->opt, XATTR_USER);
> #else
> errorfc(fc, "{,no}user_xattr options not supported");
> #endif
> @@ -473,16 +473,16 @@ static int erofs_fc_parse_param(struct fs_context *fc,
> case Opt_acl:
> #ifdef CONFIG_EROFS_FS_POSIX_ACL
> if (result.boolean)
> - set_opt(&ctx->opt, POSIX_ACL);
> + set_opt(&sbi->opt, POSIX_ACL);
> else
> - clear_opt(&ctx->opt, POSIX_ACL);
> + clear_opt(&sbi->opt, POSIX_ACL);
> #else
> errorfc(fc, "{,no}acl options not supported");
> #endif
> break;
> case Opt_cache_strategy:
> #ifdef CONFIG_EROFS_FS_ZIP
> - ctx->opt.cache_strategy = result.uint_32;
> + sbi->opt.cache_strategy = result.uint_32;
> #else
> errorfc(fc, "compression not supported, cache_strategy ignored");
> #endif
> @@ -504,27 +504,27 @@ static int erofs_fc_parse_param(struct fs_context *fc,
> kfree(dif);
> return -ENOMEM;
> }
> - down_write(&ctx->devs->rwsem);
> - ret = idr_alloc(&ctx->devs->tree, dif, 0, 0, GFP_KERNEL);
> - up_write(&ctx->devs->rwsem);
> + down_write(&sbi->devs->rwsem);
> + ret = idr_alloc(&sbi->devs->tree, dif, 0, 0, GFP_KERNEL);
> + up_write(&sbi->devs->rwsem);
> if (ret < 0) {
> kfree(dif->path);
> kfree(dif);
> return ret;
> }
> - ++ctx->devs->extra_devices;
> + ++sbi->devs->extra_devices;
> break;
> #ifdef CONFIG_EROFS_FS_ONDEMAND
> case Opt_fsid:
> - kfree(ctx->fsid);
> - ctx->fsid = kstrdup(param->string, GFP_KERNEL);
> - if (!ctx->fsid)
> + kfree(sbi->fsid);
> + sbi->fsid = kstrdup(param->string, GFP_KERNEL);
> + if (!sbi->fsid)
> return -ENOMEM;
> break;
> case Opt_domain_id:
> - kfree(ctx->domain_id);
> - ctx->domain_id = kstrdup(param->string, GFP_KERNEL);
> - if (!ctx->domain_id)
> + kfree(sbi->domain_id);
> + sbi->domain_id = kstrdup(param->string, GFP_KERNEL);
> + if (!sbi->domain_id)
> return -ENOMEM;
> break;
> #else
> @@ -582,7 +582,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
> {
> struct inode *inode;
> struct erofs_sb_info *sbi = EROFS_SB(sb);
> - struct erofs_fs_context *ctx = fc->fs_private;
> int err;
>
> sb->s_magic = EROFS_SUPER_MAGIC;
> @@ -590,14 +589,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
> sb->s_maxbytes = MAX_LFS_FILESIZE;
> sb->s_op = &erofs_sops;
>
> - sb->s_fs_info = sbi;
> - sbi->opt = ctx->opt;
> - sbi->devs = ctx->devs;
> - ctx->devs = NULL;
> - ctx->fsid = NULL;
> - sbi->domain_id = ctx->domain_id;
> - ctx->domain_id = NULL;
> -
> sbi->blkszbits = PAGE_SHIFT;
> if (erofs_is_fscache_mode(sb)) {
> sb->s_blocksize = PAGE_SIZE;
> @@ -701,15 +692,8 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>
> static int erofs_fc_get_tree(struct fs_context *fc)
> {
> - struct erofs_fs_context *ctx = fc->fs_private;
> - struct erofs_sb_info *sbi;
> -
> - sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
> - if (!sbi)
> - return -ENOMEM;
> + struct erofs_sb_info *sbi = fc->s_fs_info;
>
> - fc->s_fs_info = sbi;
> - sbi->fsid = ctx->fsid;
> if (IS_ENABLED(CONFIG_EROFS_FS_ONDEMAND) && sbi->fsid)
> return get_tree_nodev(fc, erofs_fc_fill_super);
>
> @@ -720,19 +704,19 @@ static int erofs_fc_reconfigure(struct fs_context *fc)
> {
> struct super_block *sb = fc->root->d_sb;
> struct erofs_sb_info *sbi = EROFS_SB(sb);
> - struct erofs_fs_context *ctx = fc->fs_private;
> + struct erofs_sb_info *new_sbi = fc->s_fs_info;
>
> DBG_BUGON(!sb_rdonly(sb));
>
> - if (ctx->fsid || ctx->domain_id)
> + if (new_sbi->fsid || new_sbi->domain_id)
> erofs_info(sb, "ignoring reconfiguration for fsid|domain_id.");
>
> - if (test_opt(&ctx->opt, POSIX_ACL))
> + if (test_opt(&new_sbi->opt, POSIX_ACL))
> fc->sb_flags |= SB_POSIXACL;
> else
> fc->sb_flags &= ~SB_POSIXACL;
>
> - sbi->opt = ctx->opt;
> + sbi->opt = new_sbi->opt;
>
> fc->sb_flags |= SB_RDONLY;
> return 0;
> @@ -763,16 +747,12 @@ static void erofs_free_dev_context(struct erofs_dev_context *devs)
>
> static void erofs_fc_free(struct fs_context *fc)
> {
> - struct erofs_fs_context *ctx = fc->fs_private;
> struct erofs_sb_info *sbi = fc->s_fs_info;
>
> - erofs_free_dev_context(ctx->devs);
> - kfree(ctx->fsid);
> - kfree(ctx->domain_id);
> - kfree(ctx);
> -
> - if (sbi)
> - kfree(sbi);
> + erofs_free_dev_context(sbi->devs);
> + kfree(sbi->fsid);
> + kfree(sbi->domain_id);
> + kfree(sbi);
> }
>
> static const struct fs_context_operations erofs_context_ops = {
> @@ -784,22 +764,22 @@ static const struct fs_context_operations erofs_context_ops = {
>
> static int erofs_init_fs_context(struct fs_context *fc)
> {
> - struct erofs_fs_context *ctx;
> + struct erofs_sb_info *sbi;
>
> - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> - if (!ctx)
> + sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
> + if (!sbi)
> return -ENOMEM;
>
> - ctx->devs = kzalloc(sizeof(struct erofs_dev_context), GFP_KERNEL);
> - if (!ctx->devs) {
> - kfree(ctx);
> + sbi->devs = kzalloc(sizeof(struct erofs_dev_context), GFP_KERNEL);
> + if (!sbi->devs) {
> + kfree(sbi);
> return -ENOMEM;
> }
> - fc->fs_private = ctx;
> + fc->fs_private = sbi;
I don't understand how your patch is going to work. fs_private isn't
transfered by the generic code to sb->s_fs_info. Did you mean
fc->s_fs_info = sbi?
>
> - idr_init(&ctx->devs->tree);
> - init_rwsem(&ctx->devs->rwsem);
> - erofs_default_options(ctx);
> + idr_init(&sbi->devs->tree);
> + init_rwsem(&sbi->devs->rwsem);
> + erofs_default_options(sbi);
> fc->ops = &erofs_context_ops;
> return 0;
> }
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH -next] erofs: get rid of erofs_fs_context
2024-04-19 10:13 ` Christian Brauner
@ 2024-04-19 11:20 ` Baokun Li via Linux-erofs
0 siblings, 0 replies; 3+ messages in thread
From: Baokun Li via Linux-erofs @ 2024-04-19 11:20 UTC (permalink / raw)
To: Christian Brauner; +Cc: yangerkun, linux-kernel, huyue2, viro, linux-erofs
On 2024/4/19 18:13, Christian Brauner wrote:
> On Fri, Apr 19, 2024 at 11:14:59AM +0800, Baokun Li wrote:
>> Instead of allocating the erofs_sb_info in get_tree() allocate it during
>> init_fs_context() and after this erofs_fs_context is no longer needed,
>> so replace ctx with sbi, no functional changes.
>>
>> Suggested-by: Jingbo Xu <jefflexu@linux.alibaba.com>
>> Signed-off-by: Baokun Li <libaokun1@huawei.com>
>> ---
>> Hi Gao Xiang,
>> Hi Jingbo,
>>
>> I noticed that Christian's original patch has been merged into the next
>> branch, so I'm sending this patch out separately.
> An an accident on my part as I left it in the vfs.fixes branch. I expect
> that the erofs tree will pick it up.
Hi Christian,
Okay, I'll send the full patch.
>> Regards,
>> Baokun
>>
>> fs/erofs/internal.h | 7 ---
>> fs/erofs/super.c | 112 ++++++++++++++++++--------------------------
>> 2 files changed, 46 insertions(+), 73 deletions(-)
>>
>> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
>> index 116c1d5d1932..53ebba952a2f 100644
>> --- a/fs/erofs/internal.h
>> +++ b/fs/erofs/internal.h
>> @@ -84,13 +84,6 @@ struct erofs_dev_context {
>> bool flatdev;
>> };
>>
>> -struct erofs_fs_context {
>> - struct erofs_mount_opts opt;
>> - struct erofs_dev_context *devs;
>> - char *fsid;
>> - char *domain_id;
>> -};
>> -
>> /* all filesystem-wide lz4 configurations */
>> struct erofs_sb_lz4_info {
>> /* # of pages needed for EROFS lz4 rolling decompression */
>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>> index 4fc34b984e51..7172271290b9 100644
>> --- a/fs/erofs/super.c
>> +++ b/fs/erofs/super.c
>> @@ -370,18 +370,18 @@ static int erofs_read_superblock(struct super_block *sb)
>> return ret;
>> }
>>
>> -static void erofs_default_options(struct erofs_fs_context *ctx)
>> +static void erofs_default_options(struct erofs_sb_info *sbi)
>> {
>> #ifdef CONFIG_EROFS_FS_ZIP
>> - ctx->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
>> - ctx->opt.max_sync_decompress_pages = 3;
>> - ctx->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_AUTO;
>> + sbi->opt.cache_strategy = EROFS_ZIP_CACHE_READAROUND;
>> + sbi->opt.max_sync_decompress_pages = 3;
>> + sbi->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_AUTO;
>> #endif
>> #ifdef CONFIG_EROFS_FS_XATTR
>> - set_opt(&ctx->opt, XATTR_USER);
>> + set_opt(&sbi->opt, XATTR_USER);
>> #endif
>> #ifdef CONFIG_EROFS_FS_POSIX_ACL
>> - set_opt(&ctx->opt, POSIX_ACL);
>> + set_opt(&sbi->opt, POSIX_ACL);
>> #endif
>> }
>>
>> @@ -426,16 +426,16 @@ static const struct fs_parameter_spec erofs_fs_parameters[] = {
>> static bool erofs_fc_set_dax_mode(struct fs_context *fc, unsigned int mode)
>> {
>> #ifdef CONFIG_FS_DAX
>> - struct erofs_fs_context *ctx = fc->fs_private;
>> + struct erofs_sb_info *sbi = fc->s_fs_info;
>>
>> switch (mode) {
>> case EROFS_MOUNT_DAX_ALWAYS:
>> - set_opt(&ctx->opt, DAX_ALWAYS);
>> - clear_opt(&ctx->opt, DAX_NEVER);
>> + set_opt(&sbi->opt, DAX_ALWAYS);
>> + clear_opt(&sbi->opt, DAX_NEVER);
>> return true;
>> case EROFS_MOUNT_DAX_NEVER:
>> - set_opt(&ctx->opt, DAX_NEVER);
>> - clear_opt(&ctx->opt, DAX_ALWAYS);
>> + set_opt(&sbi->opt, DAX_NEVER);
>> + clear_opt(&sbi->opt, DAX_ALWAYS);
>> return true;
>> default:
>> DBG_BUGON(1);
>> @@ -450,7 +450,7 @@ static bool erofs_fc_set_dax_mode(struct fs_context *fc, unsigned int mode)
>> static int erofs_fc_parse_param(struct fs_context *fc,
>> struct fs_parameter *param)
>> {
>> - struct erofs_fs_context *ctx = fc->fs_private;
>> + struct erofs_sb_info *sbi = fc->s_fs_info;
>> struct fs_parse_result result;
>> struct erofs_device_info *dif;
>> int opt, ret;
>> @@ -463,9 +463,9 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>> case Opt_user_xattr:
>> #ifdef CONFIG_EROFS_FS_XATTR
>> if (result.boolean)
>> - set_opt(&ctx->opt, XATTR_USER);
>> + set_opt(&sbi->opt, XATTR_USER);
>> else
>> - clear_opt(&ctx->opt, XATTR_USER);
>> + clear_opt(&sbi->opt, XATTR_USER);
>> #else
>> errorfc(fc, "{,no}user_xattr options not supported");
>> #endif
>> @@ -473,16 +473,16 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>> case Opt_acl:
>> #ifdef CONFIG_EROFS_FS_POSIX_ACL
>> if (result.boolean)
>> - set_opt(&ctx->opt, POSIX_ACL);
>> + set_opt(&sbi->opt, POSIX_ACL);
>> else
>> - clear_opt(&ctx->opt, POSIX_ACL);
>> + clear_opt(&sbi->opt, POSIX_ACL);
>> #else
>> errorfc(fc, "{,no}acl options not supported");
>> #endif
>> break;
>> case Opt_cache_strategy:
>> #ifdef CONFIG_EROFS_FS_ZIP
>> - ctx->opt.cache_strategy = result.uint_32;
>> + sbi->opt.cache_strategy = result.uint_32;
>> #else
>> errorfc(fc, "compression not supported, cache_strategy ignored");
>> #endif
>> @@ -504,27 +504,27 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>> kfree(dif);
>> return -ENOMEM;
>> }
>> - down_write(&ctx->devs->rwsem);
>> - ret = idr_alloc(&ctx->devs->tree, dif, 0, 0, GFP_KERNEL);
>> - up_write(&ctx->devs->rwsem);
>> + down_write(&sbi->devs->rwsem);
>> + ret = idr_alloc(&sbi->devs->tree, dif, 0, 0, GFP_KERNEL);
>> + up_write(&sbi->devs->rwsem);
>> if (ret < 0) {
>> kfree(dif->path);
>> kfree(dif);
>> return ret;
>> }
>> - ++ctx->devs->extra_devices;
>> + ++sbi->devs->extra_devices;
>> break;
>> #ifdef CONFIG_EROFS_FS_ONDEMAND
>> case Opt_fsid:
>> - kfree(ctx->fsid);
>> - ctx->fsid = kstrdup(param->string, GFP_KERNEL);
>> - if (!ctx->fsid)
>> + kfree(sbi->fsid);
>> + sbi->fsid = kstrdup(param->string, GFP_KERNEL);
>> + if (!sbi->fsid)
>> return -ENOMEM;
>> break;
>> case Opt_domain_id:
>> - kfree(ctx->domain_id);
>> - ctx->domain_id = kstrdup(param->string, GFP_KERNEL);
>> - if (!ctx->domain_id)
>> + kfree(sbi->domain_id);
>> + sbi->domain_id = kstrdup(param->string, GFP_KERNEL);
>> + if (!sbi->domain_id)
>> return -ENOMEM;
>> break;
>> #else
>> @@ -582,7 +582,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>> {
>> struct inode *inode;
>> struct erofs_sb_info *sbi = EROFS_SB(sb);
>> - struct erofs_fs_context *ctx = fc->fs_private;
>> int err;
>>
>> sb->s_magic = EROFS_SUPER_MAGIC;
>> @@ -590,14 +589,6 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>> sb->s_maxbytes = MAX_LFS_FILESIZE;
>> sb->s_op = &erofs_sops;
>>
>> - sb->s_fs_info = sbi;
>> - sbi->opt = ctx->opt;
>> - sbi->devs = ctx->devs;
>> - ctx->devs = NULL;
>> - ctx->fsid = NULL;
>> - sbi->domain_id = ctx->domain_id;
>> - ctx->domain_id = NULL;
>> -
>> sbi->blkszbits = PAGE_SHIFT;
>> if (erofs_is_fscache_mode(sb)) {
>> sb->s_blocksize = PAGE_SIZE;
>> @@ -701,15 +692,8 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
>>
>> static int erofs_fc_get_tree(struct fs_context *fc)
>> {
>> - struct erofs_fs_context *ctx = fc->fs_private;
>> - struct erofs_sb_info *sbi;
>> -
>> - sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
>> - if (!sbi)
>> - return -ENOMEM;
>> + struct erofs_sb_info *sbi = fc->s_fs_info;
>>
>> - fc->s_fs_info = sbi;
>> - sbi->fsid = ctx->fsid;
>> if (IS_ENABLED(CONFIG_EROFS_FS_ONDEMAND) && sbi->fsid)
>> return get_tree_nodev(fc, erofs_fc_fill_super);
>>
>> @@ -720,19 +704,19 @@ static int erofs_fc_reconfigure(struct fs_context *fc)
>> {
>> struct super_block *sb = fc->root->d_sb;
>> struct erofs_sb_info *sbi = EROFS_SB(sb);
>> - struct erofs_fs_context *ctx = fc->fs_private;
>> + struct erofs_sb_info *new_sbi = fc->s_fs_info;
>>
>> DBG_BUGON(!sb_rdonly(sb));
>>
>> - if (ctx->fsid || ctx->domain_id)
>> + if (new_sbi->fsid || new_sbi->domain_id)
>> erofs_info(sb, "ignoring reconfiguration for fsid|domain_id.");
>>
>> - if (test_opt(&ctx->opt, POSIX_ACL))
>> + if (test_opt(&new_sbi->opt, POSIX_ACL))
>> fc->sb_flags |= SB_POSIXACL;
>> else
>> fc->sb_flags &= ~SB_POSIXACL;
>>
>> - sbi->opt = ctx->opt;
>> + sbi->opt = new_sbi->opt;
>>
>> fc->sb_flags |= SB_RDONLY;
>> return 0;
>> @@ -763,16 +747,12 @@ static void erofs_free_dev_context(struct erofs_dev_context *devs)
>>
>> static void erofs_fc_free(struct fs_context *fc)
>> {
>> - struct erofs_fs_context *ctx = fc->fs_private;
>> struct erofs_sb_info *sbi = fc->s_fs_info;
>>
>> - erofs_free_dev_context(ctx->devs);
>> - kfree(ctx->fsid);
>> - kfree(ctx->domain_id);
>> - kfree(ctx);
>> -
>> - if (sbi)
>> - kfree(sbi);
>> + erofs_free_dev_context(sbi->devs);
>> + kfree(sbi->fsid);
>> + kfree(sbi->domain_id);
>> + kfree(sbi);
>> }
>>
>> static const struct fs_context_operations erofs_context_ops = {
>> @@ -784,22 +764,22 @@ static const struct fs_context_operations erofs_context_ops = {
>>
>> static int erofs_init_fs_context(struct fs_context *fc)
>> {
>> - struct erofs_fs_context *ctx;
>> + struct erofs_sb_info *sbi;
>>
>> - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>> - if (!ctx)
>> + sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
>> + if (!sbi)
>> return -ENOMEM;
>>
>> - ctx->devs = kzalloc(sizeof(struct erofs_dev_context), GFP_KERNEL);
>> - if (!ctx->devs) {
>> - kfree(ctx);
>> + sbi->devs = kzalloc(sizeof(struct erofs_dev_context), GFP_KERNEL);
>> + if (!sbi->devs) {
>> + kfree(sbi);
>> return -ENOMEM;
>> }
>> - fc->fs_private = ctx;
>> + fc->fs_private = sbi;
> I don't understand how your patch is going to work. fs_private isn't
> transfered by the generic code to sb->s_fs_info. Did you mean
> fc->s_fs_info = sbi?
Yes, it's fc->s_fs_info, my mistake!
The original plan was to split into two patches, getting rid of
erofs_fs_context in the first and fixing the problem in the second.
I wrote the patch yesterday and tested it, but when I sent it out
today after testing it, I noticed that your original patch was
merged in, and then I made a mistake when rebasing on the
new next branch, sorry about that. I'll send out the previous
patch soon.
Thanks for the correction!
>
>>
>> - idr_init(&ctx->devs->tree);
>> - init_rwsem(&ctx->devs->rwsem);
>> - erofs_default_options(ctx);
>> + idr_init(&sbi->devs->tree);
>> + init_rwsem(&sbi->devs->rwsem);
>> + erofs_default_options(sbi);
>> fc->ops = &erofs_context_ops;
>> return 0;
>> }
>> --
>> 2.31.1
>>
Thanks again!
--
With Best Regards,
Baokun Li
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-04-19 11:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-19 3:14 [PATCH -next] erofs: get rid of erofs_fs_context Baokun Li via Linux-erofs
2024-04-19 10:13 ` Christian Brauner
2024-04-19 11:20 ` Baokun Li via Linux-erofs
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).