Re: [PATCH V3 7/7] f2fs: switch to the new mount api

[Chao Yu <chao@kernel.org>]

On 5/8/25 23:59, Eric Sandeen wrote:
> On 5/8/25 4:19 AM, Chao Yu wrote:
>>> @@ -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;
>> There is a retry flow during f2fs_fill_super(), I intenionally inject a
>> fault into f2fs_fill_super() to trigger the retry flow, it turns out that
>> mount option may be missed w/ below testcase:
> 
> I never did understand that retry logic (introduced in ed2e621a95d long
> ago). What errors does it expect to be able to retry, with success?

IIRC, it will retry mount if there is recovery failure due to inconsistent
metadata.

> 
> Anyway ...
> 
> Can you show me (as a patch) exactly what you did to trigger the retry,
> just so we are looking at the same thing?

You can try this?

---
 fs/f2fs/super.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 0ee783224953..10f0e66059f8 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -5066,6 +5066,12 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc)
 		goto reset_checkpoint;
 	}

+	if (retry_cnt) {
+		err = -EIO;
+		skip_recovery = true;
+		goto free_meta;
+	}
+
 	/* recover fsynced data */
 	if (!test_opt(sbi, DISABLE_ROLL_FORWARD) &&
 			!test_opt(sbi, NORECOVERY)) {
-- 
2.49.0

Thanks,

> 
>> - mkfs.f2fs -f -O encrypt /dev/vdb
>> - mount -o test_dummy_encryption /dev/vdb /mnt/f2fs/
>> : return success
>> - dmesg -c
>>
>> [   83.619982] f2fs_fill_super, retry_cnt:1
>> [   83.620914] F2FS-fs (vdb): Test dummy encryption mode enabled
>> [   83.668380] f2fs_fill_super, retry_cnt:0
>> [   83.671601] F2FS-fs (vdb): Mounted with checkpoint version = 7a8dfca5
>>
>> - mount|grep f2fs
>> /dev/vdb on /mnt/f2fs type f2fs (rw,relatime,lazytime,background_gc=on,nogc_merge,
>> discard,discard_unit=block,user_xattr,inline_xattr,acl,inline_data,inline_dentry,
>> flush_merge,barrier,extent_cache,mode=adaptive,active_logs=6,alloc_mode=reuse,
>> checkpoint_merge,fsync_mode=posix,memory=normal,errors=continue)
>>
>> The reason may be it has cleared F2FS_CTX_INFO(ctx).dummy_enc_policy in
>> f2fs_apply_test_dummy_encryption().
>>
>> static void f2fs_apply_test_dummy_encryption(struct fs_context *fc,
>> 					     struct super_block *sb)
>> {
>> 	struct f2fs_fs_context *ctx = fc->fs_private;
>> 	struct f2fs_sb_info *sbi = F2FS_SB(sb);
>>
>> 	if (!fscrypt_is_dummy_policy_set(&F2FS_CTX_INFO(ctx).dummy_enc_policy) ||
>> 		/* if already set, it was already verified to be the same */
>> 		fscrypt_is_dummy_policy_set(&F2FS_OPTION(sbi).dummy_enc_policy))
>> 		return;
>> 	F2FS_OPTION(sbi).dummy_enc_policy = F2FS_CTX_INFO(ctx).dummy_enc_policy;
>> 	memset(&F2FS_CTX_INFO(ctx).dummy_enc_policy, 0,
>> 		sizeof(F2FS_CTX_INFO(ctx).dummy_enc_policy));
>> 	f2fs_warn(sbi, "Test dummy encryption mode enabled");
>> }
>>
>> Can we save old mount_info from sbi or ctx from fc, and try to recover it
>> before we retry mount flow?
> 
> I'll have to take more time to understand this concern. But thanks for pointing
> it out.
> 
> -Eric
> 
>> Thanks,
>