[PATCH] nvme-fabrics: Ignore nr_io_queues option for discovery controllers

[PATCH] nvme-fabrics: Ignore nr_io_queues option for discovery controllers

[Roland Dreier]

From: Roland Dreier <roland@purestorage.com>

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(+)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 5dd4ceefed8f..acb6ee90d026 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -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;
-- 
2.15.1

[PATCH] nvme-fabrics: Ignore nr_io_queues option for discovery controllers

[James Smart]

On 3/2/2018 9:45 AM, Roland Dreier wrote:
> @@ -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

[PATCH] nvme-fabrics: Ignore nr_io_queues option for discovery controllers

[Sagi Grimberg]

>> @@ -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.

Agreed, Roland can you respin with pr_debug?

[PATCH] nvme-fabrics: Ignore nr_io_queues option for discovery controllers

[Roland Dreier]

> Agreed, Roland can you respin with pr_debug?

Sure, no problem.

 - R.