Re: [PATCHv3] nvme: skip noiob for zoned devices

From: Christoph Hellwig <hch@lst.de>
To: Keith Busch <kbusch@kernel.org>
Cc: sagi@grimberg.me, linux-nvme@lists.infradead.org, hch@lst.de
Subject: Re: [PATCHv3] nvme: skip noiob for zoned devices
Date: Tue, 18 Aug 2020 19:25:04 +0200	[thread overview]
Message-ID: <20200818172504.GA28474@lst.de> (raw)
In-Reply-To: <20200818162941.2674340-1-kbusch@kernel.org>

On Tue, Aug 18, 2020 at 09:29:41AM -0700, Keith Busch wrote:
> Zoned block devices reuse the chunk_sectors queue limit to define zone
> boundaries. If a such a device happens to also report an optimal
> boundary, log a warning of the conflicting attributes and do not use
> that to define the chunk_sectors as that may intermittently interfere
> with io splitting and zone size queries.
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> v2 -> v3:
> 
>  Emit a warning on the initial validation if a zoned device reports an
>  optimal boundary.
> 
>  drivers/nvme/host/core.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 88cff309d8e4..10cf40d2cfef 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2075,8 +2075,13 @@ static int __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
>  		}
>  	}
>  
> -	if (iob)
> -		blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> +	if (iob) {
> +		if (!blk_queue_is_zoned(disk->queue))
> +			blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> +		else if (!(disk->flags & GENHD_FL_UP))
> +			pr_warn("%s: zone namespace has unused IO boundary:%u\n",
> +				disk->disk_name, iob);

This has some overly long line and looks fairly convoluted, what
about something like:

	if (iob) {
		unsigned int chunk_sectors = rounddown_pow_of_two(iob);

		if (blk_queue_is_zoned(disk->queue) &&
		    chunk_sectors > blk_queue_zone_sectors(disk->queue))
			pr_warn("%s: ignoring too large IO boundary: %u\n",
				disk->disk_name, iob);
		else
			blk_queue_chunk_sectors(ns->queue, chunk_sectors);
	}

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme