linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: james_p_freyensee@linux.intel.com (Jay Freyensee)
Subject: [PATCH RFC 1/4] fabrics: define admin sqsize min default, per spec
Date: Wed, 10 Aug 2016 21:07:15 -0700	[thread overview]
Message-ID: <1470888438-823-2-git-send-email-james_p_freyensee@linux.intel.com> (raw)
In-Reply-To: <1470888438-823-1-git-send-email-james_p_freyensee@linux.intel.com>

Upon admin queue connect(), the rdma qp was being
set based on NVMF_AQ_DEPTH.  However, the fabrics layer was
using the sqsize field value set for I/O queues for the admin
queue, which through the nvme layer and rdma layer off-whack:

root at fedora23-fabrics-host1 nvmf]# dmesg
[ 3507.798642] nvme_fabrics: nvmf_connect_admin_queue():admin sqsize
being sent is: 128
[ 3507.798858] nvme nvme0: creating 16 I/O queues.
[ 3507.896407] nvme nvme0: new ctrl: NQN "nullside-nqn", addr
192.168.1.3:4420

Thus, to have a different admin queue value (which the fabrics
spec states the minimum depth for a fabrics admin queue is 32 via
the ASQSZ definition), we need also a new variable to hold
the sqsize for admin fabrics queue.  This also allows fabric
implementation layers to set an admin sqsize greater than the
specification-defined minimum, which the fabrics spec allows.

Reported-by: Daniel Verkamp <daniel.verkamp at intel.com>
Signed-off-by: Jay Freyensee <james_p_freyensee at linux.intel.com>
---
 drivers/nvme/host/fabrics.c | 16 +++++++++++++++-
 drivers/nvme/host/nvme.h    |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index dc99676..cfc3607 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -363,7 +363,14 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
 	cmd.connect.opcode = nvme_fabrics_command;
 	cmd.connect.fctype = nvme_fabrics_type_connect;
 	cmd.connect.qid = 0;
-	cmd.connect.sqsize = cpu_to_le16(ctrl->sqsize);
+
+	/*
+	 * fabrics spec sets a minimum of depth 32 for admin queue, but
+	 * fabric implementation layer may choose to define something deeper.
+	 */
+	cmd.connect.sqsize = (ctrl->admin_sqsize < NVMF_AQ_DEPTH-1) ?
+			     cpu_to_le16(NVMF_AQ_DEPTH-1) :
+			     cpu_to_le16(ctrl->admin_sqsize);
 	/*
 	 * Set keep-alive timeout in seconds granularity (ms * 1000)
 	 * and add a grace period for controller kato enforcement
@@ -833,6 +840,13 @@ static ssize_t nvmf_dev_write(struct file *file, const char __user *ubuf,
 
 	seq_file->private = ctrl;
 
+	/*
+	 * set admin_sqsize a minimum default value
+	 * but allow fabric implementation modules
+	 * to define a larger sqsize, if desired
+	 */
+	ctrl->admin_sqsize = NVMF_AQ_DEPTH-1;
+
 out_unlock:
 	mutex_unlock(&nvmf_dev_mutex);
 	kfree(buf);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index ab18b78..32577a7 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -137,6 +137,7 @@ struct nvme_ctrl {
 	struct delayed_work ka_work;
 
 	/* Fabrics only */
+	u16 admin_sqsize;
 	u16 sqsize;
 	u32 ioccsz;
 	u32 iorcsz;
-- 
2.7.4

  reply	other threads:[~2016-08-11  4:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-11  4:07 [PATCH RFC 0/4] sqsize zero-based fixes Jay Freyensee
2016-08-11  4:07 ` Jay Freyensee [this message]
2016-08-11  9:01   ` [PATCH RFC 1/4] fabrics: define admin sqsize min default, per spec Sagi Grimberg
2016-08-11 15:08     ` Christoph Hellwig
2016-08-11 16:33       ` J Freyensee
2016-08-11  4:07 ` [PATCH RFC 2/4] nvme-rdma: fix sqsize/hsqsize/hrqsize " Jay Freyensee
2016-08-11  7:03   ` Sagi Grimberg
2016-08-11 16:35     ` J Freyensee
2016-08-11 15:21   ` Christoph Hellwig
2016-08-11 16:40     ` J Freyensee
2016-08-11 23:24     ` J Freyensee
2016-08-11  4:07 ` [PATCH RFC 3/4] nvmet-rdma: +1 to *queue_size from hsqsize/hrqsize Jay Freyensee
2016-08-11  4:07 ` [PATCH RFC 4/4] nvme-loop: set sqsize to 0-based value, per spec Jay Freyensee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1470888438-823-2-git-send-email-james_p_freyensee@linux.intel.com \
    --to=james_p_freyensee@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).