Re: [PATCH v5 05/28] btrfs: disallow space_cache in HMZONED mode

[David Sterba <dsterba@suse.cz>]

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.