Re: [PATCH v5 03/28] btrfs: Check and enable HMZONED mode

Naohiro Aota
2019-12-05T05:17:25Z

On Wed, Dec 04, 2019 at 05:07:34PM +0100, Johannes Thumshirn wrote:
>On Wed, Dec 04, 2019 at 05:17:10PM +0900, Naohiro Aota wrote:
>> HMZONED mode cannot be used together with the RAID5/6 profile for now.
>> Introduce the function btrfs_check_hmzoned_mode() to check this. This
>> function will also check if HMZONED flag is enabled on the file system and
>> if the file system consists of zoned devices with equal zone size.
>
>I have a question, you wrote you check for a file system consisting of zoned
>devices with equal zone size. What happens if you create a multi device file
>system combining zoned and regular devices? Is this even supported and if no
>where are the checks for it?

We don't allow creaing a file system mixed with zoned and regular device.
This is checked by btrfs_check_hmzoned_mode() (called from open_ctree()) at
the mount time. "if (hmzoned_devices != nr_devices) { ... }" is doing the
actual check.

# I noticed putting "fs_info->zone_size = zone_size;" after this check is
# better.

Also, btrfs_check_device_zone_type() (called from btrfs_init_new_device()
and btrfs_init_dev_replace_tgtdev()) does the similar check against new
device for "btrfs device add" and "btrfs replace".

>
>[...]
>
>> +int btrfs_check_hmzoned_mode(struct btrfs_fs_info *fs_info)
>> +{
>> + struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
>> + struct btrfs_device *device;
>> + u64 hmzoned_devices = 0;
>> + u64 nr_devices = 0;
>> + u64 zone_size = 0;
>> + int incompat_hmzoned = btrfs_fs_incompat(fs_info, HMZONED);
>> + int ret = 0;
>> +
>> + /* Count zoned devices */
>> + list_for_each_entry(device, &fs_devices->devices, dev_list) {
>> + if (!device->bdev)
>> + continue;
>
>Nit:
> enum blk_zoned_model zone_model = blk_zoned_model(device->bdev);
>
> if (zone_model == BLK_ZONED_HM ||
> zone_model == BLK_ZONED_HA &&
> incompat_hmzoned) {
>

Thanks, it's clearer.

>> + if (bdev_zoned_model(device->bdev) == BLK_ZONED_HM ||
>> + (bdev_zoned_model(device->bdev) == BLK_ZONED_HA &&
>> + incompat_hmzoned)) {
>> + 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");
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + }
>> + nr_devices++;
>> + }