Re: [PATCH v10 07/41] btrfs: disallow space_cache in ZONED mode

[Anand Jain <anand.jain@oracle.com>]

On 19/11/20 6:42 pm, Anand Jain wrote:
> 
> 
>> @@ -985,6 +992,8 @@ int btrfs_parse_options(struct btrfs_fs_info 
>> *info, char *options,
>>           ret = -EINVAL;
>>       }
>> +    if (!ret)
>> +        ret = btrfs_check_mountopts_zoned(info);
>>       if (!ret && btrfs_test_opt(info, SPACE_CACHE))
>>           btrfs_info(info, "disk space caching is enabled");
>>       if (!ret && btrfs_test_opt(info, FREE_SPACE_TREE))
>> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
>> index 2897432eb43c..d6b8165e2c91 100644
>> --- a/fs/btrfs/zoned.c
>> +++ b/fs/btrfs/zoned.c
>> @@ -274,3 +274,21 @@ int btrfs_check_zoned_mode(struct btrfs_fs_info 
>> *fs_info)
>>   out:
>>       return ret;
>>   }
>> +
>> +int btrfs_check_mountopts_zoned(struct btrfs_fs_info *info)
>> +{
>> +    if (!btrfs_is_zoned(info))
>> +        return 0;
>> +
>> +    /*
>> +     * Space cache writing is not COWed. Disable that to avoid write
>> +     * errors in sequential zones.
>> +     */
>> +    if (btrfs_test_opt(info, SPACE_CACHE)) {
>> +        btrfs_err(info,
>> +              "zoned: space cache v1 is not supported");
>> +        return -EINVAL;
>> +    }
>> +
>> +    return 0;
>> +}
>> diff --git a/fs/btrfs/zoned.h b/fs/btrfs/zoned.h
>> index 52aa6af5d8dc..81c00a3ed202 100644
>> --- a/fs/btrfs/zoned.h
>> +++ b/fs/btrfs/zoned.h
>> @@ -25,6 +25,7 @@ int btrfs_get_dev_zone(struct btrfs_device *device, 
>> u64 pos,
>>   int btrfs_get_dev_zone_info(struct btrfs_device *device);
>>   void btrfs_destroy_dev_zone_info(struct btrfs_device *device);
>>   int btrfs_check_zoned_mode(struct btrfs_fs_info *fs_info);
>> +int btrfs_check_mountopts_zoned(struct btrfs_fs_info *info);
>>   #else /* CONFIG_BLK_DEV_ZONED */
>>   static inline int btrfs_get_dev_zone(struct btrfs_device *device, 
>> u64 pos,
>>                        struct blk_zone *zone)
>> @@ -48,6 +49,11 @@ static inline int btrfs_check_zoned_mode(struct 
>> btrfs_fs_info *fs_info)
>>       return -EOPNOTSUPP;
>>   }
>> +static inline int btrfs_check_mountopts_zoned(struct btrfs_fs_info 
>> *info)
>> +{
>> +    return 0;
>> +}
>> +
>>   #endif
>>   static inline bool btrfs_dev_is_sequential(struct btrfs_device 
>> *device, u64 pos)
>>
> 
> The whole of the above code can be replaced by..
> 



> -------------------
> @@ -810,8 +810,15 @@ int btrfs_parse_options(struct btrfs_fs_info *info, 
> char *options,
>                          break;
>                  case Opt_space_cache:
>                  case Opt_space_cache_version:
>                          if (token == Opt_space_cache ||
>                              strcmp(args[0].from, "v1") == 0) {
> +                               if (btrfs_is_zoned(info)) {
> +                                       btrfs_err(info,
> +                                       "zoned: space cache v1 is not 
> supported");
> +                                       ret = -EINVAL;
> +                                       goto out;
> +                               }
>                                  btrfs_clear_opt(info->mount_opt,
>                                                  FREE_SPACE_TREE);
>                                  btrfs_set_and_info(info, SPACE_CACHE,
> -------------------
> 


Later patches add more flag-checks to btrfs_check_mountopts_zoned(),
I have no preference for either this way or adding those checks
individually in btrfs_parse_options(). So you may ignore this.

Thanks.

> Thanks.