Skip to contentSkip to conversation

generic NVMe target and NVMe loop driver

RFC New Christoph Hellwig 4 participants 24 messages Nov 7, 2015 – Nov 17, 2015

Subsystems (2)
NVMe NVMe target
lore
Share & tools
lore

Christoph Hellwig

author maintainer
#0 lore raw

This series continues the NVMe host drive split and also starts adding a consume for it. The consumer is mostly interesting for developers at this point as it's simply a 'loopback' NVMe device that ties the split NVMe driver fronted into the new generic NVMe target subsystem. This was developed for our NVMe over Fabrics prototype, but will also be useful for other work like Ming's virtio-nvme or event an implementation traditional PCIe NVMe using vhost.

The biggest gap at this point is aborts and resets, which are still under heavy development. Once those are done and I've finished the configfs layout the code will hopefully be ready for inclusion in Linux 4.5.

I've also written a python based shell to ease configuration of the target subsystem which I'll hopefully be able to release soon.

As this depends on a lot of pending NVMe patch a git tree is also available:

http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/nvme-loop.2

author maintainer
#1 lore raw

PATCH 1/8 nvme: move set_queue_count to common code

Signed-off-by Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/host/core.c | 17 +++++++++++++++++
 drivers/nvme/host/nvme.h |  1 +
 drivers/nvme/host/pci.c  | 19 +------------------
 3 files changed, 19 insertions(+), 18 deletions(-)
drivers/nvme/host/core.c +17 -0
index 3600a0c..a63a71a 100644
@@ -334,6 +334,23 @@ int nvme_get_log_page(struct nvme_ctrl *dev, struct nvme_smart_log **log)
 	return error;
 }
 
+int nvme_set_queue_count(struct nvme_ctrl *ctrl, int count)
+{
+	int status;
+	u32 result;
+	u32 q_count = (count - 1) | ((count - 1) << 16);
+
+	status = nvme_set_features(ctrl, NVME_FEAT_NUM_QUEUES, q_count, 0,
+			&result);
+	if (status < 0)
+		return status;
+	if (status > 0) {
+		dev_err(ctrl->dev, "Could not set queue count (%d)\n", status);
+		return 0;
+	}
+	return min(result & 0xffff, result >> 16) + 1;
+}
+
 static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 {
 	struct nvme_user_io io;
drivers/nvme/host/nvme.h +1 -0
index d72bbe0..1692fda 100644
@@ -227,6 +227,7 @@ int nvme_get_features(struct nvme_ctrl *dev, unsigned fid, unsigned nsid,
 			dma_addr_t dma_addr, u32 *result);
 int nvme_set_features(struct nvme_ctrl *dev, unsigned fid, unsigned dword11,
 			dma_addr_t dma_addr, u32 *result);
+int nvme_set_queue_count(struct nvme_ctrl *ctrl, int count);
 
 extern spinlock_t dev_list_lock;
drivers/nvme/host/pci.c +1 -18
index 2ef408f..89d2442 100644
@@ -1464,23 +1464,6 @@ static void nvme_create_io_queues(struct nvme_dev *dev)
 		}
 }
 
-static int set_queue_count(struct nvme_dev *dev, int count)
-{
-	int status;
-	u32 result;
-	u32 q_count = (count - 1) | ((count - 1) << 16);
-
-	status = nvme_set_features(&dev->ctrl, NVME_FEAT_NUM_QUEUES, q_count, 0,
-								&result);
-	if (status < 0)
-		return status;
-	if (status > 0) {
-		dev_err(dev->dev, "Could not set queue count (%d)\n", status);
-		return 0;
-	}
-	return min(result & 0xffff, result >> 16) + 1;
-}
-
 static void __iomem *nvme_map_cmb(struct nvme_dev *dev)
 {
 	u64 szu, size, offset;

@@ -1545,7 +1528,7 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
 	int result, i, vecs, nr_io_queues, size;
 
 	nr_io_queues = num_possible_cpus();
-	result = set_queue_count(dev, nr_io_queues);
+	result = nvme_set_queue_count(&dev->ctrl, nr_io_queues);
 	if (result <= 0)
 		return result;
 	if (result < nr_io_queues)
Signature · 1 line
1.9.1
author maintainer
#2 lore raw

PATCH 2/8 nvme: move enable/disable/shutdown_ctrl to common code

In reply to Christoph Hellwig
Signed-off-by Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/host/core.c | 87 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/host/nvme.h |  4 +++
 drivers/nvme/host/pci.c  | 94 +++++-------------------------------------------
 3 files changed, 100 insertions(+), 85 deletions(-)
drivers/nvme/host/core.c +87 -0
index a63a71a..c061a6a 100644
@@ -14,6 +14,7 @@
 
 #include <linux/blkdev.h>
 #include <linux/blk-mq.h>
+#include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/hdreg.h>
 #include <linux/kernel.h>

@@ -32,6 +33,10 @@
 
 #define NVME_MINORS		(1U << MINORBITS)
 
+static unsigned char shutdown_timeout = 5;
+module_param(shutdown_timeout, byte, 0644);
+MODULE_PARM_DESC(shutdown_timeout, "timeout in seconds for controller shutdown");
+
 static int nvme_major;
 module_param(nvme_major, int, 0);
 

@@ -351,6 +356,88 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int count)
 	return min(result & 0xffff, result >> 16) + 1;
 }
 
+static int nvme_update_ctrl_config(struct nvme_ctrl *ctrl, u64 cap,
+		bool enabled)
+{
+	unsigned long timeout =
+		((NVME_CAP_TIMEOUT(cap) + 1) * HZ / 2) + jiffies;
+	u32 bit = enabled ? NVME_CSTS_RDY : 0, csts;
+	int error;
+
+	error = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
+	if (error)
+		return error;
+
+	while (!(error = ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts))) {
+		if ((csts & NVME_CSTS_RDY) == bit)
+			break;
+
+		msleep(100);
+		if (fatal_signal_pending(current))
+			return -EINTR;
+		if (time_after(jiffies, timeout)) {
+			dev_err(ctrl->dev,
+				"Controller not ready; aborting %s\n", enabled ?
+						"initialisation" : "reset");
+			return -ENODEV;
+		}
+	}
+
+	return error;
+}
+
+/*
+ * If the device has been passed off to us in an enabled state, just clear
+ * the enabled bit.  The spec says we should set the 'shutdown notification
+ * bits', but doing so may cause the device to complete commands to the
+ * admin queue ... and we don't know what memory that might be pointing at!
+ */
+int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
+{
+	ctrl->ctrl_config &= ~NVME_CC_SHN_MASK;
+	ctrl->ctrl_config &= ~NVME_CC_ENABLE;
+
+	return nvme_update_ctrl_config(ctrl, cap, false);
+}
+
+int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
+{
+	ctrl->ctrl_config &= ~NVME_CC_SHN_MASK;
+	ctrl->ctrl_config |= NVME_CC_ENABLE;
+
+	return nvme_update_ctrl_config(ctrl, cap, true);
+}
+
+int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
+{
+	unsigned long timeout = (shutdown_timeout * HZ) + jiffies;
+	u32 csts;
+	int error;
+
+	ctrl->ctrl_config &= ~NVME_CC_SHN_MASK;
+	ctrl->ctrl_config |= NVME_CC_SHN_NORMAL;
+
+	error = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
+	if (error)
+		return error;
+
+	while (!(error = ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts))) {
+		if ((csts & NVME_CSTS_SHST_MASK) != NVME_CSTS_SHST_CMPLT)
+			break;
+
+		msleep(100);
+		if (fatal_signal_pending(current))
+			return -EINTR;
+		if (time_after(jiffies, timeout)) {
+			dev_err(ctrl->dev,
+				"Device shutdown incomplete; abort shutdown\n");
+			return -ENODEV;
+		}
+	}
+
+	return error;
+}
+
 static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 {
 	struct nvme_user_io io;
drivers/nvme/host/nvme.h +4 -0
index 1692fda..426cafc 100644
@@ -52,6 +52,7 @@ struct nvme_ctrl {
 	char serial[20];
 	char model[40];
 	char firmware_rev[8];
+	u32 ctrl_config;
 	u32 max_hw_sectors;
 	u32 stripe_size;
 	u32 page_size;

@@ -228,6 +229,9 @@ int nvme_get_features(struct nvme_ctrl *dev, unsigned fid, unsigned nsid,
 int nvme_set_features(struct nvme_ctrl *dev, unsigned fid, unsigned dword11,
 			dma_addr_t dma_addr, u32 *result);
 int nvme_set_queue_count(struct nvme_ctrl *ctrl, int count);
+int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap);
+int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap);
+int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl);
 
 extern spinlock_t dev_list_lock;
drivers/nvme/host/pci.c +9 -85

147 lines not on this page · raw message

Signature · 1 line
1.9.1
#3 lore raw

PATCH 2/8 nvme: move enable/disable/shutdown_ctrl to common code

Maybe it'd be better to split this to two patches.
1. pass ctrl to enable/disable/shutdown_ctrl
2. move enable/disable/shutdown_ctrl to common code

Other then that looks good.

author maintainer
#4 lore raw

PATCH 2/8 nvme: move enable/disable/shutdown_ctrl to common code

Quoted 5 lines · Sagi Grimberg

Maybe it'd be better to split this to two patches.
1. pass ctrl to enable/disable/shutdown_ctrl
2. move enable/disable/shutdown_ctrl to common code

Other then that looks good.

I haven't really done that for any of the previous split patches either as it sees a bit confusing.

author maintainer
#5 lore raw

PATCH 3/8 nvme: move CC setup into nvme_enable_ctrl

In reply to Christoph Hellwig
Signed-off-by Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/host/core.c | 7 +++++--
 drivers/nvme/host/nvme.h | 2 +-
 drivers/nvme/host/pci.c  | 7 +------
 3 files changed, 7 insertions(+), 9 deletions(-)
drivers/nvme/host/core.c +5 -2
index c061a6a..2254ce9 100644
@@ -400,9 +400,12 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 	return nvme_update_ctrl_config(ctrl, cap, false);
 }
 
-int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
+int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap, unsigned page_shift)
 {
-	ctrl->ctrl_config &= ~NVME_CC_SHN_MASK;
+	ctrl->ctrl_config = NVME_CC_CSS_NVM;
+	ctrl->ctrl_config |= (page_shift - 12) << NVME_CC_MPS_SHIFT;
+	ctrl->ctrl_config |= NVME_CC_ARB_RR | NVME_CC_SHN_NONE;
+	ctrl->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;
 	ctrl->ctrl_config |= NVME_CC_ENABLE;
 
 	return nvme_update_ctrl_config(ctrl, cap, true);
drivers/nvme/host/nvme.h +1 -1
index 426cafc..8cf6fa5 100644
@@ -230,7 +230,7 @@ int nvme_set_features(struct nvme_ctrl *dev, unsigned fid, unsigned dword11,
 			dma_addr_t dma_addr, u32 *result);
 int nvme_set_queue_count(struct nvme_ctrl *ctrl, int count);
 int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap);
-int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap);
+int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap, unsigned page_shift);
 int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl);
 
 extern spinlock_t dev_list_lock;
drivers/nvme/host/pci.c +1 -6
index d50a2b7..45255e3 100644
@@ -1292,16 +1292,11 @@ static int nvme_configure_admin_queue(struct nvme_dev *dev)
 
 	dev->page_size = 1 << page_shift;
 
-	dev->ctrl.ctrl_config = NVME_CC_CSS_NVM;
-	dev->ctrl.ctrl_config |= (page_shift - 12) << NVME_CC_MPS_SHIFT;
-	dev->ctrl.ctrl_config |= NVME_CC_ARB_RR | NVME_CC_SHN_NONE;
-	dev->ctrl.ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;
-
 	writel(aqa, dev->bar + NVME_REG_AQA);
 	writeq(nvmeq->sq_dma_addr, dev->bar + NVME_REG_ASQ);
 	writeq(nvmeq->cq_dma_addr, dev->bar + NVME_REG_ACQ);
 
-	result = nvme_enable_ctrl(&dev->ctrl, cap);
+	result = nvme_enable_ctrl(&dev->ctrl, cap, page_shift);
 	if (result)
 		goto free_nvmeq;
Signature · 1 line
1.9.1
author maintainer
#6 lore raw

PATCH 4/8 nvme: move the timeout module paramters to common code

In reply to Christoph Hellwig
Signed-off-by Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/host/core.c | 8 ++++++++
 drivers/nvme/host/pci.c  | 8 --------
 2 files changed, 8 insertions(+), 8 deletions(-)
drivers/nvme/host/core.c +8 -0
index 2254ce9..6737aaf 100644
@@ -33,6 +33,14 @@
 
 #define NVME_MINORS		(1U << MINORBITS)
 
+unsigned char admin_timeout = 60;
+module_param(admin_timeout, byte, 0644);
+MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
+
+unsigned char nvme_io_timeout = 30;
+module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
+MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
+
 static unsigned char shutdown_timeout = 5;
 module_param(shutdown_timeout, byte, 0644);
 MODULE_PARM_DESC(shutdown_timeout, "timeout in seconds for controller shutdown");
drivers/nvme/host/pci.c +0 -8
index 45255e3..91522bb 100644
@@ -49,14 +49,6 @@
 #define SQ_SIZE(depth)		(depth * sizeof(struct nvme_command))
 #define CQ_SIZE(depth)		(depth * sizeof(struct nvme_completion))
 
-unsigned char admin_timeout = 60;
-module_param(admin_timeout, byte, 0644);
-MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
-
-unsigned char nvme_io_timeout = 30;
-module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
-MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
-
 static int use_threaded_interrupts;
 module_param(use_threaded_interrupts, int, 0);
Signature · 1 line
1.9.1
author maintainer
#7 lore raw

PATCH 5/8 nvme: add segment limitations

In reply to Christoph Hellwig
Signed-off-by Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/host/core.c | 11 +++++++++--
 drivers/nvme/host/nvme.h |  2 ++
 2 files changed, 11 insertions(+), 2 deletions(-)
drivers/nvme/host/core.c +9 -2
index 6737aaf..14ee152 100644
@@ -982,9 +982,16 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 	blk_queue_logical_block_size(ns->queue, 1 << ns->lba_shift);
 	if (ctrl->max_hw_sectors) {
 		blk_queue_max_hw_sectors(ns->queue, ctrl->max_hw_sectors);
-		blk_queue_max_segments(ns->queue,
-			((ctrl->max_hw_sectors << 9) / ctrl->page_size) + 1);
+
+		if (!ctrl->max_segments)
+			ctrl->max_segments = ((ctrl->max_hw_sectors << 9) /
+						ctrl->page_size) + 1;
 	}
+
+	if (ctrl->max_segments)
+		blk_queue_max_segments(ns->queue, ctrl->max_segments);
+	if (ctrl->max_segment_size)
+		blk_queue_max_segment_size(ns->queue, ctrl->max_segment_size);
 	if (ctrl->stripe_size)
 		blk_queue_chunk_sectors(ns->queue, ctrl->stripe_size >> 9);
 	if (ctrl->vwc & NVME_CTRL_VWC_PRESENT)
drivers/nvme/host/nvme.h +2 -0
index 8cf6fa5..8377a3f 100644
@@ -55,6 +55,8 @@ struct nvme_ctrl {
 	u32 ctrl_config;
 	u32 max_hw_sectors;
 	u32 stripe_size;
+	u32 max_segments;
+	u32 max_segment_size;
 	u32 page_size;
 	u16 oncs;
 	atomic_t abort_limit;
Signature · 1 line
1.9.1
author maintainer
#8 lore raw

PATCH 6/8 nvme: export symbols needed for nvme-loop

In reply to Christoph Hellwig
Signed-off-by Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/host/core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
drivers/nvme/host/core.c +14 -0
index 14ee152..400b1ea 100644
@@ -36,10 +36,12 @@
 unsigned char admin_timeout = 60;
 module_param(admin_timeout, byte, 0644);
 MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
+EXPORT_SYMBOL_GPL(admin_timeout);
 
 unsigned char nvme_io_timeout = 30;
 module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
 MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
+EXPORT_SYMBOL_GPL(nvme_io_timeout);
 
 static unsigned char shutdown_timeout = 5;
 module_param(shutdown_timeout, byte, 0644);

@@ -97,6 +99,7 @@ void nvme_requeue_req(struct request *req)
 		blk_mq_kick_requeue_list(req->q);
 	spin_unlock_irqrestore(req->q->queue_lock, flags);
 }
+EXPORT_SYMBOL_GPL(nvme_requeue_req);
 
 struct request *nvme_alloc_request(struct request_queue *q,
 		struct nvme_command *cmd, bool nowait)

@@ -324,6 +327,7 @@ int nvme_set_features(struct nvme_ctrl *dev, unsigned fid, unsigned dword11,
 
 	return __nvme_submit_sync_cmd(dev->admin_q, &c, NULL, 0, result, 0);
 }
+EXPORT_SYMBOL_GPL(nvme_set_features);
 
 int nvme_get_log_page(struct nvme_ctrl *dev, struct nvme_smart_log **log)
 {

@@ -363,6 +367,7 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int count)
 	}
 	return min(result & 0xffff, result >> 16) + 1;
 }
+EXPORT_SYMBOL_GPL(nvme_set_queue_count);
 
 static int nvme_update_ctrl_config(struct nvme_ctrl *ctrl, u64 cap,
 		bool enabled)

@@ -407,6 +412,7 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 
 	return nvme_update_ctrl_config(ctrl, cap, false);
 }
+EXPORT_SYMBOL_GPL(nvme_disable_ctrl);
 
 int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap, unsigned page_shift)
 {

@@ -418,6 +424,7 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap, unsigned page_shift)
 
 	return nvme_update_ctrl_config(ctrl, cap, true);
 }
+EXPORT_SYMBOL_GPL(nvme_enable_ctrl);
 
 int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
 {

@@ -448,6 +455,7 @@ int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
 
 	return error;
 }
+EXPORT_SYMBOL_GPL(nvme_shutdown_ctrl);
 
 static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 {

@@ -851,6 +859,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 	kfree(id);
 	return 0;
 }
+EXPORT_SYMBOL(nvme_init_identify);
 
 static int nvme_dev_open(struct inode *inode, struct file *file)
 {

@@ -1071,6 +1080,7 @@ void nvme_scan_namespaces(struct nvme_ctrl *ctrl)
 	__nvme_scan_namespaces(ctrl, le32_to_cpup(&id->nn));
 	kfree(id);
 }
+EXPORT_SYMBOL_GPL(nvme_scan_namespaces);
 
 void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
 {

@@ -1079,6 +1089,7 @@ void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
 	list_for_each_entry_safe(ns, next, &ctrl->namespaces, list)
 		nvme_ns_remove(ns);
 }
+EXPORT_SYMBOL_GPL(nvme_remove_namespaces);
 
 static DEFINE_IDA(nvme_instance_ida);
 

@@ -1118,6 +1129,7 @@ void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
 	list_del(&ctrl->node);
 	spin_unlock(&dev_list_lock);
 }
+EXPORT_SYMBOL_GPL(nvme_uninit_ctrl);
 
 static void nvme_free_ctrl(struct kref *kref)
 {

@@ -1133,6 +1145,7 @@ void nvme_put_ctrl(struct nvme_ctrl *ctrl)
 {
 	kref_put(&ctrl->kref, nvme_free_ctrl);
 }
+EXPORT_SYMBOL_GPL(nvme_put_ctrl);
 
 /*
  * Initialize a NVMe controller structures.  This needs to be called during

@@ -1184,6 +1197,7 @@ out_release_instance:
 out:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(nvme_init_ctrl);
 
 int __init nvme_core_init(void)
 {
Signature · 1 line
1.9.1
author maintainer
#9 lore raw

PATCH 7/8 nvmet: add a generic NVMe target

In reply to Christoph Hellwig
Signed-off-by Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/Kconfig            |   1 +
 drivers/nvme/Makefile           |   1 +
 drivers/nvme/target/Kconfig     |   4 +
 drivers/nvme/target/Makefile    |   4 +
 drivers/nvme/target/admin-cmd.c | 353 +++++++++++++++++++++++++++++++
 drivers/nvme/target/configfs.c  | 205 ++++++++++++++++++
 drivers/nvme/target/core.c      | 454 ++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/target/io-cmd.c    | 114 ++++++++++
 drivers/nvme/target/nvmet.h     | 172 +++++++++++++++
 9 files changed, 1308 insertions(+)
 create mode 100644 drivers/nvme/target/Kconfig
 create mode 100644 drivers/nvme/target/Makefile
 create mode 100644 drivers/nvme/target/admin-cmd.c
 create mode 100644 drivers/nvme/target/configfs.c
 create mode 100644 drivers/nvme/target/core.c
 create mode 100644 drivers/nvme/target/io-cmd.c
 create mode 100644 drivers/nvme/target/nvmet.h
drivers/nvme/Kconfig +1 -0
index a39d943..b7c78a5 100644
@@ -1 +1,2 @@
 source "drivers/nvme/host/Kconfig"
+source "drivers/nvme/target/Kconfig"
drivers/nvme/Makefile +1 -0
index 9421e82..0096a7f 100644
@@ -1,2 +1,3 @@
 
 obj-y		+= host/
+obj-y		+= target/
drivers/nvme/target/Kconfig new file +4 -0
new file mode 100644
index 0000000..9a3d742
@@ -0,0 +1,4 @@
+
+config NVME_TARGET
+	depends on BLOCK
+	tristate
drivers/nvme/target/Makefile new file +4 -0
new file mode 100644
index 0000000..9c236e4
@@ -0,0 +1,4 @@
+
+obj-$(CONFIG_NVME_TARGET)		+= nvmet.o
+
+nvmet-y		+= core.o configfs.o admin-cmd.o io-cmd.o
drivers/nvme/target/admin-cmd.c new file +353 -0

353 lines not on this page · raw message

drivers/nvme/target/configfs.c new file +205 -0

205 lines not on this page · raw message

drivers/nvme/target/core.c new file +454 -0

454 lines not on this page · raw message

drivers/nvme/target/io-cmd.c new file +114 -0

114 lines not on this page · raw message

drivers/nvme/target/nvmet.h new file +172 -0

172 lines not on this page · raw message

Signature · 1 line
1.9.1
#10 lore raw

PATCH 7/8 nvmet: add a generic NVMe target

Quoted 11 lines
drivers/nvme/target/core.c new file +10 -0
+void nvmet_ns_free(struct nvmet_ns *ns)
+{
+	struct nvmet_subsys *subsys = ns->subsys;
+
+	mutex_lock(&subsys->lock);
+	if (!list_empty(&ns->dev_link))
+		list_del_init(&ns->dev_link);
+	mutex_unlock(&subsys->lock);
+
+	nvmet_put_namespace(ns);

drivers/nvme/target/core.c line 122

Quoted 10 lines
drivers/nvme/target/core.c new file +10 -0
+}
+
+struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
+{
+	struct nvmet_ns *ns;
+
+	ns = kzalloc(sizeof(*ns), GFP_KERNEL);
+	if (!ns)
+		return NULL;
+

This lacks also:
INIT_LIST_HEAD(&ns->dev_link);

So the list_empty() check in ns_free won't be bogus... It causes a NULL deref on configfs rmdir.

Quoted 5 lines
drivers/nvme/target/core.c new file +5 -0
+	kref_init(&ns->ref);
+	ns->nsid = nsid;
+	ns->subsys = subsys;
+	return ns;
+}
author maintainer
#11 lore raw

PATCH 7/8 nvmet: add a generic NVMe target

Agreed, I'll fix it up.

author maintainer
#12 lore raw

PATCH 8/8 nvme-loop: add a NVMe loopback device

In reply to Christoph Hellwig

Usage:

------ snip ------
NAME="testsubsystem"
DEV="/dev/?da"

mkdir /sys/kernel/config/nvmet/subsystems/${NAME}
mkdir /sys/kernel/config/nvmet/subsystems/${NAME}/namespaces/1
echo -n ${DEV} > /sys/kernel/config/nvmet/subsystems/${NAME}/namespaces/1/device_path

echo "name=${NAME}" > /sys/class/nvme-loop/ctl/add_ctrl ------ snip ------

Signed-off-by Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/target/Kconfig  |  10 +
 drivers/nvme/target/Makefile |   2 +
 drivers/nvme/target/loop.c   | 688 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 700 insertions(+)
 create mode 100644 drivers/nvme/target/loop.c
drivers/nvme/target/Kconfig +10 -0
index 9a3d742..1bf92db 100644
@@ -2,3 +2,13 @@
 config NVME_TARGET
 	depends on BLOCK
 	tristate
+
+config NVME_TARGET_LOOP
+	tristate "NVMe loopback device support"
+	depends on BLK_DEV_NVME
+	select NVME_TARGET
+	help
+	  This enabled the NVMe loopback device support, which can be useful
+	  to test NVMe host and target side features.
+
+	  If unsure, say N.
drivers/nvme/target/Makefile +2 -0
index 9c236e4..21e9134 100644
@@ -1,4 +1,6 @@
 
 obj-$(CONFIG_NVME_TARGET)		+= nvmet.o
+obj-$(CONFIG_NVME_TARGET_LOOP)		+= nvme-loop.o
 
 nvmet-y		+= core.o configfs.o admin-cmd.o io-cmd.o
+nvme-loop-y	+= loop.o
drivers/nvme/target/loop.c new file +688 -0

688 lines not on this page · raw message

Signature · 1 line
1.9.1
#13 lore raw

PATCH 8/8 nvme-loop: add a NVMe loopback device

drivers/nvme/target/loop.c line 243

Quoted 6 lines
drivers/nvme/target/loop.c new file +5 -0
+static void nvme_loop_free_ctrl(struct nvme_ctrl *nctrl)
+{
+	struct nvme_loop_ctrl *ctrl = to_loop_ctrl(nctrl);
+
+	list_del(&ctrl->list);

This should be list_del_init so that cleanup_module list_empty() check will be correct. unloading nvme-loop with active controllers is getting a list corruption.

#14 lore raw

PATCH 8/8 nvme-loop: add a NVMe loopback device

Quoted 10 lines · Sagi Grimberg

Review

Quoted 6 lines
(quoted diff) +5 -0
+static void nvme_loop_free_ctrl(struct nvme_ctrl *nctrl)
+{
+    struct nvme_loop_ctrl *ctrl = to_loop_ctrl(nctrl);
+
+    list_del(&ctrl->list);

This should be list_del_init so that cleanup_module list_empty() check will be correct. unloading nvme-loop with active controllers is getting a list corruption.

Actually this is wrong. Whats wrong here is that ctrl->list is being removed twice (once in nvme_loop_cleanup_module and then here).

The following worked for me:
--

drivers/nvme/target/loop.c +2 -9
index a0eac07..cf0f745 100644
@@ -869,18 +869,11 @@ out:
 
  static void __exit nvme_loop_cleanup_module(void)
  {
-       struct nvme_loop_ctrl *ctrl;
+       struct nvme_loop_ctrl *ctrl, *tmp;
 
         mutex_lock(&nvme_loop_ctrl_mutex);
-       while (!list_empty(&nvme_loop_ctrl_list)) {
-               ctrl = list_entry(nvme_loop_ctrl_list.next,
-                               struct nvme_loop_ctrl, list);
-
-               if (!list_empty(&ctrl->list))
-                       list_del(&ctrl->list);
-
+       list_for_each_entry_safe(ctrl, tmp, &nvme_loop_ctrl_list, list)
                 __nvme_loop_remove_ctrl(ctrl);
-       }
         mutex_unlock(&nvme_loop_ctrl_mutex);
 
         device_destroy(nvme_loop_class, MKDEV(0, 0));

--

author maintainer
#15 lore raw

PATCH 8/8 nvme-loop: add a NVMe loopback device

Quoted 3 lines · Sagi Grimberg

Actually this is wrong. Whats wrong here is that ctrl->list is being removed twice (once in nvme_loop_cleanup_module and then here).

Yes, this looks fine.

#16 lore raw

PATCH 8/8 nvme-loop: add a NVMe loopback device

In reply to Christoph Hellwig

drivers/nvme/target/loop.c line 537

Quoted 6 lines
drivers/nvme/target/loop.c new file +5 -0
+	ctrl->queue_count = 1; /* admin queue */;
+	if (nr_io_queues > 0)
+		ctrl->queue_count += nr_io_queues;
+	else
+		ctrl->queue_count += num_possible_cpus();

This should really be num_online_cpus(), I get way more than I need with num_possible_cpus()...

author maintainer
#17 lore raw

PATCH 8/8 nvme-loop: add a NVMe loopback device

Quoted 9 lines · Sagi Grimberg

drivers/nvme/target/loop.c line 537

Quoted 6 lines
drivers/nvme/target/loop.c new file +5 -0
+	ctrl->queue_count = 1; /* admin queue */;
+	if (nr_io_queues > 0)
+		ctrl->queue_count += nr_io_queues;
+	else
+		ctrl->queue_count += num_possible_cpus();

This should really be num_online_cpus(), I get way more than I need with num_possible_cpus()...

Maybe. But then we'd want a hot plug event. The whole number of contexts thing is a bit of a mess.

#18 lore raw

PATCH 8/8 nvme-loop: add a NVMe loopback device

Quoted 11 lines · Christoph Hellwig
Quoted 9 lines · Sagi Grimberg

drivers/nvme/target/loop.c line 537

Quoted 6 lines
drivers/nvme/target/loop.c new file +5 -0
+	ctrl->queue_count = 1; /* admin queue */;
+	if (nr_io_queues > 0)
+		ctrl->queue_count += nr_io_queues;
+	else
+		ctrl->queue_count += num_possible_cpus();

This should really be num_online_cpus(), I get way more than I need with num_possible_cpus()...

Quoted 2 lines

Maybe. But then we'd want a hot plug event. The whole number of contexts thing is a bit of a mess.

Or we can just remain with more or less queues then cpus... it is already possible if the user used nr_io_queues != num_online_cpus()...

#19 lore raw
In reply to Christoph Hellwig
Quoted 4 lines · Christoph Hellwig

This series continues the NVMe host drive split and also starts adding a consume for it. The consumer is mostly interesting for developers at this point as it's simply a 'loopback' NVMe device that ties the split NVMe driver fronted into the new generic NVMe target subsystem.

Very excited to see this code posted. 8-)

Quoted 4 lines

This was developed for our NVMe over Fabrics prototype, but will also be useful for other work like Ming's virtio-nvme or event an implementation traditional PCIe NVMe using vhost.

Wrt to vhost-nvme, the WIP code (Dr. Hannes + Dave CC'ed) I'm currently hacking on is here:

https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/log/?h=vhost-nvme-wip

Note it's still a week or two away (using rts-megasas as a reference) from actually functioning across a modest number of queue resources, but should at least give interested folks an idea of how things look so far.

Quoted 7 lines

The biggest gap at this point is aborts and resets, which are still under heavy development. Once those are done and I've finished the configfs layout the code will hopefully be ready for inclusion in Linux 4.5.

I've also written a python based shell to ease configuration of the target subsystem which I'll hopefully be able to release soon.

;)

Quoted 5 lines

As this depends on a lot of pending NVMe patch a git tree is also available:

http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/nvme-loop.2

So on vhost-nvme configfs side, my initial code will be using target_core_fabric_configfs.c to process incoming nvme-hi frames into existing backend struct se_device.

That said, I'm planning to rebase atop your WIP tree in the short-term for the common nvme opcode + command set decoding bits, that both vhost-nvme and nvme-fabrics code are going to require.

--nab

#20 lore raw
Quoted 13 lines · Nicholas A. Bellinger
Quoted 4 lines · Christoph Hellwig

This series continues the NVMe host drive split and also starts adding a consume for it. The consumer is mostly interesting for developers at this point as it's simply a 'loopback' NVMe device that ties the split NVMe driver fronted into the new generic NVMe target subsystem.

Very excited to see this code posted. 8-)

Quoted 4 lines

This was developed for our NVMe over Fabrics prototype, but will also be useful for other work like Ming's virtio-nvme or event an implementation traditional PCIe NVMe using vhost.

Quoted 8 lines

Wrt to vhost-nvme, the WIP code (Dr. Hannes + Dave CC'ed) I'm currently hacking on is here:

https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/log/?h=vhost-nvme-wip

Note it's still a week or two away (using rts-megasas as a reference) from actually functioning across a modest number of queue resources, but should at least give interested folks an idea of how things look so far.

Hi Nic,

FYI,

I have done the vhost-nvme patches(based on our previous discussion) on top of NVMe target.

I'll post kernel & qemu patches early this week.

But the tests I have done so far didn't show competitive performance compared with vhost-scsi. Maybe because the mmio thing is slow.

I'm going to do anther implementation with virtio-nvme/vhost-nvme. With nvme-split, now virtio-nvme seems make sense.

I have ported virtio-nvme last month.
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-split/virtio

Thanks,
Ming

Quoted 26 lines
Quoted 8 lines

The biggest gap at this point is aborts and resets, which are still under heavy development. Once those are done and I've finished the configfs layout the code will hopefully be ready for inclusion in Linux 4.5.

I've also written a python based shell to ease configuration of the target subsystem which I'll hopefully be able to release soon.

;)

Quoted 5 lines

As this depends on a lot of pending NVMe patch a git tree is also available:

http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/nvme-loop.2

So on vhost-nvme configfs side, my initial code will be using target_core_fabric_configfs.c to process incoming nvme-hi frames into existing backend struct se_device.

That said, I'm planning to rebase atop your WIP tree in the short-term for the common nvme opcode + command set decoding bits, that both vhost-nvme and nvme-fabrics code are going to require.

--nab

#21 lore raw
Quoted 23 lines · Ming Lin
Quoted 13 lines · Nicholas A. Bellinger
Quoted 4 lines · Christoph Hellwig

This series continues the NVMe host drive split and also starts adding a consume for it. The consumer is mostly interesting for developers at this point as it's simply a 'loopback' NVMe device that ties the split NVMe driver fronted into the new generic NVMe target subsystem.

Very excited to see this code posted. 8-)

Quoted 4 lines

This was developed for our NVMe over Fabrics prototype, but will also be useful for other work like Ming's virtio-nvme or event an implementation traditional PCIe NVMe using vhost.

Quoted 8 lines

Wrt to vhost-nvme, the WIP code (Dr. Hannes + Dave CC'ed) I'm currently hacking on is here:

https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/log/?h=vhost-nvme-wip

Note it's still a week or two away (using rts-megasas as a reference) from actually functioning across a modest number of queue resources, but should at least give interested folks an idea of how things look so far.

Quoted 9 lines

Hi Nic,

FYI,

I have done the vhost-nvme patches(based on our previous discussion) on top of NVMe target.

I'll post kernel & qemu patches early this week.

Great. Looking forward to seeing the prototype code.

Quoted 3 lines

But the tests I have done so far didn't show competitive performance compared with vhost-scsi. Maybe because the mmio thing is slow.

Anything interesting hot-spots that show up in perf output..?

Quoted 5 lines

I'm going to do anther implementation with virtio-nvme/vhost-nvme. With nvme-split, now virtio-nvme seems make sense.

I have ported virtio-nvme last month.
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-split/virtio

Cool. :)

#22 lore raw
Quoted 34 lines · Nicholas A. Bellinger
Quoted 23 lines · Ming Lin
Quoted 13 lines · Nicholas A. Bellinger
Quoted 4 lines · Christoph Hellwig

This series continues the NVMe host drive split and also starts adding a consume for it. The consumer is mostly interesting for developers at this point as it's simply a 'loopback' NVMe device that ties the split NVMe driver fronted into the new generic NVMe target subsystem.

Very excited to see this code posted. 8-)

Quoted 4 lines

This was developed for our NVMe over Fabrics prototype, but will also be useful for other work like Ming's virtio-nvme or event an implementation traditional PCIe NVMe using vhost.

Quoted 8 lines

Wrt to vhost-nvme, the WIP code (Dr. Hannes + Dave CC'ed) I'm currently hacking on is here:

https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/log/?h=vhost-nvme-wip

Note it's still a week or two away (using rts-megasas as a reference) from actually functioning across a modest number of queue resources, but should at least give interested folks an idea of how things look so far.

Quoted 9 lines

Hi Nic,

FYI,

I have done the vhost-nvme patches(based on our previous discussion) on top of NVMe target.

I'll post kernel & qemu patches early this week.

Quoted 7 lines

Great. Looking forward to seeing the prototype code.

Quoted 3 lines

But the tests I have done so far didn't show competitive performance compared with vhost-scsi. Maybe because the mmio thing is slow.

Anything interesting hot-spots that show up in perf output..?

To ease development, I use nested kvm.
"vm_host" runs on bare metal and "vm_guest" runs on "vm_host"

I just integrated Google's extension to vhost-nvme.
https://github.com/rlnelson-git/linux-nvme.git

It's amazing performance improves a lot. I use a 256M /dev/ram0 on vm_host as backend.

fio 4k read:
qemu-nvme: ~20M to ~30M
qemu-vhost-nvme + google ext: 80M to 200M(not very stable though)

(BTW, still waiting for employer's approval to send out patches)

PerfTop: 1039 irqs/sec kernel:99.8% exact: 0.0% [4000Hz cpu-clock], (all, 4 CPUs)
---------------------------------------------------------------------------------------------

    36.93%  [kernel]       [k] _raw_spin_unlock_irq          
    20.98%  [kernel]       [k] vmx_handle_external_intr      
    10.10%  [kernel]       [k] _raw_spin_unlock_irqrestore   
     4.95%  [kernel]       [k] __mutex_unlock_slowpath       
     4.41%  [kernel]       [k] lock_acquire                  
     4.15%  [kernel]       [k] lock_is_held                  
     2.30%  [kernel]       [k] mutex_lock_nested             
     1.68%  [kernel]       [k] lock_release                  
     1.14%  [kernel]       [k] put_compound_page             
     0.93%  [kernel]       [k] debug_lockdep_rcu_enabled     
     0.66%  [kernel]       [k] check_preemption_disabled     
     0.64%  [kernel]       [k] __schedule                    
     0.62%  [kernel]       [k] lock_acquired                 
     0.54%  [kernel]       [k] rcu_lockdep_current_cpu_online
     0.54%  [kernel]       [k] preempt_count_sub             
     0.54%  [kernel]       [k] preempt_count_add             
     0.46%  [kernel]       [k] find_vma                      
     0.45%  [kernel]       [k] vmcs_writel                   
     0.40%  [kernel]       [k] ___might_sleep                
     0.38%  [kernel]       [k] rcu_note_context_switch       
     0.37%  [kernel]       [k] rcu_read_lock_sched_held      
     0.32%  [kernel]       [k] __rcu_is_watching             
     0.32%  [kernel]       [k] follow_trans_huge_pmd         
     0.31%  [kernel]       [k] debug_smp_processor_id        
     0.22%  [kernel]       [k] follow_page_mask              
     0.18%  [kernel]       [k] __get_user_pages              
     0.16%  [kernel]       [k] vmx_read_guest_seg_ar         
     0.16%  [kernel]       [k] nvmet_vhost_rw                
     0.15%  [kernel]       [k] kthread_should_stop           
     0.14%  [kernel]       [k] schedule                      
     0.14%  [kernel]       [k] rcu_is_watching               
     0.12%  [kernel]       [k] nvmet_vhost_sq_thread         
     0.11%  [kernel]       [k] get_parent_ip                 
     0.11%  [kernel]       [k] _raw_spin_lock_irqsave
author maintainer
#23 lore raw
In reply to Ming Lin
Quoted 2 lines · Ming Lin

But the tests I have done so far didn't show competitive performance compared with vhost-scsi. Maybe because the mmio thing is slow.

It is. Take a look at the google patches for the NVMe even index extension, which make it behave a lot more like virtio.