* [PATCH 1/2] scsi: Fix handling of host-aware ZBC disks
2020-09-13 6:03 [PATCH 0/2] Fix handling of host-aware ZBC disks Damien Le Moal
@ 2020-09-13 6:03 ` Damien Le Moal
2020-09-13 7:05 ` Borislav Petkov
2020-09-13 8:03 ` Johannes Thumshirn
2020-09-13 6:03 ` [PATCH 2/2] scsi: Fix ZBC disk initialization Damien Le Moal
2020-09-13 7:05 ` [PATCH 0/2] Fix handling of host-aware ZBC disks Borislav Petkov
2 siblings, 2 replies; 7+ messages in thread
From: Damien Le Moal @ 2020-09-13 6:03 UTC (permalink / raw)
To: linux-scsi, Martin K . Petersen, Borislav Petkov
Cc: linux-block, Jens Axboe, Johannes Thumshirn
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@alien8.de>
Fixes: b72053072c0b ("block: allow partitions on host aware zone devices")
Cc: <stable@vger.kernel.org>
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
drivers/scsi/sd.c | 22 ++++++++++++++++++----
drivers/scsi/sd.h | 2 +-
drivers/scsi/sd_zbc.c | 6 +++++-
3 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 95018e650f2d..7f0371185a45 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2968,22 +2968,36 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
} else {
sdkp->zoned = (buffer[8] >> 4) & 3;
if (sdkp->zoned == 1 && !disk_has_partitions(sdkp->disk)) {
+#ifdef CONFIG_BLK_DEV_ZONED
/* Host-aware */
q->limits.zoned = BLK_ZONED_HA;
+#else
+ /* Host-aware drive is treated as a regular disk */
+ q->limits.zoned = BLK_ZONED_NONE;
+#endif
} else {
/*
* Treat drive-managed devices and host-aware devices
* with partitions as regular block devices.
*/
q->limits.zoned = BLK_ZONED_NONE;
- if (sdkp->zoned == 2 && sdkp->first_scan)
- sd_printk(KERN_NOTICE, sdkp,
- "Drive-managed SMR disk\n");
}
}
- if (blk_queue_is_zoned(q) && sdkp->first_scan)
+
+ if (!sdkp->first_scan)
+ goto out;
+
+ if (blk_queue_is_zoned(q)) {
sd_printk(KERN_NOTICE, sdkp, "Host-%s zoned block device\n",
q->limits.zoned == BLK_ZONED_HM ? "managed" : "aware");
+ } else {
+ if (sdkp->zoned == 1)
+ sd_printk(KERN_NOTICE, sdkp,
+ "Host-aware SMR disk used as regular disk\n");
+ else if (sdkp->zoned == 2)
+ sd_printk(KERN_NOTICE, sdkp,
+ "Drive-managed SMR disk\n");
+ }
out:
kfree(buffer);
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 4933e7daf17d..7251434100e6 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -259,7 +259,7 @@ static inline blk_status_t sd_zbc_setup_zone_mgmt_cmnd(struct scsi_cmnd *cmd,
static inline unsigned int sd_zbc_complete(struct scsi_cmnd *cmd,
unsigned int good_bytes, struct scsi_sense_hdr *sshdr)
{
- return 0;
+ return good_bytes;
}
static inline blk_status_t sd_zbc_prepare_zone_append(struct scsi_cmnd *cmd,
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 0e94ff056bff..a739456dea02 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -667,7 +667,11 @@ int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
u32 max_append;
int ret = 0;
- if (!sd_is_zoned(sdkp))
+ /*
+ * There is nothing to do for regular disks, including host-aware disks
+ * that have partitions.
+ */
+ if (!blk_queue_is_zoned(q))
return 0;
/*
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] scsi: Fix handling of host-aware ZBC disks
2020-09-13 6:03 ` [PATCH 1/2] scsi: " Damien Le Moal
@ 2020-09-13 7:05 ` Borislav Petkov
2020-09-13 8:03 ` Johannes Thumshirn
1 sibling, 0 replies; 7+ messages in thread
From: Borislav Petkov @ 2020-09-13 7:05 UTC (permalink / raw)
To: Damien Le Moal
Cc: linux-scsi, Martin K . Petersen, linux-block, Jens Axboe,
Johannes Thumshirn
On Sun, Sep 13, 2020 at 03:03:03PM +0900, Damien Le Moal wrote:
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 95018e650f2d..7f0371185a45 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2968,22 +2968,36 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
> } else {
> sdkp->zoned = (buffer[8] >> 4) & 3;
> if (sdkp->zoned == 1 && !disk_has_partitions(sdkp->disk)) {
> +#ifdef CONFIG_BLK_DEV_ZONED
You could make that
if (IS_ENABLED(CONFIG_BLK_DEV_ZONED))
and get rid of the ugly ifdeffery.
Just a nitpick anyway.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] scsi: Fix handling of host-aware ZBC disks
2020-09-13 6:03 ` [PATCH 1/2] scsi: " Damien Le Moal
2020-09-13 7:05 ` Borislav Petkov
@ 2020-09-13 8:03 ` Johannes Thumshirn
1 sibling, 0 replies; 7+ messages in thread
From: Johannes Thumshirn @ 2020-09-13 8:03 UTC (permalink / raw)
To: Damien Le Moal, linux-scsi, Martin K . Petersen, Borislav Petkov
Cc: linux-block, Jens Axboe
With Boris' comment worked in,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] scsi: Fix ZBC disk initialization
2020-09-13 6:03 [PATCH 0/2] Fix handling of host-aware ZBC disks Damien Le Moal
2020-09-13 6:03 ` [PATCH 1/2] scsi: " Damien Le Moal
@ 2020-09-13 6:03 ` Damien Le Moal
2020-09-13 8:05 ` Johannes Thumshirn
2020-09-13 7:05 ` [PATCH 0/2] Fix handling of host-aware ZBC disks Borislav Petkov
2 siblings, 1 reply; 7+ messages in thread
From: Damien Le Moal @ 2020-09-13 6:03 UTC (permalink / raw)
To: linux-scsi, Martin K . Petersen, Borislav Petkov
Cc: linux-block, Jens Axboe, Johannes Thumshirn
Make sure to call sd_zbc_init_disk() when the sdkp->zoned field is
known, that is, once sd_read_block_characteristics() is executed in
sd_revalidate_disk(), so that host-aware disks also get initialized.
To do so, move sd_zbc_init_disk() call in sd_zbc_revalidate_zones() and
make sure to execute it for all zoned disks, including for host-aware
disks used as regular disks as these disk zoned model may be changed
back to BLK_ZONED_HA when partitions are deleted.
Reported-by: Borislav Petkov <bp@alien8.de>
Fixes: 5795eb443060 ("scsi: sd_zbc: emulate ZONE_APPEND commands")
Cc: <stable@vger.kernel.org> # v5.8+
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
drivers/scsi/sd.c | 4 ---
drivers/scsi/sd.h | 6 -----
drivers/scsi/sd_zbc.c | 60 +++++++++++++++++++++++++------------------
3 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 7f0371185a45..3d2cffb8e9aa 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3418,10 +3418,6 @@ static int sd_probe(struct device *dev)
sdkp->first_scan = 1;
sdkp->max_medium_access_timeouts = SD_MAX_MEDIUM_TIMEOUTS;
- error = sd_zbc_init_disk(sdkp);
- if (error)
- goto out_free_index;
-
sd_revalidate_disk(gd);
gd->flags = GENHD_FL_EXT_DEVT;
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 7251434100e6..a3aad608bc38 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -215,7 +215,6 @@ static inline int sd_is_zoned(struct scsi_disk *sdkp)
#ifdef CONFIG_BLK_DEV_ZONED
-int sd_zbc_init_disk(struct scsi_disk *sdkp);
void sd_zbc_release_disk(struct scsi_disk *sdkp);
int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned char *buffer);
int sd_zbc_revalidate_zones(struct scsi_disk *sdkp);
@@ -231,11 +230,6 @@ blk_status_t sd_zbc_prepare_zone_append(struct scsi_cmnd *cmd, sector_t *lba,
#else /* CONFIG_BLK_DEV_ZONED */
-static inline int sd_zbc_init_disk(struct scsi_disk *sdkp)
-{
- return 0;
-}
-
static inline void sd_zbc_release_disk(struct scsi_disk *sdkp) {}
static inline int sd_zbc_read_zones(struct scsi_disk *sdkp,
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index a739456dea02..cf07b7f93579 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -651,6 +651,28 @@ static void sd_zbc_print_zones(struct scsi_disk *sdkp)
sdkp->zone_blocks);
}
+static int sd_zbc_init_disk(struct scsi_disk *sdkp)
+{
+ sdkp->zones_wp_offset = NULL;
+ spin_lock_init(&sdkp->zones_wp_offset_lock);
+ sdkp->rev_wp_offset = NULL;
+ mutex_init(&sdkp->rev_mutex);
+ INIT_WORK(&sdkp->zone_wp_offset_work, sd_zbc_update_wp_offset_workfn);
+ sdkp->zone_wp_update_buf = kzalloc(SD_BUF_SIZE, GFP_KERNEL);
+ if (!sdkp->zone_wp_update_buf)
+ return -ENOMEM;
+
+ return 0;
+}
+
+void sd_zbc_release_disk(struct scsi_disk *sdkp)
+{
+ kvfree(sdkp->zones_wp_offset);
+ sdkp->zones_wp_offset = NULL;
+ kfree(sdkp->zone_wp_update_buf);
+ sdkp->zone_wp_update_buf = NULL;
+}
+
static void sd_zbc_revalidate_zones_cb(struct gendisk *disk)
{
struct scsi_disk *sdkp = scsi_disk(disk);
@@ -667,6 +689,19 @@ int sd_zbc_revalidate_zones(struct scsi_disk *sdkp)
u32 max_append;
int ret = 0;
+ /*
+ * For all zoned disks, initialize zone append emulation data if not
+ * already done. This is necessary also for host-aware disks used as
+ * regular disks due to the presence of partitions as these partitions
+ * may be deleted and the disk zoned model changed back from
+ * BLK_ZONED_NONE to BLK_ZONED_HA.
+ */
+ if (sd_is_zoned(sdkp) && !sdkp->zone_wp_update_buf) {
+ ret = sd_zbc_init_disk(sdkp);
+ if (ret)
+ return ret;
+ }
+
/*
* There is nothing to do for regular disks, including host-aware disks
* that have partitions.
@@ -768,28 +803,3 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned char *buf)
return ret;
}
-
-int sd_zbc_init_disk(struct scsi_disk *sdkp)
-{
- if (!sd_is_zoned(sdkp))
- return 0;
-
- sdkp->zones_wp_offset = NULL;
- spin_lock_init(&sdkp->zones_wp_offset_lock);
- sdkp->rev_wp_offset = NULL;
- mutex_init(&sdkp->rev_mutex);
- INIT_WORK(&sdkp->zone_wp_offset_work, sd_zbc_update_wp_offset_workfn);
- sdkp->zone_wp_update_buf = kzalloc(SD_BUF_SIZE, GFP_KERNEL);
- if (!sdkp->zone_wp_update_buf)
- return -ENOMEM;
-
- return 0;
-}
-
-void sd_zbc_release_disk(struct scsi_disk *sdkp)
-{
- kvfree(sdkp->zones_wp_offset);
- sdkp->zones_wp_offset = NULL;
- kfree(sdkp->zone_wp_update_buf);
- sdkp->zone_wp_update_buf = NULL;
-}
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/2] Fix handling of host-aware ZBC disks
2020-09-13 6:03 [PATCH 0/2] Fix handling of host-aware ZBC disks Damien Le Moal
2020-09-13 6:03 ` [PATCH 1/2] scsi: " Damien Le Moal
2020-09-13 6:03 ` [PATCH 2/2] scsi: Fix ZBC disk initialization Damien Le Moal
@ 2020-09-13 7:05 ` Borislav Petkov
2 siblings, 0 replies; 7+ messages in thread
From: Borislav Petkov @ 2020-09-13 7:05 UTC (permalink / raw)
To: Damien Le Moal
Cc: linux-scsi, Martin K . Petersen, linux-block, Jens Axboe,
Johannes Thumshirn
Mornin',
On Sun, Sep 13, 2020 at 03:03:02PM +0900, Damien Le Moal wrote:
> I tested all this. I could recreate the hang you are seeing with
> CONFIG_BLK_DEV_ZONED disabled. The cause for this hang was that
> good_bytes always ended up being 0 for all IOs to the host-aware disk.
> The fix for this is in the first patch.
> If you could test this (on top of 5.9-rc), it would be great. Thanks !
Sure, below is the diff of both boot dmesgs, with CONFIG_BLK_DEV_ZONED
and without it. So for both:
Tested-by: Borislav Petkov <bp@suse.de>
Thanks Damien and sorry for ruining your weekend.
---
--- 09-rc4+.CONFIG_BLK_DEV_ZONED 2020-09-13 08:36:13.423999302 +0200
+++ 09-rc4+.CONFIG_BLK_DEV_ZONED.off 2020-09-13 08:43:45.371999496 +0200
@@ -825,28 +825,26 @@ input: DATACOMP SteelS쀁̄Љ̒DATA Cons
hid-generic 0003:04B4:0101.0002: input,hidraw1: USB HID v1.00 Device [DATACOMP SteelS쀁̄Љ̒DATA] on usb-0000:03:00.0-12/input1
usb 1-13: new low-speed USB device number 3 using xhci_hcd
usb 1-13: New USB device found, idVendor=046d, idProduct=c018, bcdDevice=43.01
-ata4: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
usb 1-13: New USB device strings: Mfr=1, Product=2, SerialNumber=0
-ata4.00: NCQ Send/Recv Log not supported
usb 1-13: Product: USB Optical Mouse
usb 1-13: Manufacturer: Logitech
+ata4: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
+ata4.00: NCQ Send/Recv Log not supported
ata4.00: ATA-10: ST8000AS0022-1WL17Z, SN01, max UDMA/133
-ata4.00: 15628053168 sectors, multi 16: LBA48 NCQ (depth 32), AA
input: Logitech USB Optical Mouse as /devices/pci0000:00/0000:00:01.3/0000:03:00.0/usb1/1-13/1-13:1.0/0003:046D:C018.0003/input/input5
+ata4.00: 15628053168 sectors, multi 16: LBA48 NCQ (depth 32), AA
ata4.00: NCQ Send/Recv Log not supported
hid-generic 0003:046D:C018.0003: input,hidraw2: USB HID v1.11 Mouse [Logitech USB Optical Mouse] on usb-0000:03:00.0-13/input0
ata4.00: configured for UDMA/133
scsi 3:0:0:0: Direct-Access ATA ST8000AS0022-1WL SN01 PQ: 0 ANSI: 5
sd 3:0:0:0: Attached scsi generic sg1 type 0
-sd 3:0:0:0: [sdb] Host-aware zoned block device
+sd 3:0:0:0: [sdb] Host-aware SMR disk used as regular disk
sd 3:0:0:0: [sdb] 15628053168 512-byte logical blocks: (8.00 TB/7.28 TiB)
sd 3:0:0:0: [sdb] 4096-byte physical blocks
sd 3:0:0:0: [sdb] Write Protect is off
sd 3:0:0:0: [sdb] Mode Sense: 00 3a 00 00
sd 3:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
-sd 3:0:0:0: [sdb] 29808 zones of 524288 logical blocks + 1 runt zone
sdb: sdb1
-sdb: disabling host aware zoned block device support due to partitions
sd 3:0:0:0: [sdb] Attached SCSI disk
ata5: failed to resume link (SControl 0)
ata5: SATA link down (SStatus 0 SControl 0)
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 7+ messages in thread