diff mbox series

[f2fs-dev,1/9] f2fs: Add fs parameter specifications for mount options

Message ID 20240814023912.3959299-2-lihongbo22@huawei.com (mailing list archive)
State New
Headers show
Series f2fs: new mount API conversion | expand

Commit Message

Hongbo Li Aug. 14, 2024, 2:39 a.m. UTC 
Use an array of `fs_parameter_spec` called f2fs_param_specs to
hold the mount option specifications for the new mount api.

Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 fs/f2fs/super.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

Comments

Eric Sandeen Aug. 30, 2024, 9 p.m. UTC | #1 
On 8/13/24 9:39 PM, Hongbo Li wrote:
> Use an array of `fs_parameter_spec` called f2fs_param_specs to
> hold the mount option specifications for the new mount api.
> 
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  fs/f2fs/super.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 3959fd137cc9..1bd923a73c1f 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -28,6 +28,7 @@
>  #include <linux/part_stat.h>
>  #include <linux/zstd.h>
>  #include <linux/lz4.h>
> +#include <linux/fs_parser.h>
>  
>  #include "f2fs.h"
>  #include "node.h"
> @@ -189,9 +190,87 @@ enum {
>  	Opt_memory_mode,
>  	Opt_age_extent_cache,
>  	Opt_errors,
> +	Opt_jqfmt,
> +	Opt_checkpoint,

If adding an opt_jqfmt to use with an enum, you can/should remove
Opt_jqfmt_vfsold Opt_jqfmt_vfsv0, and Opt_jqfmt_vfsv1, because they
are no longer used.

Similarly for Opt_checkpoint_disable_* symbols.

>  	Opt_err,
>  };
>  
> +static const struct constant_table f2fs_param_jqfmt[] = {
> +	{"vfsold",	QFMT_VFS_OLD},
> +	{"vfsv0",	QFMT_VFS_V0},
> +	{"vfsv1",	QFMT_VFS_V1},
> +	{}
> +};
> +
> +static const struct fs_parameter_spec f2fs_param_specs[] = {
> +	fsparam_string("background_gc", Opt_gc_background),
> +	fsparam_flag("disable_roll_forward", Opt_disable_roll_forward),
> +	fsparam_flag("norecovery", Opt_norecovery),

Many/most other filesystems tab-align the param_spec, like

...
+	fsparam_string	("background_gc",	Opt_gc_background),
+	fsparam_flag	("disable_roll_forward",Opt_disable_roll_forward),
+	fsparam_flag	("norecovery",		Opt_norecovery),
...

but that's just a style thing, up to you and the maintainers.

I'd also suggest making more use of enums (as you did for f2fs_param_jqfmt).
I think it can simplify parsing in the long run if you choose to. It avoids
the "if strcmp() else if strcmp() else if strcmp()... pattern, for example
you can do:

static const struct constant_table f2fs_param_background_gc[] = {
	{"on",		BGGC_MODE_ON},
	{"off",		BGGC_MODE_OFF},
	{"sync",	BGGC_MODE_SYNC},
	{}
};

...

	fsparam_enum	("background_gc",	Opt_gc_background, f2fs_param_background_gc),

...

and then parsing becomes simply:

	case Opt_gc_background:
		F2FS_CTX_INFO(ctx).bggc_mode = result.uint_32;
		ctx->spec_mask |= F2FS_SPEC_background_gc;
		break;

When I tried this I made a lot of use of enums, see
https://git.kernel.org/pub/scm/linux/kernel/git/sandeen/linux.git/tree/fs/f2fs/super.c?h=f2fs-mount-api#n182
and see what you think?

Thanks,
-Eric
diff mbox series

Patch

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 3959fd137cc9..1bd923a73c1f 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -28,6 +28,7 @@ 
 #include <linux/part_stat.h>
 #include <linux/zstd.h>
 #include <linux/lz4.h>
+#include <linux/fs_parser.h>
 
 #include "f2fs.h"
 #include "node.h"
@@ -189,9 +190,87 @@  enum {
 	Opt_memory_mode,
 	Opt_age_extent_cache,
 	Opt_errors,
+	Opt_jqfmt,
+	Opt_checkpoint,
 	Opt_err,
 };
 
+static const struct constant_table f2fs_param_jqfmt[] = {
+	{"vfsold",	QFMT_VFS_OLD},
+	{"vfsv0",	QFMT_VFS_V0},
+	{"vfsv1",	QFMT_VFS_V1},
+	{}
+};
+
+static const struct fs_parameter_spec f2fs_param_specs[] = {
+	fsparam_string("background_gc", Opt_gc_background),
+	fsparam_flag("disable_roll_forward", Opt_disable_roll_forward),
+	fsparam_flag("norecovery", Opt_norecovery),
+	fsparam_flag("discard", Opt_discard),
+	fsparam_flag("nodiscard", Opt_nodiscard),
+	fsparam_flag("no_heap", Opt_noheap),
+	fsparam_flag("heap", Opt_heap),
+	fsparam_flag("user_xattr", Opt_user_xattr),
+	fsparam_flag("nouser_xattr", Opt_nouser_xattr),
+	fsparam_flag("acl", Opt_acl),
+	fsparam_flag("noacl", Opt_noacl),
+	fsparam_s32("active_logs", Opt_active_logs),
+	fsparam_flag("disable_ext_identify", Opt_disable_ext_identify),
+	fsparam_flag("inline_xattr", Opt_inline_xattr),
+	fsparam_flag("noinline_xattr", Opt_noinline_xattr),
+	fsparam_s32("inline_xattr_size", Opt_inline_xattr_size),
+	fsparam_flag("inline_data", Opt_inline_data),
+	fsparam_flag("inline_dentry", Opt_inline_dentry),
+	fsparam_flag("noinline_dentry", Opt_noinline_dentry),
+	fsparam_flag("flush_merge", Opt_flush_merge),
+	fsparam_flag("noflush_merge", Opt_noflush_merge),
+	fsparam_flag("barrier", Opt_barrier),
+	fsparam_flag("nobarrier", Opt_nobarrier),
+	fsparam_flag("fastboot", Opt_fastboot),
+	fsparam_flag("extent_cache", Opt_extent_cache),
+	fsparam_flag("noextent_cache", Opt_noextent_cache),
+	fsparam_flag("noinline_data", Opt_noinline_data),
+	fsparam_flag("data_flush", Opt_data_flush),
+	fsparam_u32("reserve_root", Opt_reserve_root),
+	fsparam_u32("resgid", Opt_resgid),
+	fsparam_u32("resuid", Opt_resuid),
+	fsparam_string("mode", Opt_mode),
+	fsparam_s32("fault_injection", Opt_fault_injection),
+	fsparam_u32("fault_type", Opt_fault_type),
+	fsparam_flag("quota", Opt_quota),
+	fsparam_flag("noquota", Opt_noquota),
+	fsparam_flag("usrquota", Opt_usrquota),
+	fsparam_flag("grpquota", Opt_grpquota),
+	fsparam_flag("prjquota", Opt_prjquota),
+	fsparam_string_empty("usrjquota", Opt_usrjquota),
+	fsparam_string_empty("grpjquota", Opt_grpjquota),
+	fsparam_string_empty("prjjquota", Opt_prjjquota),
+	fsparam_enum("jqfmt", Opt_jqfmt, f2fs_param_jqfmt),
+	fsparam_string("alloc_mode", Opt_alloc),
+	fsparam_string("fsync_mode", Opt_fsync),
+	fsparam_string("test_dummy_encryption", Opt_test_dummy_encryption),
+	fsparam_flag("test_dummy_encryption", Opt_test_dummy_encryption),
+	fsparam_flag("inlinecrypt", Opt_inlinecrypt),
+	fsparam_string("checkpoint", Opt_checkpoint),
+	fsparam_flag("checkpoint_merge", Opt_checkpoint_merge),
+	fsparam_flag("nocheckpoint_merge", Opt_nocheckpoint_merge),
+	fsparam_string("compress_algorithm", Opt_compress_algorithm),
+	fsparam_u32("compress_log_size", Opt_compress_log_size),
+	fsparam_string("compress_extension", Opt_compress_extension),
+	fsparam_string("nocompress_extension", Opt_nocompress_extension),
+	fsparam_flag("compress_chksum", Opt_compress_chksum),
+	fsparam_string("compress_mode", Opt_compress_mode),
+	fsparam_flag("compress_cache", Opt_compress_cache),
+	fsparam_flag("atgc", Opt_atgc),
+	fsparam_flag("gc_merge", Opt_gc_merge),
+	fsparam_flag("nogc_merge", Opt_nogc_merge),
+	fsparam_string("discard_unit", Opt_discard_unit),
+	fsparam_string("memory", Opt_memory_mode),
+	fsparam_flag("age_extent_cache", Opt_age_extent_cache),
+	fsparam_string("errors", Opt_errors),
+	{}
+};
+
 static match_table_t f2fs_tokens = {
 	{Opt_gc_background, "background_gc=%s"},
 	{Opt_disable_roll_forward, "disable_roll_forward"},