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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020/09/14 16:20, Christoph Hellwig wrote:
> 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);
> 	}
> 

Yes, that's nice. Will send something along these lines. But since this is a bug
fix for the current cycle & stable, we probably should keep the patch as is and
add the improvement on top for 5.10, no ?

-- 
Damien Le Moal
Western Digital Research




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux