Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] nvme_fc : add module to ops template to allow module references
@ 2019-11-14 23:15 James Smart
  2019-11-22 17:09 ` Himanshu Madhani
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: James Smart @ 2019-11-14 23:15 UTC (permalink / raw)
  To: linux-nvme; +Cc: James Smart

In nvme-fc: it's possible to have connected active controllers
and as no references are taken on the LLDD, the LLDD can be
unloaded.  The controller would enter a reconnect state and as
long as the LLDD resumed within the reconnect timeout, the
controller would resume.  But if a namespace on the controller
is the root device, allowing the driver to unload can be problematic.
To reload the driver, it may require new io to the boot device,
and as it's no longer connected we get into a catch-22 that
eventually fails, and the system locks up.

Fix this issue by taking a module reference for every connected
controller (which is what the core layer did to the transport
module). Reference is cleared when the controller is removed.

Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/nvme/host/fc.c          | 14 ++++++++++++--
 drivers/nvme/target/fcloop.c    |  1 +
 drivers/scsi/lpfc/lpfc_nvme.c   |  2 ++
 drivers/scsi/qla2xxx/qla_nvme.c |  1 +
 include/linux/nvme-fc-driver.h  |  4 ++++
 5 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 679a721ae229..40f9241c592f 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -342,7 +342,8 @@ nvme_fc_register_localport(struct nvme_fc_port_info *pinfo,
 	    !template->ls_req || !template->fcp_io ||
 	    !template->ls_abort || !template->fcp_abort ||
 	    !template->max_hw_queues || !template->max_sgl_segments ||
-	    !template->max_dif_sgl_segments || !template->dma_boundary) {
+	    !template->max_dif_sgl_segments || !template->dma_boundary ||
+	    !template->module) {
 		ret = -EINVAL;
 		goto out_reghost_failed;
 	}
@@ -2015,6 +2016,7 @@ nvme_fc_ctrl_free(struct kref *ref)
 {
 	struct nvme_fc_ctrl *ctrl =
 		container_of(ref, struct nvme_fc_ctrl, ref);
+	struct nvme_fc_lport *lport = ctrl->lport;
 	unsigned long flags;
 
 	if (ctrl->ctrl.tagset) {
@@ -2041,6 +2043,7 @@ nvme_fc_ctrl_free(struct kref *ref)
 	if (ctrl->ctrl.opts)
 		nvmf_free_options(ctrl->ctrl.opts);
 	kfree(ctrl);
+	module_put(lport->ops->module);
 }
 
 static void
@@ -3059,10 +3062,15 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
 		goto out_fail;
 	}
 
+	if (!try_module_get(lport->ops->module)) {
+		ret = -EUNATCH;
+		goto out_free_ctrl;
+	}
+
 	idx = ida_simple_get(&nvme_fc_ctrl_cnt, 0, 0, GFP_KERNEL);
 	if (idx < 0) {
 		ret = -ENOSPC;
-		goto out_free_ctrl;
+		goto out_mod_put;
 	}
 
 	ctrl->ctrl.opts = opts;
@@ -3215,6 +3223,8 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
 out_free_ida:
 	put_device(ctrl->dev);
 	ida_simple_remove(&nvme_fc_ctrl_cnt, ctrl->cnum);
+out_mod_put:
+	module_put(lport->ops->module);
 out_free_ctrl:
 	kfree(ctrl);
 out_fail:
diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
index b50b53db3746..1c50af6219f3 100644
--- a/drivers/nvme/target/fcloop.c
+++ b/drivers/nvme/target/fcloop.c
@@ -850,6 +850,7 @@ fcloop_targetport_delete(struct nvmet_fc_target_port *targetport)
 #define FCLOOP_DMABOUND_4G		0xFFFFFFFF
 
 static struct nvme_fc_port_template fctemplate = {
+	.module			= THIS_MODULE,
 	.localport_delete	= fcloop_localport_delete,
 	.remoteport_delete	= fcloop_remoteport_delete,
 	.create_queue		= fcloop_create_queue,
diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index a227e36cbdc2..8e0f03ef346b 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -1976,6 +1976,8 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
 
 /* Declare and initialization an instance of the FC NVME template. */
 static struct nvme_fc_port_template lpfc_nvme_template = {
+	.module	= THIS_MODULE,
+
 	/* initiator-based functions */
 	.localport_delete  = lpfc_nvme_localport_delete,
 	.remoteport_delete = lpfc_nvme_remoteport_delete,
diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index 6cc19e060afc..6e4d71302534 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -610,6 +610,7 @@ static void qla_nvme_remoteport_delete(struct nvme_fc_remote_port *rport)
 }
 
 static struct nvme_fc_port_template qla_nvme_fc_transport = {
+	.module	= THIS_MODULE,
 	.localport_delete = qla_nvme_localport_delete,
 	.remoteport_delete = qla_nvme_remoteport_delete,
 	.create_queue   = qla_nvme_alloc_queue,
diff --git a/include/linux/nvme-fc-driver.h b/include/linux/nvme-fc-driver.h
index 10f81629b9ce..6d0d70f3219c 100644
--- a/include/linux/nvme-fc-driver.h
+++ b/include/linux/nvme-fc-driver.h
@@ -270,6 +270,8 @@ struct nvme_fc_remote_port {
  *
  * Host/Initiator Transport Entrypoints/Parameters:
  *
+ * @module:  The LLDD module using the interface
+ *
  * @localport_delete:  The LLDD initiates deletion of a localport via
  *       nvme_fc_deregister_localport(). However, the teardown is
  *       asynchronous. This routine is called upon the completion of the
@@ -383,6 +385,8 @@ struct nvme_fc_remote_port {
  *       Value is Mandatory. Allowed to be zero.
  */
 struct nvme_fc_port_template {
+	struct module	*module;
+
 	/* initiator-based functions */
 	void	(*localport_delete)(struct nvme_fc_local_port *);
 	void	(*remoteport_delete)(struct nvme_fc_remote_port *);
-- 
2.13.7


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] nvme_fc : add module to ops template to allow module references
  2019-11-14 23:15 [PATCH] nvme_fc : add module to ops template to allow module references James Smart
@ 2019-11-22 17:09 ` Himanshu Madhani
  2019-11-26 16:51 ` Christoph Hellwig
  2019-11-26 17:39 ` Keith Busch
  2 siblings, 0 replies; 4+ messages in thread
From: Himanshu Madhani @ 2019-11-22 17:09 UTC (permalink / raw)
  To: James Smart, linux-nvme



On 11/14/19, 5:15 PM, "Linux-nvme on behalf of James Smart" <linux-nvme-bounces@lists.infradead.org on behalf of jsmart2021@gmail.com> wrote:

    In nvme-fc: it's possible to have connected active controllers
    and as no references are taken on the LLDD, the LLDD can be
    unloaded.  The controller would enter a reconnect state and as
    long as the LLDD resumed within the reconnect timeout, the
    controller would resume.  But if a namespace on the controller
    is the root device, allowing the driver to unload can be problematic.
    To reload the driver, it may require new io to the boot device,
    and as it's no longer connected we get into a catch-22 that
    eventually fails, and the system locks up.
    
    Fix this issue by taking a module reference for every connected
    controller (which is what the core layer did to the transport
    module). Reference is cleared when the controller is removed.
    
    Signed-off-by: James Smart <jsmart2021@gmail.com>
    ---
     drivers/nvme/host/fc.c          | 14 ++++++++++++--
     drivers/nvme/target/fcloop.c    |  1 +
     drivers/scsi/lpfc/lpfc_nvme.c   |  2 ++
     drivers/scsi/qla2xxx/qla_nvme.c |  1 +
     include/linux/nvme-fc-driver.h  |  4 ++++
     5 files changed, 20 insertions(+), 2 deletions(-)
    
    diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
    index 679a721ae229..40f9241c592f 100644
    --- a/drivers/nvme/host/fc.c
    +++ b/drivers/nvme/host/fc.c
    @@ -342,7 +342,8 @@ nvme_fc_register_localport(struct nvme_fc_port_info *pinfo,
     	    !template->ls_req || !template->fcp_io ||
     	    !template->ls_abort || !template->fcp_abort ||
     	    !template->max_hw_queues || !template->max_sgl_segments ||
    -	    !template->max_dif_sgl_segments || !template->dma_boundary) {
    +	    !template->max_dif_sgl_segments || !template->dma_boundary ||
    +	    !template->module) {
     		ret = -EINVAL;
     		goto out_reghost_failed;
     	}
    @@ -2015,6 +2016,7 @@ nvme_fc_ctrl_free(struct kref *ref)
     {
     	struct nvme_fc_ctrl *ctrl =
     		container_of(ref, struct nvme_fc_ctrl, ref);
    +	struct nvme_fc_lport *lport = ctrl->lport;
     	unsigned long flags;
     
     	if (ctrl->ctrl.tagset) {
    @@ -2041,6 +2043,7 @@ nvme_fc_ctrl_free(struct kref *ref)
     	if (ctrl->ctrl.opts)
     		nvmf_free_options(ctrl->ctrl.opts);
     	kfree(ctrl);
    +	module_put(lport->ops->module);
     }
     
     static void
    @@ -3059,10 +3062,15 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
     		goto out_fail;
     	}
     
    +	if (!try_module_get(lport->ops->module)) {
    +		ret = -EUNATCH;
    +		goto out_free_ctrl;
    +	}
    +
     	idx = ida_simple_get(&nvme_fc_ctrl_cnt, 0, 0, GFP_KERNEL);
     	if (idx < 0) {
     		ret = -ENOSPC;
    -		goto out_free_ctrl;
    +		goto out_mod_put;
     	}
     
     	ctrl->ctrl.opts = opts;
    @@ -3215,6 +3223,8 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
     out_free_ida:
     	put_device(ctrl->dev);
     	ida_simple_remove(&nvme_fc_ctrl_cnt, ctrl->cnum);
    +out_mod_put:
    +	module_put(lport->ops->module);
     out_free_ctrl:
     	kfree(ctrl);
     out_fail:
    diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
    index b50b53db3746..1c50af6219f3 100644
    --- a/drivers/nvme/target/fcloop.c
    +++ b/drivers/nvme/target/fcloop.c
    @@ -850,6 +850,7 @@ fcloop_targetport_delete(struct nvmet_fc_target_port *targetport)
     #define FCLOOP_DMABOUND_4G		0xFFFFFFFF
     
     static struct nvme_fc_port_template fctemplate = {
    +	.module			= THIS_MODULE,
     	.localport_delete	= fcloop_localport_delete,
     	.remoteport_delete	= fcloop_remoteport_delete,
     	.create_queue		= fcloop_create_queue,
    diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
    index a227e36cbdc2..8e0f03ef346b 100644
    --- a/drivers/scsi/lpfc/lpfc_nvme.c
    +++ b/drivers/scsi/lpfc/lpfc_nvme.c
    @@ -1976,6 +1976,8 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
     
     /* Declare and initialization an instance of the FC NVME template. */
     static struct nvme_fc_port_template lpfc_nvme_template = {
    +	.module	= THIS_MODULE,
    +
     	/* initiator-based functions */
     	.localport_delete  = lpfc_nvme_localport_delete,
     	.remoteport_delete = lpfc_nvme_remoteport_delete,
    diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
    index 6cc19e060afc..6e4d71302534 100644
    --- a/drivers/scsi/qla2xxx/qla_nvme.c
    +++ b/drivers/scsi/qla2xxx/qla_nvme.c
    @@ -610,6 +610,7 @@ static void qla_nvme_remoteport_delete(struct nvme_fc_remote_port *rport)
     }
     
     static struct nvme_fc_port_template qla_nvme_fc_transport = {
    +	.module	= THIS_MODULE,
     	.localport_delete = qla_nvme_localport_delete,
     	.remoteport_delete = qla_nvme_remoteport_delete,
     	.create_queue   = qla_nvme_alloc_queue,
    diff --git a/include/linux/nvme-fc-driver.h b/include/linux/nvme-fc-driver.h
    index 10f81629b9ce..6d0d70f3219c 100644
    --- a/include/linux/nvme-fc-driver.h
    +++ b/include/linux/nvme-fc-driver.h
    @@ -270,6 +270,8 @@ struct nvme_fc_remote_port {
      *
      * Host/Initiator Transport Entrypoints/Parameters:
      *
    + * @module:  The LLDD module using the interface
    + *
      * @localport_delete:  The LLDD initiates deletion of a localport via
      *       nvme_fc_deregister_localport(). However, the teardown is
      *       asynchronous. This routine is called upon the completion of the
    @@ -383,6 +385,8 @@ struct nvme_fc_remote_port {
      *       Value is Mandatory. Allowed to be zero.
      */
     struct nvme_fc_port_template {
    +	struct module	*module;
    +
     	/* initiator-based functions */
     	void	(*localport_delete)(struct nvme_fc_local_port *);
     	void	(*remoteport_delete)(struct nvme_fc_remote_port *);
    -- 
    2.13.7
    
    
Looks Good. 

Acked-by: Himanshu Madhani <hmadhani@marvell.com>
    _______________________________________________
    Linux-nvme mailing list
    Linux-nvme@lists.infradead.org
    https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.infradead.org_mailman_listinfo_linux-2Dnvme&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lanuADUiz2ynuLzGAQkJPTKDNod8UMh5Vi117R7DgS4&m=GOKmVBaIoqoCSyVOFWX--ntviAVgaEIkfk-jW7xxIW8&s=LqOd21e4tGu2eizf9Erqgh10Hu2205lyAoDppURxzJ8&e= 
    

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] nvme_fc : add module to ops template to allow module references
  2019-11-14 23:15 [PATCH] nvme_fc : add module to ops template to allow module references James Smart
  2019-11-22 17:09 ` Himanshu Madhani
@ 2019-11-26 16:51 ` Christoph Hellwig
  2019-11-26 17:39 ` Keith Busch
  2 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2019-11-26 16:51 UTC (permalink / raw)
  To: James Smart; +Cc: linux-nvme

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] nvme_fc : add module to ops template to allow module references
  2019-11-14 23:15 [PATCH] nvme_fc : add module to ops template to allow module references James Smart
  2019-11-22 17:09 ` Himanshu Madhani
  2019-11-26 16:51 ` Christoph Hellwig
@ 2019-11-26 17:39 ` Keith Busch
  2 siblings, 0 replies; 4+ messages in thread
From: Keith Busch @ 2019-11-26 17:39 UTC (permalink / raw)
  To: James Smart; +Cc: linux-nvme

Applied to nvme/for-5.5

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-14 23:15 [PATCH] nvme_fc : add module to ops template to allow module references James Smart
2019-11-22 17:09 ` Himanshu Madhani
2019-11-26 16:51 ` Christoph Hellwig
2019-11-26 17:39 ` Keith Busch

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git