* [PATCH v2 0/2] Fix handling of host-aware ZBC disks @ 2020-09-14 0:34 Damien Le Moal 2020-09-14 0:34 ` [PATCH v2 1/2] scsi: " Damien Le Moal 2020-09-14 0:34 ` [PATCH v2 2/2] scsi: Fix ZBC disk initialization Damien Le Moal 0 siblings, 2 replies; 7+ messages in thread From: Damien Le Moal @ 2020-09-14 0:34 UTC (permalink / raw) To: linux-scsi, Martin K . Petersen, Borislav Petkov Cc: linux-block, Jens Axboe, Johannes Thumshirn Martin, Two patches for this cycle (with a cc stable) to fix handling of host-aware ZBC disks that have partitions, that is, used as regular disks. The first patch fixes host-aware disk initialization and command completion processing. It also enables the use of host-aware disks as regular disks when CONFIG_BLK_DEV_ZONED is disabled. The second patch fixes the CONFIG_BLK_DEV_ZONED enabled configuration so that zone append emulation is not initialized for host-aware disks with partitions/used as regular disks. While at it, this patch also removes a problem with sd_zbc_init_disk() error handling in sd_revalidate_disk() by moving this function execution inside sd_zbc_revalidate_zones(). Borislav tested the series and confirmed that it solves his problem (thanks Borislav !) Changes from v1: * Rebased on rc5 * Use "if (IS_DEFINED())" instead of #ifdef in patch 1 Damien Le Moal (2): scsi: Fix handling of host-aware ZBC disks scsi: Fix ZBC disk initialization drivers/scsi/sd.c | 32 ++++++++++++++------- drivers/scsi/sd.h | 8 +----- drivers/scsi/sd_zbc.c | 66 ++++++++++++++++++++++++++----------------- 3 files changed, 63 insertions(+), 43 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] scsi: Fix handling of host-aware ZBC disks 2020-09-14 0:34 [PATCH v2 0/2] Fix handling of host-aware ZBC disks Damien Le Moal @ 2020-09-14 0:34 ` Damien Le Moal 2020-09-14 7:20 ` Christoph Hellwig 2020-09-14 0:34 ` [PATCH v2 2/2] scsi: Fix ZBC disk initialization Damien Le Moal 1 sibling, 1 reply; 7+ messages in thread From: Damien Le Moal @ 2020-09-14 0:34 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> Tested-by: Borislav Petkov <bp@suse.de> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> --- drivers/scsi/sd.c | 28 ++++++++++++++++++++++------ drivers/scsi/sd.h | 2 +- drivers/scsi/sd_zbc.c | 6 +++++- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 95018e650f2d..e99f57bfeff4 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2968,22 +2968,38 @@ 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)) { - /* Host-aware */ - q->limits.zoned = BLK_ZONED_HA; + /* + * 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; - 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 v2 1/2] scsi: Fix handling of host-aware ZBC disks 2020-09-14 0:34 ` [PATCH v2 1/2] scsi: " Damien Le Moal @ 2020-09-14 7:20 ` Christoph Hellwig 2020-09-14 9:03 ` Damien Le Moal 0 siblings, 1 reply; 7+ messages in thread From: Christoph Hellwig @ 2020-09-14 7:20 UTC (permalink / raw) To: Damien Le Moal Cc: linux-scsi, Martin K . Petersen, Borislav Petkov, linux-block, Jens Axboe, Johannes Thumshirn 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@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> > Tested-by: Borislav Petkov <bp@suse.de> > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > --- > 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); } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] scsi: Fix handling of host-aware ZBC disks 2020-09-14 7:20 ` Christoph Hellwig @ 2020-09-14 9:03 ` Damien Le Moal 2020-09-14 14:20 ` hch 0 siblings, 1 reply; 7+ messages in thread From: Damien Le Moal @ 2020-09-14 9:03 UTC (permalink / raw) To: hch Cc: linux-scsi, Martin K . Petersen, Borislav Petkov, linux-block, Jens Axboe, Johannes Thumshirn 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@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> >> Tested-by: Borislav Petkov <bp@suse.de> >> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> >> --- >> 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] scsi: Fix handling of host-aware ZBC disks 2020-09-14 9:03 ` Damien Le Moal @ 2020-09-14 14:20 ` hch 2020-09-14 23:26 ` Damien Le Moal 0 siblings, 1 reply; 7+ messages in thread From: hch @ 2020-09-14 14:20 UTC (permalink / raw) To: Damien Le Moal Cc: hch, linux-scsi, Martin K . Petersen, Borislav Petkov, linux-block, Jens Axboe, Johannes Thumshirn On Mon, Sep 14, 2020 at 09:03:49AM +0000, Damien Le Moal wrote: > 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 ? I'd much rather also add the trivial helper for 5.9 - otherwise we'll need to pull current mainline into the for-5.10 tree and create all kinds of mess. And just adding the helper and using it for sd only will have no side effects elsewhere. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] scsi: Fix handling of host-aware ZBC disks 2020-09-14 14:20 ` hch @ 2020-09-14 23:26 ` Damien Le Moal 0 siblings, 0 replies; 7+ messages in thread From: Damien Le Moal @ 2020-09-14 23:26 UTC (permalink / raw) To: hch Cc: linux-scsi, Martin K . Petersen, Borislav Petkov, linux-block, Jens Axboe, Johannes Thumshirn On 2020/09/14 23:20, hch@infradead.org wrote: > On Mon, Sep 14, 2020 at 09:03:49AM +0000, Damien Le Moal wrote: >> 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 ? > > I'd much rather also add the trivial helper for 5.9 - otherwise we'll > need to pull current mainline into the for-5.10 tree and create > all kinds of mess. And just adding the helper and using it for sd > only will have no side effects elsewhere. > OK. Sending a v3 with this added. -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] scsi: Fix ZBC disk initialization 2020-09-14 0:34 [PATCH v2 0/2] Fix handling of host-aware ZBC disks Damien Le Moal 2020-09-14 0:34 ` [PATCH v2 1/2] scsi: " Damien Le Moal @ 2020-09-14 0:34 ` Damien Le Moal 1 sibling, 0 replies; 7+ messages in thread From: Damien Le Moal @ 2020-09-14 0:34 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> Tested-by: Borislav Petkov <bp@suse.de> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@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 e99f57bfeff4..b3c80fa47c79 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -3420,10 +3420,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
end of thread, other threads:[~2020-09-14 23:26 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-14 0:34 [PATCH v2 0/2] Fix handling of host-aware ZBC disks Damien Le Moal 2020-09-14 0:34 ` [PATCH v2 1/2] scsi: " Damien Le Moal 2020-09-14 7:20 ` Christoph Hellwig 2020-09-14 9:03 ` Damien Le Moal 2020-09-14 14:20 ` hch 2020-09-14 23:26 ` Damien Le Moal 2020-09-14 0:34 ` [PATCH v2 2/2] scsi: Fix ZBC disk initialization Damien Le Moal
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.