Re: [PATCH 15/17] btrfs: calculate file system wide queue limit for zoned mode

Damien Le Moal
2022-09-02T01:56:48Z

On 9/1/22 16:42, Christoph Hellwig wrote:
> To be able to split a write into properly sized zone append commands,
> we need a queue_limits structure that contains the least common
> denominator suitable for all devices.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/btrfs/ctree.h | 4 +++-
> fs/btrfs/zoned.c | 36 ++++++++++++++++++------------------
> fs/btrfs/zoned.h | 1 -
> 3 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 5e57e3c6a1fd6..a37129363e184 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1071,8 +1071,10 @@ struct btrfs_fs_info {
> */
> u64 zone_size;
>
> - /* Max size to emit ZONE_APPEND write command */
> + /* Constraints for ZONE_APPEND commands: */
> + struct queue_limits limits;
> u64 max_zone_append_size;

Can't we get rid of this one and have the code directly use
fs_info->limits.max_zone_append_sectors through a little helper doing a
conversion to bytes (a 9 bit shift) ?

[...]
> /* Count zoned devices */
> list_for_each_entry(device, &fs_devices->devices, dev_list) {
> enum blk_zoned_model model;
> @@ -685,11 +677,9 @@ int btrfs_check_zoned_mode(struct btrfs_fs_info *fs_info)
> ret = -EINVAL;
> goto out;
> }
> - if (!max_zone_append_size ||
> - (zone_info->max_zone_append_size &&
> - zone_info->max_zone_append_size < max_zone_append_size))
> - max_zone_append_size =
> - zone_info->max_zone_append_size;
> + blk_stack_limits(lim,
> + &bdev_get_queue(device->bdev)->limits,
> + 0);

This does:

t->max_zone_append_sectors = min(t->max_zone_append_sectors,
b->max_zone_append_sectors);

So if we are mixing zoned and non-zoned devices in a multi-dev volume,
we'll end up with max_zone_append_sectors being 0. The previous code
prevented that.

Note that I am not sure if it is allowed to mix zoned and non-zoned drives
in the same volume. Given that we have a fake zone emulation for non-zoned
drives with zoned btrfs, I do not see why it would not work. But I may be
wrong.

--
Damien Le Moal
Western Digital Research