Re: [PATCH 2/3] nvme-core: don't check non-mdts for disc ctrl

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

On 4/11/22 05:12, Christoph Hellwig wrote:
> On Mon, Apr 11, 2022 at 12:09:31PM +0000, Chaitanya Kulkarni wrote:
>> -	ret = nvme_init_non_mdts_limits(ctrl);
>> -	if (ret < 0)
>> -		return ret;
>> +	if (ctrl->cntrltype == NVME_CTRL_IO) {
>> +		ret = nvme_init_non_mdts_limits(ctrl);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
> 
> This looks better, but I don't think it will actually work as-is.
> The CNTRLTYPE field is never than discovry controllers.  We
> special case them in nvme_init_subsystem, which should move to
> nvme_init_identify instead so that ctrl->cntrltype is always valid
> after that.
> 

hmmm I think let's keep it simple with following tagset check than 
adding more code to make sure ctrl->cntrltype is valid:-


nvme (nvme-5.18) # cat 
0001-nvme-core-check-non-mdts-only-for-I-O-ctrl.patch
 From ee8439487a88b08e5abb6c6558ca6a902a0e6b1d Mon Sep 17 00:00:00 2001
From: Chaitanya Kulkarni <kch@nvidia.com>
Date: Fri, 8 Apr 2022 15:50:10 -0700
Subject: [PATCH] nvme-core: check non-mdts only for I/O ctrl

Validate ctrl is an I/O ctrl before checking the non mdts limits by
checking the ctrl->tagset member, as ctrl->tagset only allocated for
the I/O queues for the controller from :-

1. nvme_fc_create_io_queues(struct nvme_fc_ctrl *ctrl)
2. nvme_rdma_configure_io_queues(struct nvme_rdma_ctrl *ctrl, bool new)
3. nvme_tcp_configure_io_queues(struct nvme_ctrl *ctrl, bool new)
4. nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
5. nvme_dev_add()

this also masks the following warning reported by the nvme_log_error()
when running blktest nvme/002: -

[ 2005.155946] run blktests nvme/002 at 2022-04-09 16:57:47
[ 2005.192223] loop: module loaded
[ 2005.196429] nvmet: adding nsid 1 to subsystem blktests-subsystem-0
[ 2005.200334] nvmet: adding nsid 1 to subsystem blktests-subsystem-1

<------------------------------SNIP---------------------------------->

[ 2008.958108] nvmet: adding nsid 1 to subsystem blktests-subsystem-997
[ 2008.962082] nvmet: adding nsid 1 to subsystem blktests-subsystem-998
[ 2008.966102] nvmet: adding nsid 1 to subsystem blktests-subsystem-999
[ 2008.973132] nvmet: creating discovery controller 1 for subsystem 
nqn.2014-08.org.nvmexpress.discovery for NQN testhostnqn.
*[ 2008.973196] nvme1: Identify(0x6), Invalid Field in Command (sct 0x0 
/ sc 0x2) MORE DNR*
[ 2008.974595] nvme nvme1: new ctrl: "nqn.2014-08.org.nvmexpress.discovery"
[ 2009.103248] nvme nvme1: Removing ctrl: NQN 
"nqn.2014-08.org.nvmexpress.discovery"

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
  drivers/nvme/host/core.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e8d6a1e52083..2e5779d233f2 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3101,9 +3101,12 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
  	if (ret)
  		return ret;

-	ret = nvme_init_non_mdts_limits(ctrl);
-	if (ret < 0)
-		return ret;
+	/* only check non mdts for the I/O controller */
+	if (ctrl->tagset) {
+		ret = nvme_init_non_mdts_limits(ctrl);
+		if (ret < 0)
+			return ret;
+	}

  	ret = nvme_configure_apst(ctrl);
  	if (ret < 0)
-- 
2.29.0

-ck