All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Christie <michael.christie@oracle.com>
To: lduncan@suse.com, martin.petersen@oracle.com,
	mrangankar@marvell.com, svernekar@marvell.com,
	linux-scsi@vger.kernel.org, jejb@linux.ibm.com
Cc: Mike Christie <michael.christie@oracle.com>
Subject: [PATCH 03/13] scsi: iscsi: prevent cmds from queueing to driver during ep_disconnect
Date: Tue, 13 Apr 2021 18:06:38 -0500	[thread overview]
Message-ID: <20210413230648.5593-4-michael.christie@oracle.com> (raw)
In-Reply-To: <20210413230648.5593-1-michael.christie@oracle.com>

When we added iser and all thes offload drivers I goofed and didn't add
a unbind_conn nl cmd to undo the bind_conn. So during ep_disconnect we
have been doing iscsi_suspend_tx/queue to stop new IO but depending on
the driver we can still get IO from:

1. __iscsi_conn_send_pdu for TMFs and nops if we haven't called
iscsi_conn_failure before ep_disconnect.
2. Userspace did a ep_disconnect during shutdown before we saw a
logout command and userspace didn't do an session unbind event.

This patch fixes the issue by adding a helper which drivers implementing
ep_disconnect can use to make sure new IO is not queued to them after
calling it and until we do a new conn_bind.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |  2 +
 drivers/scsi/be2iscsi/be_iscsi.c         |  2 +-
 drivers/scsi/bnx2i/bnx2i_iscsi.c         |  2 +-
 drivers/scsi/cxgbi/libcxgbi.c            |  3 +-
 drivers/scsi/libiscsi.c                  | 56 +++++++++++++++++++++---
 drivers/scsi/qedi/qedi_iscsi.c           |  2 +-
 drivers/scsi/qla4xxx/ql4_def.h           |  1 +
 drivers/scsi/qla4xxx/ql4_os.c            |  2 +
 include/scsi/libiscsi.h                  |  1 +
 9 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 8fcaa1136f2c..3089502116a5 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -899,6 +899,8 @@ iscsi_iser_ep_disconnect(struct iscsi_endpoint *ep)
 
 	iser_info("ep %p iser conn %p\n", ep, iser_conn);
 
+	iscsi_ep_prep_disconnect(iser_conn->iscsi_conn);
+
 	mutex_lock(&iser_conn->state_mutex);
 	iser_conn_terminate(iser_conn);
 
diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index a13c203ef7a9..a3b5f7f6ccc8 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -1314,7 +1314,7 @@ void beiscsi_ep_disconnect(struct iscsi_endpoint *ep)
 
 	if (beiscsi_ep->conn) {
 		beiscsi_conn = beiscsi_ep->conn;
-		iscsi_suspend_queue(beiscsi_conn->conn);
+		iscsi_ep_prep_disconnect(beiscsi_conn->conn);
 	}
 
 	if (!beiscsi_hba_is_online(phba)) {
diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index 1e6d8f62ea3c..5cbca9657c35 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -2129,7 +2129,7 @@ static void bnx2i_ep_disconnect(struct iscsi_endpoint *ep)
 	if (bnx2i_ep->conn) {
 		bnx2i_conn = bnx2i_ep->conn;
 		conn = bnx2i_conn->cls_conn->dd_data;
-		iscsi_suspend_queue(conn);
+		iscsi_ep_prep_disconnect(conn);
 	}
 	hba = bnx2i_ep->hba;
 
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index f078b3c4e083..dd7b41a092f1 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -2968,7 +2968,8 @@ void cxgbi_ep_disconnect(struct iscsi_endpoint *ep)
 		ep, cep, cconn, csk, csk->state, csk->flags);
 
 	if (cconn && cconn->iconn) {
-		iscsi_suspend_tx(cconn->iconn);
+		iscsi_ep_prep_disconnect(cconn->iconn);
+
 		write_lock_bh(&csk->callback_lock);
 		cep->csk->user_data = NULL;
 		cconn->cep = NULL;
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 3ff440d37a36..3a37628e4024 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1387,22 +1387,28 @@ void iscsi_session_failure(struct iscsi_session *session,
 }
 EXPORT_SYMBOL_GPL(iscsi_session_failure);
 
-void iscsi_conn_failure(struct iscsi_conn *conn, enum iscsi_err err)
+static void iscsi_set_conn_failed(struct iscsi_conn *conn)
 {
 	struct iscsi_session *session = conn->session;
 
-	spin_lock_bh(&session->frwd_lock);
-	if (session->state == ISCSI_STATE_FAILED) {
-		spin_unlock_bh(&session->frwd_lock);
+	if (session->state == ISCSI_STATE_FAILED)
 		return;
-	}
 
 	if (conn->stop_stage == 0)
 		session->state = ISCSI_STATE_FAILED;
-	spin_unlock_bh(&session->frwd_lock);
 
 	set_bit(ISCSI_SUSPEND_BIT, &conn->suspend_tx);
 	set_bit(ISCSI_SUSPEND_BIT, &conn->suspend_rx);
+}
+
+void iscsi_conn_failure(struct iscsi_conn *conn, enum iscsi_err err)
+{
+	struct iscsi_session *session = conn->session;
+
+	spin_lock_bh(&session->frwd_lock);
+	iscsi_set_conn_failed(conn);
+	spin_unlock_bh(&session->frwd_lock);
+
 	iscsi_conn_error_event(conn->cls_conn, err);
 }
 EXPORT_SYMBOL_GPL(iscsi_conn_failure);
@@ -2220,6 +2226,44 @@ static void iscsi_check_transport_timeouts(struct timer_list *t)
 	spin_unlock(&session->frwd_lock);
 }
 
+/*
+ * iscsi_ep_prep_disconnect - prepare the conn for a ep disconnect
+ * @conn: iscsi conn ep is bound to.
+ *
+ * This must be called by drivers implementing the ep_disconnect callout.
+ */
+void iscsi_ep_prep_disconnect(struct iscsi_conn *conn)
+{
+	struct iscsi_session *session;
+
+	/* Check if bound for the driver */
+	if (!conn)
+		return;
+
+	session = conn->session;
+	/*
+	 * Wait for iscsi_eh calls to exit. We don't wait for the tmf to
+	 * complete or timeout. The caller just wants to know what's running
+	 * is everything that needs to be cleaned up, and no cmds will be
+	 * queued.
+	 */
+	mutex_lock(&session->eh_mutex);
+	spin_lock_bh(&session->frwd_lock);
+
+	set_bit(ISCSI_SUSPEND_BIT, &conn->suspend_tx);
+	/*
+	 * if logout timed out before userspace could even send a PDU the
+	 * state might still be in ISCSI_STATE_LOGGED_IN and allowing new cmds
+	 * and TMFs.
+	 */
+	if (session->state == ISCSI_STATE_LOGGED_IN)
+		iscsi_set_conn_failed(conn);
+
+	spin_unlock_bh(&session->frwd_lock);
+	mutex_unlock(&session->eh_mutex);
+}
+EXPORT_SYMBOL_GPL(iscsi_ep_prep_disconnect);
+
 static void iscsi_prep_abort_task_pdu(struct iscsi_task *task,
 				      struct iscsi_tm *hdr)
 {
diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
index 08c05403cd72..8ed1852627e7 100644
--- a/drivers/scsi/qedi/qedi_iscsi.c
+++ b/drivers/scsi/qedi/qedi_iscsi.c
@@ -1008,7 +1008,7 @@ static void qedi_ep_disconnect(struct iscsi_endpoint *ep)
 	if (qedi_ep->conn) {
 		qedi_conn = qedi_ep->conn;
 		conn = qedi_conn->cls_conn->dd_data;
-		iscsi_suspend_queue(conn);
+		iscsi_ep_prep_disconnect(conn);
 		abrt_conn = qedi_conn->abrt_conn;
 
 		while (count--)	{
diff --git a/drivers/scsi/qla4xxx/ql4_def.h b/drivers/scsi/qla4xxx/ql4_def.h
index 031569c496e5..4ca7f4228b4b 100644
--- a/drivers/scsi/qla4xxx/ql4_def.h
+++ b/drivers/scsi/qla4xxx/ql4_def.h
@@ -838,6 +838,7 @@ struct ql4_task_data {
 struct qla_endpoint {
 	struct Scsi_Host *host;
 	struct sockaddr_storage dst_addr;
+	struct iscsi_conn *conn;
 };
 
 struct qla_conn {
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 7bd9a4a04ad5..5977fe403086 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -1772,6 +1772,7 @@ static void qla4xxx_ep_disconnect(struct iscsi_endpoint *ep)
 	ha = to_qla_host(qla_ep->host);
 	DEBUG2(ql4_printk(KERN_INFO, ha, "%s: host: %ld\n", __func__,
 			  ha->host_no));
+	iscsi_ep_prep_disconnect(qla_ep->conn);
 	iscsi_destroy_endpoint(ep);
 }
 
@@ -3234,6 +3235,7 @@ static int qla4xxx_conn_bind(struct iscsi_cls_session *cls_session,
 	conn = cls_conn->dd_data;
 	qla_conn = conn->dd_data;
 	qla_conn->qla_ep = ep->dd_data;
+	qla_conn->qla_ep->conn = conn;
 	return 0;
 }
 
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 8c6d358a8abc..f409f521681d 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -441,6 +441,7 @@ extern int iscsi_conn_get_addr_param(struct sockaddr_storage *addr,
 extern void iscsi_suspend_tx(struct iscsi_conn *conn);
 extern void iscsi_suspend_queue(struct iscsi_conn *conn);
 extern void iscsi_conn_queue_work(struct iscsi_conn *conn);
+extern void iscsi_ep_prep_disconnect(struct iscsi_conn *conn);
 
 #define iscsi_conn_printk(prefix, _c, fmt, a...) \
 	iscsi_cls_conn_printk(prefix, ((struct iscsi_conn *)_c)->cls_conn, \
-- 
2.25.1


  parent reply	other threads:[~2021-04-13 23:07 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-13 23:06 [PATCH 00/13] scsi: libicsi and qedi tmf fixes Mike Christie
2021-04-13 23:06 ` [PATCH 01/13] scsi: iscsi: add task completion helper Mike Christie
2021-04-13 23:06 ` [PATCH 02/13] scsi: iscsi: fix lun/target reset cmd cleanup handling Mike Christie
2021-04-14  4:17   ` kernel test robot
2021-04-14  4:17     ` kernel test robot
2021-04-14  4:17   ` [PATCH] scsi: iscsi: fix boolreturn.cocci warnings kernel test robot
2021-04-14  4:17     ` kernel test robot
2021-04-13 23:06 ` Mike Christie [this message]
2021-04-13 23:06 ` [PATCH 04/13] scsi: qedi: fix null ref during abort handling Mike Christie
2021-04-13 23:06 ` [PATCH 05/13] scsi: qedi: fix abort vs cmd re-use race Mike Christie
2021-04-14 12:19   ` kernel test robot
2021-04-14 12:19     ` kernel test robot
2021-04-14 14:58     ` Mike Christie
2021-04-14 14:58       ` Mike Christie
2021-04-15 14:21   ` kernel test robot
2021-04-15 14:21     ` kernel test robot
2021-04-13 23:06 ` [PATCH 06/13] scsi: qedi: fix use after free during abort cleanup Mike Christie
2021-04-13 23:06 ` [PATCH 07/13] scsi: qedi: fix tmf tid allocation Mike Christie
2021-04-13 23:06 ` [PATCH 08/13] scsi: qedi: use GFP_NOIO for tmf allocation Mike Christie
2021-04-13 23:06 ` [PATCH 09/13] scsi: qedi: fix session block/unblock abuse during tmf handling Mike Christie
2021-04-13 23:06 ` [PATCH 10/13] scsi: qedi: fix session block/unblock abuse during cleanup Mike Christie
2021-04-13 23:06 ` [PATCH 11/13] scsi: qedi: pass send_iscsi_tmf task to abort Mike Christie
2021-04-13 23:06 ` [PATCH 12/13] scsi: qedi: make sure tmf works are done before disconnect Mike Christie
2021-04-13 23:06 ` [PATCH 13/13] scsi: qedi: always wake up if cmd_cleanup_req is set Mike Christie

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=20210413230648.5593-4-michael.christie@oracle.com \
    --to=michael.christie@oracle.com \
    --cc=jejb@linux.ibm.com \
    --cc=lduncan@suse.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mrangankar@marvell.com \
    --cc=svernekar@marvell.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.