Re: [PATCHv2] nvme: only use power of two io boundaries

From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: Keith Busch <kbusch@kernel.org>
Cc: sagi@grimberg.me, linux-nvme@lists.infradead.org, hch@lst.de
Subject: Re: [PATCHv2] nvme: only use power of two io boundaries
Date: Thu, 27 Aug 2020 11:48:21 -0400	[thread overview]
Message-ID: <yq1bliwm72c.fsf@ca-mkp.ca.oracle.com> (raw)
In-Reply-To: <20200827152824.2784396-1-kbusch@kernel.org> (Keith Busch's message of "Thu, 27 Aug 2020 08:28:24 -0700")

Keith,

> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 154942fc64eb..9bbbd600621f 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2097,8 +2097,12 @@ static int __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
>  		}
>  	}
>  
> -	if (iob && !blk_queue_is_zoned(ns->queue))
> -		blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> +	if (is_power_of_2(iob) && !blk_queue_is_zoned(ns->queue))
> +		blk_queue_chunk_sectors(ns->queue, iob);
> +	else if (iob && !(disk->flags & GENHD_FL_UP))
> +		dev_warn(ctrl->device, "namespace:%u has unused io boundary:%u\n",

You used "unusable" in the patch description above. Did you mean that
here too?

Maybe something like:

	"namespace%u: ignoring unsupported I/O boundary:%u\n"

to make it clear that this is due to a kernel limitation and not the
drive reporting garbage.

> +			ns->head->ns_id, iob);
> +
>  	nvme_update_disk_info(disk, ns, id);
>  #ifdef CONFIG_NVME_MULTIPATH
>  	if (ns->head->disk) {

Otherwise OK.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

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