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
