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@xxxxxxxxxx>
> ---
> 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
