Skip to contentSkip to conversation

nvme-loop: add support for multiple ports

PATCH New Christoph Hellwig 3 participants 5 messages May 26, 2018 – May 28, 2018

Subsystems (2)
lore
Share & tools
lore

Christoph Hellwig

author maintainer
#0 lore raw

PATCH 1/1 nvme-loop: add support for multiple ports

This is useful at least for multipath testing.

Signed-off-by Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/target/loop.c  | 47 +++++++++++++++++++++++--------------
 drivers/nvme/target/nvmet.h |  2 +-
 2 files changed, 30 insertions(+), 19 deletions(-)
drivers/nvme/target/loop.c +29 -18
index 4284cefe6d32..d8a170749a45 100644
@@ -45,6 +45,7 @@ struct nvme_loop_ctrl {
 	struct nvme_ctrl	ctrl;
 
 	struct nvmet_ctrl	*target_ctrl;
+	struct nvmet_port	*port;
 };
 
 static inline struct nvme_loop_ctrl *to_loop_ctrl(struct nvme_ctrl *ctrl)
@@ -63,7 +64,8 @@ struct nvme_loop_queue {
 	unsigned long		flags;
 };
 
-static struct nvmet_port *nvmet_loop_port;
+static LIST_HEAD(nvme_loop_ports);
+static DEFINE_MUTEX(nvme_loop_ports_mutex);
 
 static LIST_HEAD(nvme_loop_ctrl_list);
 static DEFINE_MUTEX(nvme_loop_ctrl_mutex);
@@ -169,7 +171,7 @@ static blk_status_t nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx,
 
 	blk_mq_start_request(req);
 	iod->cmd.common.flags |= NVME_CMD_SGL_METABUF;
-	iod->req.port = nvmet_loop_port;
+	iod->req.port = queue->ctrl->port;
 	if (!nvmet_req_init(&iod->req, &queue->nvme_cq,
 			&queue->nvme_sq, &nvme_loop_ops))
 		return BLK_STS_OK;
@@ -517,6 +519,7 @@ static const struct nvme_ctrl_ops nvme_loop_ctrl_ops = {
 	.free_ctrl		= nvme_loop_free_ctrl,
 	.submit_async_event	= nvme_loop_submit_async_event,
 	.delete_ctrl		= nvme_loop_delete_ctrl_host,
+	.get_address		= nvmf_get_address,
 };
 
 static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
@@ -565,6 +568,22 @@ static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
 	return ret;
 }
 
+static struct nvmet_port *nvme_loop_find_port(struct nvme_ctrl *ctrl)
+{
+	struct nvmet_port *p, *found = NULL;
+	mutex_lock(&nvme_loop_ports_mutex);
+	list_for_each_entry(p, &nvme_loop_ports, entry) {
+		/* if no transport address is specified use the first port */
+		if ((ctrl->opts->mask & NVMF_OPT_TRADDR) &&
+		    strcmp(ctrl->opts->traddr, p->disc_addr.traddr))
+			continue;
+		found = p;
+		break;
+	}
+	mutex_unlock(&nvme_loop_ports_mutex);
+	return found;
+}
+
 static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
 		struct nvmf_ctrl_options *opts)
 {
@@ -589,6 +608,7 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
 
 	ctrl->ctrl.sqsize = opts->queue_size - 1;
 	ctrl->ctrl.kato = opts->kato;
+	ctrl->port = nvme_loop_find_port(&ctrl->ctrl);
 
 	ctrl->queues = kcalloc(opts->nr_io_queues + 1, sizeof(*ctrl->queues),
 			GFP_KERNEL);
@@ -646,27 +666,17 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
 
 static int nvme_loop_add_port(struct nvmet_port *port)
 {
-	/*
-	 * XXX: disalow adding more than one port so
-	 * there is no connection rejections when a
-	 * a subsystem is assigned to a port for which
-	 * loop doesn't have a pointer.
-	 * This scenario would be possible if we allowed
-	 * more than one port to be added and a subsystem
-	 * was assigned to a port other than nvmet_loop_port.
-	 */
-
-	if (nvmet_loop_port)
-		return -EPERM;
-
-	nvmet_loop_port = port;
+	mutex_lock(&nvme_loop_ports_mutex);
+	list_add_tail(&port->entry, &nvme_loop_ports);
+	mutex_unlock(&nvme_loop_ports_mutex);
 	return 0;
 }
 
 static void nvme_loop_remove_port(struct nvmet_port *port)
 {
-	if (port == nvmet_loop_port)
-		nvmet_loop_port = NULL;
+	mutex_lock(&nvme_loop_ports_mutex);
+	list_del_init(&port->entry);
+	mutex_unlock(&nvme_loop_ports_mutex);
 }
 
 static const struct nvmet_fabrics_ops nvme_loop_ops = {
@@ -682,6 +692,7 @@ static struct nvmf_transport_ops nvme_loop_transport = {
 	.name		= "loop",
 	.module		= THIS_MODULE,
 	.create_ctrl	= nvme_loop_create_ctrl,
+	.allowed_opts	= NVMF_OPT_TRADDR,
 };
 
 static int __init nvme_loop_init_module(void)
drivers/nvme/target/nvmet.h +1 -1
index e63ab3c0c655..87c05a56c2a0 100644
@@ -87,7 +87,7 @@ struct nvmet_sq {
 /**
  * struct nvmet_port -	Common structure to keep port
  *				information for the target.
- * @entry:		List head for holding a list of these elements.
+ * @entry:		Entry into referrals or transport list.
  * @disc_addr:		Address information is stored in a format defined
  *				for a discovery log page entry.
  * @group:		ConfigFS group for this element's folder.
Signature · 1 line
2.17.0
#1 lore raw

PATCH 1/1 nvme-loop: add support for multiple ports

Quoted 1 line · Christoph Hellwig

This is useful at least for multipath testing.

Awesome, thanks

I'll give it a shot on monday.

Signature · 6 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
#2 lore raw

PATCH 1/1 nvme-loop: add support for multiple ports

In reply to Christoph Hellwig
Quoted 48 lines · Christoph Hellwig

This is useful at least for multipath testing.

Signed-off-by Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/target/loop.c  | 47 +++++++++++++++++++++++--------------
 drivers/nvme/target/nvmet.h |  2 +-
 2 files changed, 30 insertions(+), 19 deletions(-)
drivers/nvme/target/loop.c +5 -2
index 4284cefe6d32..d8a170749a45 100644
@@ -45,6 +45,7 @@ struct nvme_loop_ctrl {
        struct nvme_ctrl        ctrl;
 
        struct nvmet_ctrl       *target_ctrl;
+       struct nvmet_port       *port;
 };
 
 static inline struct nvme_loop_ctrl *to_loop_ctrl(struct nvme_ctrl *ctrl)
@@ -63,7 +64,8 @@ struct nvme_loop_queue {
        unsigned long           flags;
 };
 
-static struct nvmet_port *nvmet_loop_port;
+static LIST_HEAD(nvme_loop_ports);
+static DEFINE_MUTEX(nvme_loop_ports_mutex);
 
 static LIST_HEAD(nvme_loop_ctrl_list);
 static DEFINE_MUTEX(nvme_loop_ctrl_mutex);
@@ -169,7 +171,7 @@ static blk_status_t nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx,
 
        blk_mq_start_request(req);
        iod->cmd.common.flags |= NVME_CMD_SGL_METABUF;
-       iod->req.port = nvmet_loop_port;
+       iod->req.port = queue->ctrl->port;
        if (!nvmet_req_init(&iod->req, &queue->nvme_cq,
                        &queue->nvme_sq, &nvme_loop_ops))
                return BLK_STS_OK;
@@ -517,6 +519,7 @@ static const struct nvme_ctrl_ops nvme_loop_ctrl_ops = {
        .free_ctrl              = nvme_loop_free_ctrl,
        .submit_async_event     = nvme_loop_submit_async_event,
        .delete_ctrl            = nvme_loop_delete_ctrl_host,
+       .get_address            = nvmf_get_address,
 };
 
 static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)

drivers/nvme/target/loop.c static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl) · line 573

Quoted 6 lines
drivers/nvme/target/loop.c +3 -0
@@ -565,6 +568,22 @@ static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
        return ret;
 }
 
+static struct nvmet_port *nvme_loop_find_port(struct nvme_ctrl *ctrl)
+{
+       struct nvmet_port *p, *found = NULL;

[CK] Can we add a new line here to get rid of the checkpatch.pl warning?

------------------------------------------------------------------------
#57: FILE: drivers/nvme/target/loop.c:574:
+ struct nvmet_port *p, *found = NULL;
+ mutex_lock(&nvme_loop_ports_mutex);
------------------------------------------------------------------------

Quoted 87 lines
drivers/nvme/target/loop.c +21 -16
+       mutex_lock(&nvme_loop_ports_mutex);
+       list_for_each_entry(p, &nvme_loop_ports, entry) {
+               /* if no transport address is specified use the first port */
+               if ((ctrl->opts->mask & NVMF_OPT_TRADDR) &&
+                   strcmp(ctrl->opts->traddr, p->disc_addr.traddr))
+                       continue;
+               found = p;
+               break;
+       }
+       mutex_unlock(&nvme_loop_ports_mutex);
+       return found;
+}
+
 static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
                struct nvmf_ctrl_options *opts)
 {
@@ -589,6 +608,7 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
 
        ctrl->ctrl.sqsize = opts->queue_size - 1;
        ctrl->ctrl.kato = opts->kato;
+       ctrl->port = nvme_loop_find_port(&ctrl->ctrl);
 
        ctrl->queues = kcalloc(opts->nr_io_queues + 1, sizeof(*ctrl->queues),
                        GFP_KERNEL);
@@ -646,27 +666,17 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
 
 static int nvme_loop_add_port(struct nvmet_port *port)
 {
-       /*
-        * XXX: disalow adding more than one port so
-        * there is no connection rejections when a
-        * a subsystem is assigned to a port for which
-        * loop doesn't have a pointer.
-        * This scenario would be possible if we allowed
-        * more than one port to be added and a subsystem
-        * was assigned to a port other than nvmet_loop_port.
-        */
-
-       if (nvmet_loop_port)
-               return -EPERM;
-
-       nvmet_loop_port = port;
+       mutex_lock(&nvme_loop_ports_mutex);
+       list_add_tail(&port->entry, &nvme_loop_ports);
+       mutex_unlock(&nvme_loop_ports_mutex);
        return 0;
 }
 
 static void nvme_loop_remove_port(struct nvmet_port *port)
 {
-       if (port == nvmet_loop_port)
-               nvmet_loop_port = NULL;
+       mutex_lock(&nvme_loop_ports_mutex);
+       list_del_init(&port->entry);
+       mutex_unlock(&nvme_loop_ports_mutex);
 }
 
 static const struct nvmet_fabrics_ops nvme_loop_ops = {
@@ -682,6 +692,7 @@ static struct nvmf_transport_ops nvme_loop_transport = {
        .name           = "loop",
        .module         = THIS_MODULE,
        .create_ctrl    = nvme_loop_create_ctrl,
+       .allowed_opts   = NVMF_OPT_TRADDR,
 };
 
 static int __init nvme_loop_init_module(void)
drivers/nvme/target/nvmet.h +1 -2
index e63ab3c0c655..87c05a56c2a0 100644
@@ -87,7 +87,7 @@ struct nvmet_sq {
 /**
  * struct nvmet_port - Common structure to keep port
  *                             information for the target.
- * @entry:             List head for holding a list of these elements.
+ * @entry:             Entry into referrals or transport list.
  * @disc_addr:         Address information is stored in a format defined
  *                             for a discovery log page entry.
  * @group:             ConfigFS group for this element's folder.
--

2.17.0

_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

author maintainer
#3 lore raw

PATCH 1/1 nvme-loop: add support for multiple ports

Quoted 6 lines

drivers/nvme/target/loop.c static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl) · line 573

Quoted 4 lines
drivers/nvme/target/loop.c +3 -0
+static struct nvmet_port *nvme_loop_find_port(struct nvme_ctrl *ctrl)
+{
+       struct nvmet_port *p, *found = NULL;

[CK] Can we add a new line here to get rid of the checkpatch.pl warning?

Sure..

#4 lore raw

PATCH 1/1 nvme-loop: add support for multiple ports

In reply to Christoph Hellwig

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>

Signature · 6 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850