On 5/14/25 10:33, Hongbo Li wrote:
>
>
> On 2025/5/13 16:59, Chao Yu wrote:
>> On 4/24/25 01:08, Eric Sandeen wrote:
>>> From: Hongbo Li <[email protected]>
>>>
>>> The new mount api will execute .parse_param, .init_fs_context, .get_tree
>>> and will call .remount if remount happened. So we add the necessary
>>> functions for the fs_context_operations. If .init_fs_context is added,
>>> the old .mount should remove.
>>>
>>> See Documentation/filesystems/mount_api.rst for more information.
>>
>> mkfs.f2fs -f -O extra_attr,flexible_inline_xattr /dev/vdb
>> mount -o inline_xattr_size=512 /dev/vdb /mnt/f2fs
>> mount: /mnt/f2fs: wrong fs type, bad option, bad superblock on /dev/vdb,
>> missing codepage or helper program, or other error.
>> dmesg(1) may have more information after failed mount system call.
>> dmesg
>> [ 1758.202282] F2FS-fs (vdb): Image doesn't support compression
>> [ 1758.202286] F2FS-fs (vdb): inline_xattr_size option should be set with
>> inline_xattr option
>>
>> Eric, Hongbo, can you please take a look at this issue?
>>
> Sorry, we only check the option hold in ctx, we should do the double check in
> sbi. Or other elegant approaches.
>
> For the "support compression", is it also the error in this testcase?
I think so, I checked this w/ additional logs, and found this:
#define F2FS_MOUNT_INLINE_XATTR_SIZE 0x00080000
#define F2FS_SPEC_compress_chksum (1 << 19) /* equal to
0x00080000)
if (!f2fs_sb_has_compression(sbi)) {
if (test_compression_spec(ctx->opt_mask) ||
ctx_test_opt(ctx, F2FS_MOUNT_COMPRESS_CACHE))
f2fs_info(sbi, "Image doesn't support compression");
clear_compression_spec(ctx);
ctx->opt_mask &= ~F2FS_MOUNT_COMPRESS_CACHE;
return 0;
}
We should use test_compression_spec(ctx->spec_mask) instead of
test_compression_spec(ctx->opt_mask) to check special mask of compression
option?
Thanks,
>
> Thanks,
> Hongbo
>
>> Thanks,
>>
>>>
>>> Signed-off-by: Hongbo Li <[email protected]>
>>> [sandeen: forward port]
>>> Signed-off-by: Eric Sandeen <[email protected]>
>>> ---
>>> fs/f2fs/super.c | 156 +++++++++++++++++++-----------------------------
>>> 1 file changed, 62 insertions(+), 94 deletions(-)
>>>
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index 37497fd80bb9..041bd6c482a0 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -1141,47 +1141,6 @@ static int f2fs_parse_param(struct fs_context *fc,
>>> struct fs_parameter *param)
>>> return 0;
>>> }
>>> -static int parse_options(struct fs_context *fc, char *options)
>>> -{
>>> - struct fs_parameter param;
>>> - char *key;
>>> - int ret;
>>> -
>>> - if (!options)
>>> - return 0;
>>> -
>>> - while ((key = strsep(&options, ",")) != NULL) {
>>> - if (*key) {
>>> - size_t v_len = 0;
>>> - char *value = strchr(key, '=');
>>> -
>>> - param.type = fs_value_is_flag;
>>> - param.string = NULL;
>>> -
>>> - if (value) {
>>> - if (value == key)
>>> - continue;
>>> -
>>> - *value++ = 0;
>>> - v_len = strlen(value);
>>> - param.string = kmemdup_nul(value, v_len, GFP_KERNEL);
>>> - if (!param.string)
>>> - return -ENOMEM;
>>> - param.type = fs_value_is_string;
>>> - }
>>> -
>>> - param.key = key;
>>> - param.size = v_len;
>>> -
>>> - ret = f2fs_parse_param(fc, ¶m);
>>> - kfree(param.string);
>>> - if (ret < 0)
>>> - return ret;
>>> - }
>>> - }
>>> - return 0;
>>> -}
>>> -
>>> /*
>>> * Check quota settings consistency.
>>> */
>>> @@ -2583,13 +2542,12 @@ static void f2fs_enable_checkpoint(struct
>>> f2fs_sb_info *sbi)
>>> f2fs_flush_ckpt_thread(sbi);
>>> }
>>> -static int f2fs_remount(struct super_block *sb, int *flags, char *data)
>>> +static int __f2fs_remount(struct fs_context *fc, struct super_block *sb)
>>> {
>>> struct f2fs_sb_info *sbi = F2FS_SB(sb);
>>> struct f2fs_mount_info org_mount_opt;
>>> - struct f2fs_fs_context ctx;
>>> - struct fs_context fc;
>>> unsigned long old_sb_flags;
>>> + unsigned int flags = fc->sb_flags;
>>> int err;
>>> bool need_restart_gc = false, need_stop_gc = false;
>>> bool need_restart_flush = false, need_stop_flush = false;
>>> @@ -2635,7 +2593,7 @@ static int f2fs_remount(struct super_block *sb, int
>>> *flags, char *data)
>>> #endif
>>> /* recover superblocks we couldn't write due to previous RO mount */
>>> - if (!(*flags & SB_RDONLY) && is_sbi_flag_set(sbi, SBI_NEED_SB_WRITE)) {
>>> + if (!(flags & SB_RDONLY) && is_sbi_flag_set(sbi, SBI_NEED_SB_WRITE)) {
>>> err = f2fs_commit_super(sbi, false);
>>> f2fs_info(sbi, "Try to recover all the superblocks, ret: %d",
>>> err);
>>> @@ -2645,21 +2603,11 @@ static int f2fs_remount(struct super_block *sb, int
>>> *flags, char *data)
>>> default_options(sbi, true);
>>> - memset(&fc, 0, sizeof(fc));
>>> - memset(&ctx, 0, sizeof(ctx));
>>> - fc.fs_private = &ctx;
>>> - fc.purpose = FS_CONTEXT_FOR_RECONFIGURE;
>>> -
>>> - /* parse mount options */
>>> - err = parse_options(&fc, data);
>>> - if (err)
>>> - goto restore_opts;
>>> -
>>> - err = f2fs_check_opt_consistency(&fc, sb);
>>> + err = f2fs_check_opt_consistency(fc, sb);
>>> if (err < 0)
>>> goto restore_opts;
>>> - f2fs_apply_options(&fc, sb);
>>> + f2fs_apply_options(fc, sb);
>>> #ifdef CONFIG_BLK_DEV_ZONED
>>> if (f2fs_sb_has_blkzoned(sbi) &&
>>> @@ -2678,20 +2626,20 @@ static int f2fs_remount(struct super_block *sb, int
>>> *flags, char *data)
>>> * Previous and new state of filesystem is RO,
>>> * so skip checking GC and FLUSH_MERGE conditions.
>>> */
>>> - if (f2fs_readonly(sb) && (*flags & SB_RDONLY))
>>> + if (f2fs_readonly(sb) && (flags & SB_RDONLY))
>>> goto skip;
>>> - if (f2fs_dev_is_readonly(sbi) && !(*flags & SB_RDONLY)) {
>>> + if (f2fs_dev_is_readonly(sbi) && !(flags & SB_RDONLY)) {
>>> err = -EROFS;
>>> goto restore_opts;
>>> }
>>> #ifdef CONFIG_QUOTA
>>> - if (!f2fs_readonly(sb) && (*flags & SB_RDONLY)) {
>>> + if (!f2fs_readonly(sb) && (flags & SB_RDONLY)) {
>>> err = dquot_suspend(sb, -1);
>>> if (err < 0)
>>> goto restore_opts;
>>> - } else if (f2fs_readonly(sb) && !(*flags & SB_RDONLY)) {
>>> + } else if (f2fs_readonly(sb) && !(flags & SB_RDONLY)) {
>>> /* dquot_resume needs RW */
>>> sb->s_flags &= ~SB_RDONLY;
>>> if (sb_any_quota_suspended(sb)) {
>>> @@ -2747,7 +2695,7 @@ static int f2fs_remount(struct super_block *sb, int
>>> *flags, char *data)
>>> goto restore_opts;
>>> }
>>> - if ((*flags & SB_RDONLY) && test_opt(sbi, DISABLE_CHECKPOINT)) {
>>> + if ((flags & SB_RDONLY) && test_opt(sbi, DISABLE_CHECKPOINT)) {
>>> err = -EINVAL;
>>> f2fs_warn(sbi, "disabling checkpoint not compatible with
>>> read-only");
>>> goto restore_opts;
>>> @@ -2758,7 +2706,7 @@ static int f2fs_remount(struct super_block *sb, int
>>> *flags, char *data)
>>> * or if background_gc = off is passed in mount
>>> * option. Also sync the filesystem.
>>> */
>>> - if ((*flags & SB_RDONLY) ||
>>> + if ((flags & SB_RDONLY) ||
>>> (F2FS_OPTION(sbi).bggc_mode == BGGC_MODE_OFF &&
>>> !test_opt(sbi, GC_MERGE))) {
>>> if (sbi->gc_thread) {
>>> @@ -2772,7 +2720,7 @@ static int f2fs_remount(struct super_block *sb, int
>>> *flags, char *data)
>>> need_stop_gc = true;
>>> }
>>> - if (*flags & SB_RDONLY) {
>>> + if (flags & SB_RDONLY) {
>>> sync_inodes_sb(sb);
>>> set_sbi_flag(sbi, SBI_IS_DIRTY);
>>> @@ -2785,7 +2733,7 @@ static int f2fs_remount(struct super_block *sb, int
>>> *flags, char *data)
>>> * We stop issue flush thread if FS is mounted as RO
>>> * or if flush_merge is not passed in mount option.
>>> */
>>> - if ((*flags & SB_RDONLY) || !test_opt(sbi, FLUSH_MERGE)) {
>>> + if ((flags & SB_RDONLY) || !test_opt(sbi, FLUSH_MERGE)) {
>>> clear_opt(sbi, FLUSH_MERGE);
>>> f2fs_destroy_flush_cmd_control(sbi, false);
>>> need_restart_flush = true;
>>> @@ -2827,7 +2775,7 @@ static int f2fs_remount(struct super_block *sb, int
>>> *flags, char *data)
>>> * triggered while remount and we need to take care of it before
>>> * returning from remount.
>>> */
>>> - if ((*flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) ||
>>> + if ((flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) ||
>>> !test_opt(sbi, MERGE_CHECKPOINT)) {
>>> f2fs_stop_ckpt_thread(sbi);
>>> } else {
>>> @@ -2854,7 +2802,7 @@ static int f2fs_remount(struct super_block *sb, int
>>> *flags, char *data)
>>> (test_opt(sbi, POSIX_ACL) ? SB_POSIXACL : 0);
>>> limit_reserve_root(sbi);
>>> - *flags = (*flags & ~SB_LAZYTIME) | (sb->s_flags & SB_LAZYTIME);
>>> + fc->sb_flags = (flags & ~SB_LAZYTIME) | (sb->s_flags & SB_LAZYTIME);
>>> sbi->umount_lock_holder = NULL;
>>> return 0;
>>> @@ -3523,7 +3471,6 @@ static const struct super_operations f2fs_sops = {
>>> .freeze_fs = f2fs_freeze,
>>> .unfreeze_fs = f2fs_unfreeze,
>>> .statfs = f2fs_statfs,
>>> - .remount_fs = f2fs_remount,
>>> .shutdown = f2fs_shutdown,
>>> };
>>> @@ -4745,16 +4692,13 @@ static void f2fs_tuning_parameters(struct
>>> f2fs_sb_info *sbi)
>>> sbi->readdir_ra = true;
>>> }
>>> -static int f2fs_fill_super(struct super_block *sb, void *data, int
>>> silent)
>>> +static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc)
>>> {
>>> struct f2fs_sb_info *sbi;
>>> struct f2fs_super_block *raw_super;
>>> - struct f2fs_fs_context ctx;
>>> - struct fs_context fc;
>>> struct inode *root;
>>> int err;
>>> bool skip_recovery = false, need_fsck = false;
>>> - char *options = NULL;
>>> int recovery, i, valid_super_block;
>>> struct curseg_info *seg_i;
>>> int retry_cnt = 1;
>>> @@ -4767,9 +4711,6 @@ static int f2fs_fill_super(struct super_block *sb,
>>> void *data, int silent)
>>> raw_super = NULL;
>>> valid_super_block = -1;
>>> recovery = 0;
>>> - memset(&fc, 0, sizeof(fc));
>>> - memset(&ctx, 0, sizeof(ctx));
>>> - fc.fs_private = &ctx;
>>> /* allocate memory for f2fs-specific super block info */
>>> sbi = kzalloc(sizeof(struct f2fs_sb_info), GFP_KERNEL);
>>> @@ -4820,22 +4761,12 @@ static int f2fs_fill_super(struct super_block *sb,
>>> void *data, int silent)
>>> sizeof(raw_super->uuid));
>>> default_options(sbi, false);
>>> - /* parse mount options */
>>> - options = kstrdup((const char *)data, GFP_KERNEL);
>>> - if (data && !options) {
>>> - err = -ENOMEM;
>>> - goto free_sb_buf;
>>> - }
>>> -
>>> - err = parse_options(&fc, options);
>>> - if (err)
>>> - goto free_options;
>>> - err = f2fs_check_opt_consistency(&fc, sb);
>>> + err = f2fs_check_opt_consistency(fc, sb);
>>> if (err < 0)
>>> - goto free_options;
>>> + goto free_sb_buf;
>>> - f2fs_apply_options(&fc, sb);
>>> + f2fs_apply_options(fc, sb);
>>> sb->s_maxbytes = max_file_blocks(NULL) <<
>>> le32_to_cpu(raw_super->log_blocksize);
>>> @@ -5160,7 +5091,6 @@ static int f2fs_fill_super(struct super_block *sb,
>>> void *data, int silent)
>>> if (err)
>>> goto sync_free_meta;
>>> }
>>> - kvfree(options);
>>> /* recover broken superblock */
>>> if (recovery) {
>>> @@ -5255,7 +5185,6 @@ static int f2fs_fill_super(struct super_block *sb,
>>> void *data, int silent)
>>> kfree(F2FS_OPTION(sbi).s_qf_names[i]);
>>> #endif
>>> fscrypt_free_dummy_policy(&F2FS_OPTION(sbi).dummy_enc_policy);
>>> - kvfree(options);
>>> free_sb_buf:
>>> kfree(raw_super);
>>> free_sbi:
>>> @@ -5271,14 +5200,39 @@ static int f2fs_fill_super(struct super_block *sb,
>>> void *data, int silent)
>>> return err;
>>> }
>>> -static struct dentry *f2fs_mount(struct file_system_type *fs_type, int
>>> flags,
>>> - const char *dev_name, void *data)
>>> +static int f2fs_get_tree(struct fs_context *fc)
>>> {
>>> - return mount_bdev(fs_type, flags, dev_name, data, f2fs_fill_super);
>>> + return get_tree_bdev(fc, f2fs_fill_super);
>>> +}
>>> +
>>> +static int f2fs_reconfigure(struct fs_context *fc)
>>> +{
>>> + struct super_block *sb = fc->root->d_sb;
>>> +
>>> + return __f2fs_remount(fc, sb);
>>> +}
>>> +
>>> +static void f2fs_fc_free(struct fs_context *fc)
>>> +{
>>> + struct f2fs_fs_context *ctx = fc->fs_private;
>>> + int i;
>>> +
>>> + if (!ctx)
>>> + return;
>>> +
>>> +#ifdef CONFIG_QUOTA
>>> + for (i = 0; i < MAXQUOTAS; i++)
>>> + kfree(F2FS_CTX_INFO(ctx).s_qf_names[i]);
>>> +#endif
>>> + fscrypt_free_dummy_policy(&F2FS_CTX_INFO(ctx).dummy_enc_policy);
>>> + kfree(ctx);
>>> }
>>> static const struct fs_context_operations f2fs_context_ops = {
>>> .parse_param = f2fs_parse_param,
>>> + .get_tree = f2fs_get_tree,
>>> + .reconfigure = f2fs_reconfigure,
>>> + .free = f2fs_fc_free,
>>> };
>>> static void kill_f2fs_super(struct super_block *sb)
>>> @@ -5322,10 +5276,24 @@ static void kill_f2fs_super(struct super_block *sb)
>>> }
>>> }
>>> +static int f2fs_init_fs_context(struct fs_context *fc)
>>> +{
>>> + struct f2fs_fs_context *ctx;
>>> +
>>> + ctx = kzalloc(sizeof(struct f2fs_fs_context), GFP_KERNEL);
>>> + if (!ctx)
>>> + return -ENOMEM;
>>> +
>>> + fc->fs_private = ctx;
>>> + fc->ops = &f2fs_context_ops;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static struct file_system_type f2fs_fs_type = {
>>> .owner = THIS_MODULE,
>>> .name = "f2fs",
>>> - .mount = f2fs_mount,
>>> + .init_fs_context = f2fs_init_fs_context,
>>> .kill_sb = kill_f2fs_super,
>>> .fs_flags = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
>>> };
>>
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
