Re: [PATCH v2 1/2] scsi: Fix handling of host-aware ZBC disks

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

On Mon, Sep 14, 2020 at 09:34:47AM +0900, Damien Le Moal wrote:
> When CONFIG_BLK_DEV_ZONED is disabled, allow using host-aware ZBC
> disks as regular disks. In this case, ensure that command completion
> is correctly executed by changing sd_zbc_complete() to return good_bytes
> instead of 0, causing a hang during device probe (endless retries).
> 
> When CONFIG_BLK_DEV_ZONED is enabled and a host-aware disk is detected
> to have partitions, it will be used as a regular disk. In this case,
> make sure to not do anything in sd_zbc_revalidate_zones() as that
> triggers warnings.
> 
> Reported-by: Borislav Petkov <bp@xxxxxxxxx>
> Fixes: b72053072c0b ("block: allow partitions on host aware zone devices")
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxx>
> Tested-by: Borislav Petkov <bp@xxxxxxx>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
> ---
>  drivers/scsi/sd.c     | 28 ++++++++++++++++++++++------
>  drivers/scsi/sd.h     |  2 +-
>  drivers/scsi/sd_zbc.c |  6 +++++-
>  3 files changed, 28 insertions(+), 8 deletions(-)
> 
>  	} else {
>  		sdkp->zoned = (buffer[8] >> 4) & 3;
>  		if (sdkp->zoned == 1 && !disk_has_partitions(sdkp->disk)) {
> +			/*
> +			 * Host-aware disk without partition: use the disk as
> +			 * such if support for zoned block devices is enabled.
> +			 * Otherwise, use it as a regular disk.
> +			 */
> +			if (IS_ENABLED(CONFIG_BLK_DEV_ZONED))
> +				q->limits.zoned = BLK_ZONED_HA;
> +			else
> +				q->limits.zoned = BLK_ZONED_NONE;
>  		} else {
>  			/*
>  			 * Treat drive-managed devices and host-aware devices
>  			 * with partitions as regular block devices.
>  			 */
>  			q->limits.zoned = BLK_ZONED_NONE;
>  		}

I think we need to lift much of this into a block layer helper,
as the logic is way subtile.  Something like this (written in the MUA
editor, not even compile tested).

static inline void blk_queue_set_zoned(struct gendisk *disk,
		enum blk_zoned_model model)
{
	switch (model) {
	case BLK_ZONED_HM:
		WARN_ON_ONCE(!IS_ENABLED(CONFIG_BLK_DEV_ZONED));
		break;
	/*
	 * Host aware drivers are neither fish nor fowl.  We can either
	 * treat them like a drive managed devices, in which case they
	 * aren't different from a normal block device, or we can try
	 * to take advantage of the Zone command set, but in that case
	 * we need to treat them like a host managed device.  We try
	 * the latter if there are not partitions and zoned block device
	 * set support is enabled, else we do nothing special as far as
	 * the block layer is concerned.
	 */
	case BLK_ZONED_HA:
		if (!IS_ENABLED(CONFIG_BLK_DEV_ZONED) ||
		    disk_has_partitions(disk)) {
			model = BLK_ZONED_NONE;
		break;
	default:
		break;

	disk->queue->limits.zoned = model;
}

Then in sd do:

	if (sdkp->device->type == TYPE_ZBC) {
		blk_queue_set_zoned(sdkp->disk, BLK_ZONED_HM);
	} else {
		sdkp->zoned = (buffer[8] >> 4) & 3;
		if (sdkp->zoned == 1)
			blk_queue_set_zoned(sdkp->disk, BLK_ZONED_HA);
		else
			blk_queue_set_zoned(sdkp->disk, BLK_ZONED_NONE);
	}