Re: [PATCH v9 05/41] btrfs: Check and enable ZONED mode

From: Naohiro Aota <naohiro.aota@wdc.com>
To: dsterba@suse.cz, linux-btrfs@vger.kernel.org, dsterba@suse.com,
	hare@suse.com, linux-fsdevel@vger.kernel.org,
	Johannes Thumshirn <johannes.thumshirn@wdc.com>,
	Damien Le Moal <damien.lemoal@wdc.com>,
	Josef Bacik <josef@toxicpanda.com>
Subject: Re: [PATCH v9 05/41] btrfs: Check and enable ZONED mode
Date: Fri, 6 Nov 2020 18:36:24 +0900	[thread overview]
Message-ID: <20201106093624.asuonbl75g52uu7s@naota.dhcp.fujisawa.hgst.com> (raw)
In-Reply-To: <20201103121345.GP6756@twin.jikos.cz>

On Tue, Nov 03, 2020 at 01:13:45PM +0100, David Sterba wrote:
>Below are suggested message updates, common prefix "zoned:" in case it
>happens inside the zone mode implementation. Some of them sound strange
>when repeating 'zoned', but for clarity I think it should stay, unless
>somebody has a better suggestion.
>
>On Fri, Oct 30, 2020 at 10:51:12PM +0900, Naohiro Aota wrote:
>> index aac3d6f4e35b..25fd4e97dd2a 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -3595,4 +3601,8 @@ static inline int btrfs_is_testing(struct btrfs_fs_info *fs_info)
>>  }
>>  #endif
>>
>> +static inline bool btrfs_is_zoned(struct btrfs_fs_info *fs_info)
>> +{
>> +	return fs_info->zoned != 0;
>> +}
>
>newline
>
>>  #endif
>
>> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
>> index 6f6d77224c2b..5e3554482af1 100644
>> --- a/fs/btrfs/dev-replace.c
>> +++ b/fs/btrfs/dev-replace.c
>> @@ -238,6 +238,13 @@ static int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
>>  		return PTR_ERR(bdev);
>>  	}
>>
>> +	if (!btrfs_check_device_zone_type(fs_info, bdev)) {
>> +		btrfs_err(fs_info,
>> +			  "zone type of target device mismatch with the filesystem!");
>
>		"dev-replace: zoned type of target device mismatch with filesystem"
>
>> +		ret = -EINVAL;
>> +		goto error;
>> +	}
>> +
>>  	sync_blockdev(bdev);
>>
>>  	list_for_each_entry(device, &fs_info->fs_devices->devices, dev_list) {
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 764001609a15..9bc51cff48b8 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -3130,7 +3133,15 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>>
>>  	btrfs_free_extra_devids(fs_devices, 1);
>>
>> +	ret = btrfs_check_zoned_mode(fs_info);
>> +	if (ret) {
>> +		btrfs_err(fs_info, "failed to init ZONED mode: %d",
>
>		"failed to inititialize zoned mode: %d"
>
>> +				ret);
>> +		goto fail_block_groups;
>> +	}
>> +
>>  	ret = btrfs_sysfs_add_fsid(fs_devices);
>> +
>>  	if (ret) {
>>  		btrfs_err(fs_info, "failed to init sysfs fsid interface: %d",
>>  				ret);
>> --- a/fs/btrfs/zoned.c
>> +++ b/fs/btrfs/zoned.c
>> +	u64 nr_devices = 0;
>> +	u64 zone_size = 0;
>> +	int incompat_zoned = btrfs_is_zoned(fs_info);
>
>	const bool
>
>> +	int ret = 0;
>> +
>> +	/* Count zoned devices */
>> +	list_for_each_entry(device, &fs_devices->devices, dev_list) {
>> +		enum blk_zoned_model model;
>> +
>> +		if (!device->bdev)
>> +			continue;
>> +
>> +		model = bdev_zoned_model(device->bdev);
>> +		if (model == BLK_ZONED_HM ||
>> +		    (model == BLK_ZONED_HA && incompat_zoned)) {
>> +			hmzoned_devices++;
>> +			if (!zone_size) {
>> +				zone_size = device->zone_info->zone_size;
>> +			} else if (device->zone_info->zone_size != zone_size) {
>> +				btrfs_err(fs_info,
>> +					  "Zoned block devices must have equal zone sizes");
>
>				"zoned: unequal block device zone sizes: have %u found %u"
>
>> +				ret = -EINVAL;
>> +				goto out;
>> +			}
>> +		}
>> +		nr_devices++;
>> +	}
>> +
>> +	if (!hmzoned_devices && !incompat_zoned)
>> +		goto out;
>> +
>> +	if (!hmzoned_devices && incompat_zoned) {
>> +		/* No zoned block device found on ZONED FS */
>> +		btrfs_err(fs_info,
>> +			  "ZONED enabled file system should have zoned devices");
>
>		"zoned: no zoned devices found on a zoned filesystem"
>
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	if (hmzoned_devices && !incompat_zoned) {
>> +		btrfs_err(fs_info,
>> +			  "Enable ZONED mode to mount HMZONED device");
>
>Is HMZONED reference leftover from previous iterations?
>
>		"zoned: mode not enabled but zoned device found"

It was intentional here to use "hmzoned", because we can technically use
Host-Aware Zoned device as regular device, so I'd like to distinguish
hmzoned vs hazoned. But, reading the code again, I changed my mind. We can
just say "a HMZONED device" or "a HAZONED device in ZONED mode" as a zoned
device. Actually, I was not counting "hazoned_devices" anyway. So, I'll
rename hmzoned_devices to zoned_devices in the next version.