diff mbox series

[1/1] qla2xxx: move IO flush to the front of NVME rport unregistration

Message ID 20190616150553.28399-1-hmadhani@marvell.com (mailing list archive)
State Accepted
Headers show
Series [1/1] qla2xxx: move IO flush to the front of NVME rport unregistration | expand

Commit Message

Himanshu Madhani June 16, 2019, 3:05 p.m. UTC 
From: Quinn Tran <qutran@marvell.com>

On session deletion, current qla code would unregister an NVMe
session before flushing IOs. This patch would move the unregistration
of NVMe session after IO flush. This way FC-NVMe layer would not
have to wait for stuck IOs. In addition, qla2xxx would stop accepting
new IOs during session deletion.

Signed-off-by: Quinn Tran <qutran@marvell.com>
Signed-off-by: Himanshu Madhani <hmadhani@marvell.com>
---
Hi Martin, 

we found one more issue where NVMe IO would be stuck while doing a
port toggle or lip_reset test case. This patch fixes issue by flushing
IO before unregistering NVMe session. 

This patch depends on series sent on June 14.
(https://marc.info/?l=linux-scsi&m=156055022726719&w=2)
 
Please apply this patch to 5.3/scsi-queue branch at your earliest
convenience.

Thanks,
Himanshu
---
 drivers/scsi/qla2xxx/qla_def.h    |  1 -
 drivers/scsi/qla2xxx/qla_gbl.h    |  2 ++
 drivers/scsi/qla2xxx/qla_nvme.c   | 14 ++------------
 drivers/scsi/qla2xxx/qla_target.c | 16 ++++++++--------
 4 files changed, 12 insertions(+), 21 deletions(-)

Comments

Bart Van Assche June 16, 2019, 4:57 p.m. UTC | #1 
On 6/16/19 8:05 AM, Himanshu Madhani wrote:
> +	INIT_WORK(&sess->free_work, qlt_free_session_done);
> +	schedule_work(&sess->free_work);

Since you are touching this code and since there are multiple 
schedule_work() and flush_work() calls in the qla2xxx driver code for 
this work item: please move the INIT_WORK() call to the session 
initialization code. Calling INIT_WORK() while another thread can call 
schedule_work() or flush_work() on the same work item is racy.

Thanks,

Bart.
Martin K. Petersen June 27, 2019, 4:12 a.m. UTC | #2 
Himanshu,

> On session deletion, current qla code would unregister an NVMe session
> before flushing IOs. This patch would move the unregistration of NVMe
> session after IO flush. This way FC-NVMe layer would not have to wait
> for stuck IOs. In addition, qla2xxx would stop accepting new IOs
> during session deletion.

Applied to 5.3/scsi-queue, thanks!
diff mbox series

Patch

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 85a27ee5d647..aad2a84eac29 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -2338,7 +2338,6 @@  typedef struct fc_port {
 	unsigned int id_changed:1;
 	unsigned int scan_needed:1;
 
-	struct work_struct nvme_del_work;
 	struct completion nvme_del_done;
 	uint32_t nvme_prli_service_param;
 #define NVME_PRLI_SP_CONF       BIT_7
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index bbe69ab5cf3f..f9669fdf7798 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -908,4 +908,6 @@  void qlt_clr_qp_table(struct scsi_qla_host *vha);
 void qlt_set_mode(struct scsi_qla_host *);
 int qla2x00_set_data_rate(scsi_qla_host_t *vha, uint16_t mode);
 
+/* nvme.c */
+void qla_nvme_unregister_remote_port(struct fc_port *fcport);
 #endif /* _QLA_GBL_H */
diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index b56dcab9d265..baea57424691 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -12,8 +12,6 @@ 
 
 static struct nvme_fc_port_template qla_nvme_fc_transport;
 
-static void qla_nvme_unregister_remote_port(struct work_struct *);
-
 static inline
 int qla_is_active_nvme_fcport(struct fc_port *fcport)
 {
@@ -50,7 +48,6 @@  int qla_nvme_register_remote(struct scsi_qla_host *vha, struct fc_port *fcport)
 		(fcport->nvme_flag & NVME_FLAG_REGISTERED))
 		return 0;
 
-	INIT_WORK(&fcport->nvme_del_work, qla_nvme_unregister_remote_port);
 	fcport->nvme_flag &= ~NVME_FLAG_RESETTING;
 
 	memset(&req, 0, sizeof(struct nvme_fc_port_info));
@@ -628,16 +625,11 @@  static void qla_nvme_remoteport_delete(struct nvme_fc_remote_port *rport)
 	fcport = qla_rport->fcport;
 	fcport->nvme_remote_port = NULL;
 	fcport->nvme_flag &= ~NVME_FLAG_REGISTERED;
-
-	complete(&fcport->nvme_del_done);
-
-	INIT_WORK(&fcport->free_work, qlt_free_session_done);
-	schedule_work(&fcport->free_work);
-
 	fcport->nvme_flag &= ~NVME_FLAG_DELETING;
 	ql_log(ql_log_info, fcport->vha, 0x2110,
 	    "remoteport_delete of %p %8phN completed.\n",
 	    fcport, fcport->port_name);
+	complete(&fcport->nvme_del_done);
 }
 
 static struct nvme_fc_port_template qla_nvme_fc_transport = {
@@ -659,10 +651,8 @@  static struct nvme_fc_port_template qla_nvme_fc_transport = {
 	.fcprqst_priv_sz = sizeof(struct nvme_private),
 };
 
-static void qla_nvme_unregister_remote_port(struct work_struct *work)
+void qla_nvme_unregister_remote_port(struct fc_port *fcport)
 {
-	struct fc_port *fcport = container_of(work, struct fc_port,
-	    nvme_del_work);
 	int ret;
 
 	if (!IS_ENABLED(CONFIG_NVME_FC))
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 3eeae72793bc..b9ee0fa37c27 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1013,6 +1013,12 @@  void qlt_free_session_done(struct work_struct *work)
 				else
 					logout_started = true;
 			}
+		} /* if sess->logout_on_delete */
+
+		if (sess->nvme_flag & NVME_FLAG_REGISTERED &&
+		    !(sess->nvme_flag & NVME_FLAG_DELETING)) {
+			sess->nvme_flag |= NVME_FLAG_DELETING;
+			qla_nvme_unregister_remote_port(sess);
 		}
 	}
 
@@ -1164,14 +1170,8 @@  void qlt_unreg_sess(struct fc_port *sess)
 	sess->last_rscn_gen = sess->rscn_gen;
 	sess->last_login_gen = sess->login_gen;
 
-	if (sess->nvme_flag & NVME_FLAG_REGISTERED &&
-	    !(sess->nvme_flag & NVME_FLAG_DELETING)) {
-		sess->nvme_flag |= NVME_FLAG_DELETING;
-		schedule_work(&sess->nvme_del_work);
-	} else {
-		INIT_WORK(&sess->free_work, qlt_free_session_done);
-		schedule_work(&sess->free_work);
-	}
+	INIT_WORK(&sess->free_work, qlt_free_session_done);
+	schedule_work(&sess->free_work);
 }
 EXPORT_SYMBOL(qlt_unreg_sess);