On 2021/03/10 17:25, Shin'ichiro Kawasaki wrote:
> Commit 24f6b6036c9e ("dm table: fix zoned iterate_devices based device
> capability checks") triggered dm table load failure when dm-zoned device
> is set up for zoned block devices and a regular device for cache.
>
> The commit inverted logic of two callback functions for iterate_devices:
> device_is_zoned_model() and device_matches_zone_sectors(). The logic of
> device_is_zoned_model() was inverted then all destination devices of all
> targets in dm table are required to have the expected zoned model. This
> is fine for dm-linear, dm-flakey and dm-crypt on zoned block devices
> since each target has only one destination device. However, this results
> in failure for dm-zoned with regular cache device since that target has
> both regular block device and zoned block devices.
>
> As for device_matches_zone_sectors(), the commit inverted the logic to
> require all zoned block devices in each target have the specified
> zone_sectors. This check also fails for regular block device which does
> not have zones.
>
> To avoid the check failures, fix the zone model check and the zone
> sectors check. For zone model check, invert the device_is_zoned_model()
> logic again to require at least one destination device in one target has
> the specified zoned model. For zone sectors check, skip the check if the
> destination device is not a zoned block device. Also add comments and
> improve error messages to clarify expectations to the two checks.
>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> Fixes: 24f6b6036c9e ("dm table: fix zoned iterate_devices based device capability checks")
> ---
> drivers/md/dm-table.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 95391f78b8d5..04b7a3978ef8 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -1585,13 +1585,13 @@ bool dm_table_has_no_data_devices(struct dm_table *table)
> return true;
> }
>
> -static int device_not_zoned_model(struct dm_target *ti, struct dm_dev *dev,
> - sector_t start, sector_t len, void *data)
> +static int device_is_zoned_model(struct dm_target *ti, struct dm_dev *dev,
> + sector_t start, sector_t len, void *data)
> {
> struct request_queue *q = bdev_get_queue(dev->bdev);
> enum blk_zoned_model *zoned_model = data;
>
> - return blk_queue_zoned_model(q) != *zoned_model;
> + return blk_queue_zoned_model(q) == *zoned_model;
> }
>
> static bool dm_table_supports_zoned_model(struct dm_table *t,
> @@ -1608,7 +1608,7 @@ static bool dm_table_supports_zoned_model(struct dm_table *t,
> return false;
>
> if (!ti->type->iterate_devices ||
> - ti->type->iterate_devices(ti, device_not_zoned_model, &zoned_model))
> + !ti->type->iterate_devices(ti, device_is_zoned_model, &zoned_model))
> return false;
> }
>
> @@ -1621,9 +1621,18 @@ static int device_not_matches_zone_sectors(struct dm_target *ti, struct dm_dev *
> struct request_queue *q = bdev_get_queue(dev->bdev);
> unsigned int *zone_sectors = data;
>
> + if (blk_queue_zoned_model(q) == BLK_ZONED_NONE)
> + return 0;
> +
> return blk_queue_zone_sectors(q) != *zone_sectors;
> }
>
> +/*
> + * Check consistency of zoned model and zone sectors across all targets.
> + * For zoned model, at least one destination device used by each target shall
> + * have the zoned model. For zone sectors, if the destination device is a zoned
> + * block device, it shall have the specified zone_sectors.
> + */
> static int validate_hardware_zoned_model(struct dm_table *table,
> enum blk_zoned_model zoned_model,
> unsigned int zone_sectors)
> @@ -1632,7 +1641,7 @@ static int validate_hardware_zoned_model(struct dm_table *table,
> return 0;
>
> if (!dm_table_supports_zoned_model(table, zoned_model)) {
> - DMERR("%s: zoned model is not consistent across all devices",
> + DMERR("%s: zoned model is not consistent across all targets",
> dm_device_name(table->md));
> return -EINVAL;
> }
> @@ -1642,7 +1651,7 @@ static int validate_hardware_zoned_model(struct dm_table *table,
> return -EINVAL;
>
> if (dm_table_any_dev_attr(table, device_not_matches_zone_sectors, &zone_sectors)) {
> - DMERR("%s: zone sectors is not consistent across all devices",
> + DMERR("%s: zone sectors is not consistent across all zoned devices",
> dm_device_name(table->md));
> return -EINVAL;
> }
>
Looks good to me.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>
--
Damien Le Moal
Western Digital Research
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
