Skip to contentSkip to conversation

nvme-fabrics: Ignore nr_io_queues option for discovery controllers

PATCH New Roland Dreier 3 participants 4 messages Mar 2, 2018 – Mar 5, 2018 NVMe

lore
Share & tools
lore

Roland Dreier

author
#0 lore raw

PATCH 1/1 nvme-fabrics: Ignore nr_io_queues option for discovery controllers

From: Roland Dreier <[email protected]>

This removes a dependency on the order options are passed when creating a fabrics controller. With the old code, if "nr_io_queues" appears before an "nqn" option specifying the discovery controller, then nr_io_queues is overridden with zero. If "nr_io_queues" appears after specifying the discovery controller, then the nr_io_queues option is used to set the number of queues, and the driver attempts to establish IO connections to the discovery controller (which doesn't work).

It seems better to ignore (and warn about) the "nr_io_queues" option if userspace has already asked to connect to the discovery controller.

Signed-off-by Roland Dreier <roland at purestorage.com>
---
 drivers/nvme/host/fabrics.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/nvme/host/fabrics.c +5 -0
index 5dd4ceefed8f..acb6ee90d026 100644
@@ -650,6 +650,11 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 				ret = -EINVAL;
 				goto out;
 			}
+			if (opts->discovery_nqn) {
+				pr_warn("Ignoring nr_io_queues value for discovery controller\n");
+				break;
+			}
+
 			opts->nr_io_queues = min_t(unsigned int,
 					num_online_cpus(), token);
 			break;
Signature · 1 line
2.15.1
#1 lore raw

PATCH 1/1 nvme-fabrics: Ignore nr_io_queues option for discovery controllers

drivers/nvme/host/fabrics.c static int nvmf_parse_options(struct nvmf_ctrl_options *opts, · line 660

Quoted 12 lines · Roland Dreier
drivers/nvme/host/fabrics.c +5 -0
@@ -650,6 +650,11 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
  				ret = -EINVAL;
  				goto out;
  			}
+			if (opts->discovery_nqn) {
+				pr_warn("Ignoring nr_io_queues value for discovery controller\n");
+				break;
+			}
+
  			opts->nr_io_queues = min_t(unsigned int,
  					num_online_cpus(), token);
  			break;

a pr_warn is a bit verbose. even a pr_warn is. I'd prefer no message at all as it's normal for a discovery controller.

-- james

maintainer
#2 lore raw

PATCH 1/1 nvme-fabrics: Ignore nr_io_queues option for discovery controllers

Quoted 10 lines
Quoted 10 lines
(quoted diff) +3 -0
@@ -650,6 +650,11 @@ static int nvmf_parse_options(struct 
 nvmf_ctrl_options *opts,
 ????????????????? ret = -EINVAL;
 ????????????????? goto out;
 ????????????? }
+??????????? if (opts->discovery_nqn) {
+??????????????? pr_warn("Ignoring nr_io_queues value for discovery 
 controller\n");
+??????????????? break;
Quoted 8 lines
Quoted 5 lines
(quoted diff) +2 -0
+??????????? }
+
 ????????????? opts->nr_io_queues = min_t(unsigned int,

????????????????????? num_online_cpus(), token);
????????????? break;

a pr_warn is a bit verbose. even a pr_warn is. I'd prefer no message at all as it's normal for a discovery controller.

Agreed, Roland can you respin with pr_debug?

author
#3 lore raw

PATCH 1/1 nvme-fabrics: Ignore nr_io_queues option for discovery controllers

Quoted 1 line

Agreed, Roland can you respin with pr_debug?

Sure, no problem.

- R.