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

[Anand Jain <anand.jain@xxxxxxxxxx>]

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.