On Fri, Dec 06, 2019 at 02:32:44PM +0900, Naohiro Aota wrote:
> >> + */
> >> + if (btrfs_test_opt(info, SPACE_CACHE)) {
> >> + btrfs_err(info,
> >> + "cannot enable disk space caching with HMZONED mode");
> >
> >"space cache v1 not supported in HMZONED mode, use v2 (free-space-tree)"
> >
> >> + return -EINVAL;
>
> Yes, we can technically use free-space-tree on HMZONED mode. But,
> since HMZONED mode now always allocate extents in a block group
> sequentially regardless of underlying device zone type, it's no use to
> enable and maintain the tree anymore.
>
> So, just telling "space cache v1 not supported in HMZONED mode" is
> better?
Ok. That v2 is possible to use but not necessary is something to put to
documentation.
> >> static inline bool btrfs_dev_is_sequential(struct btrfs_device *device, u64 pos)
> >> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> >> index 616f5abec267..d411574298f4 100644
> >> --- a/fs/btrfs/super.c
> >> +++ b/fs/btrfs/super.c
> >> @@ -442,8 +442,12 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
> >> cache_gen = btrfs_super_cache_generation(info->super_copy);
> >> if (btrfs_fs_compat_ro(info, FREE_SPACE_TREE))
> >> btrfs_set_opt(info->mount_opt, FREE_SPACE_TREE);
> >> - else if (cache_gen)
> >> - btrfs_set_opt(info->mount_opt, SPACE_CACHE);
> >> + else if (cache_gen) {
> >> + if (btrfs_fs_incompat(info, HMZONED))
> >> + WARN_ON(1);
> >
> >So this is supposed to catch invalid combination, hmzoned-compatible
> >options are verified at the beginning. 'cache_gen' can be potentially
> >non-zero (fuzzed image, accidental random overwrite from last time), so
> >I think a message should be printed. If it's possible to continue, eg.
> >completely ignoring the existing space cache that's more user friendly
> >than a plain unexplained WARN_ON.
>
> We can just ignore the generation value and continue. I'll rewrite to
> use btrfs_info(info, "ignoring existing space cache in HMZONED mode.")
> instead of WARN_ON.
Sounds good.
