public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Damien Le Moal <damien.lemoal@wdc.com>
Cc: linux-scsi@vger.kernel.org,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	Borislav Petkov <bp@suse.de>,
	linux-block@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
	Johannes Thumshirn <johannes.thumshirn@wdc.com>
Subject: Re: [PATCH v2 1/2] scsi: Fix handling of host-aware ZBC disks
Date: Mon, 14 Sep 2020 08:20:34 +0100	[thread overview]
Message-ID: <20200914072034.GA25808@infradead.org> (raw)
In-Reply-To: <20200914003448.471624-2-damien.lemoal@wdc.com>

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);
	}

  reply	other threads:[~2020-09-14  7:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200914072034.GA25808@infradead.org \
    --to=hch@infradead.org \
    --cc=axboe@kernel.dk \
    --cc=bp@suse.de \
    --cc=damien.lemoal@wdc.com \
    --cc=johannes.thumshirn@wdc.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox