[dm-devel] [PATCH] dm table: Fix zoned model check and zone sectors check

[Damien Le Moal]

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 at wdc.com>
> 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 at wdc.com>

-- 
Damien Le Moal
Western Digital Research