Re: [PATCH V3 5/7] f2fs: separate the options parsing and options checking

[Eric Sandeen <sandeen@redhat.com>]

On 5/6/25 5:01 PM, Jaegeuk Kim wrote:

<snip>

>> +static int f2fs_check_opt_consistency(struct fs_context *fc,
>> +				      struct super_block *sb)
>> +{
>> +	struct f2fs_fs_context *ctx = fc->fs_private;
>> +	struct f2fs_sb_info *sbi = F2FS_SB(sb);
>> +	int err;
>> +
>> +	if (ctx_test_opt(ctx, F2FS_MOUNT_NORECOVERY) && !f2fs_readonly(sb))
>> +		return -EINVAL;
>> +
>> +	if (f2fs_hw_should_discard(sbi) && (ctx->opt_mask & F2FS_MOUNT_DISCARD)
>> +				&& !ctx_test_opt(ctx, F2FS_MOUNT_DISCARD)) {
> Applied.
> 
>        if (f2fs_hw_should_discard(sbi) &&
>                        (ctx->opt_mask & F2FS_MOUNT_DISCARD) &&
>                        !ctx_test_opt(ctx, F2FS_MOUNT_DISCARD)) {
> 

yes that's nicer

>> +		f2fs_warn(sbi, "discard is required for zoned block devices");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (f2fs_sb_has_device_alias(sbi)) {
> Shouldn't this be?
> 
> 	if (f2fs_sb_has_device_alias(sbi) &&
> 			!ctx_test_opt(ctx, F2FS_MOUNT_READ_EXTENT_CACHE)) {
> 

Whoops, I don't know how I missed that, or how my testing missed it, sorry.
And maybe it should be later in the function so it doesn't interrupt the=
discard cases.
 
>> +		f2fs_err(sbi, "device aliasing requires extent cache");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (!f2fs_hw_support_discard(sbi) && (ctx->opt_mask & F2FS_MOUNT_DISCARD)
>> +				&& ctx_test_opt(ctx, F2FS_MOUNT_DISCARD)) {
>        if (!f2fs_hw_support_discard(sbi) &&
>                        (ctx->opt_mask & F2FS_MOUNT_DISCARD) &&
>                        ctx_test_opt(ctx, F2FS_MOUNT_DISCARD)) {
>