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

Chaitanya Kulkarni
2022-04-11T12:55:05Z

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