On 9/2/22 10:56, Damien Le Moal wrote:
> 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@xxxxxx>
>> ---
>> 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) ?
Note: Only a suggestion, not sure that would be much of a cleanup.
>
> [...]
>> /* 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
