target-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Christie <michael.christie@oracle.com>
To: mlombard@redhat.com, martin.petersen@oracle.com,
	mgurtovoy@nvidia.com, sagi@grimberg.me, d.bogdanov@yadro.com,
	linux-scsi@vger.kernel.org, target-devel@vger.kernel.org
Cc: Mike Christie <michael.christie@oracle.com>
Subject: [PATCH v3 11/14] scsi: target: iscsit: Cleanup isert commands at conn closure
Date: Sun, 29 Jan 2023 17:44:38 -0600	[thread overview]
Message-ID: <20230129234441.116310-12-michael.christie@oracle.com> (raw)
In-Reply-To: <20230129234441.116310-1-michael.christie@oracle.com>

Currently, isert does target_wait_for_cmds before iscsit calls
iscsit_release_commands_from_conn because we can race where LIO core
calls into isert when it wants to cleanup the connection. The wait
prevents isert from freeing the connection while trying to post responses
but it can result in a hang during connection closure if there are se_cmds
on the iscsit response queue, because when isert calls
target_wait_for_cmds the tx thread is stopped or we are running the wait
from it.

For example this is hit when a command times out on the initiator,  the
initiator sends an ABORT, then the connection is closed. When the command
completes it will be placed on the response queue if TAS is set, and the
ABORT response will be placed on the response queue. So at the very
least we will hang waiting on the last put on the ABORT's se_cmd which
will never happen.

This patch adds support to iscsit so it can now handle isert and iscsit
running commands during connection closure so we can have a common place
for the code.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/iscsi/iscsi_target.c      | 33 ++++++++++++++++++------
 drivers/target/iscsi/iscsi_target_util.c |  8 +++++-
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 2a011afa6dff..a57527beb340 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -4231,6 +4231,15 @@ static void iscsit_release_commands_from_conn(struct iscsit_conn *conn)
 				 */
 				list_move_tail(&cmd->i_conn_node,
 					       &conn->conn_cmd_list);
+			} else if (conn->sess->sess_ops->RDMAExtensions &&
+				   (se_cmd->transport_state & CMD_T_COMPLETE) &&
+				   !iscsit_cmd_failed(cmd)) {
+				/*
+				 * isert is still handling these cmds so wait in
+				 * target_wait_for_cmds.
+				 */
+				list_move_tail(&cmd->i_conn_node,
+					       &conn->conn_cmd_list);
 			} else {
 				se_cmd->transport_state |= CMD_T_FABRIC_STOP;
 			}
@@ -4243,19 +4252,27 @@ static void iscsit_release_commands_from_conn(struct iscsit_conn *conn)
 		list_del_init(&cmd->i_conn_node);
 
 		iscsit_increment_maxcmdsn(cmd, sess);
+		/*
+		 * Free cmds that:
+		 * 1. we have not got acks for.
+		 * 2. are (or will be when the backend completes them) stuck
+		 * on the response/immediate queue (failed cmds, TMRs, iscsi
+		 * reqs).
+		 * 3. completed ok on the backend, but hit the CMD_T_FABRIC_STOP
+		 * or CMD_T_STOP checks.
+		 */
 		iscsit_free_cmd(cmd, true);
-
 	}
 
 	/*
-	 * Wait on commands that were cleaned up via the aborted_task path.
-	 * LLDs that implement iscsit_wait_conn will already have waited for
-	 * commands.
+	 * We need to wait:
+	 * 1. for commands that are being cleaned up via the aborted_task path.
+	 * 2. for isert we need to wait for iscsit_queue_status calls
+	 * that posted a response after the ib_drain_qp call returned but
+	 * have not yet called isert_send_done.
 	 */
-	if (!conn->conn_transport->iscsit_wait_conn) {
-		target_stop_cmd_counter(conn->cmd_cnt);
-		target_wait_for_cmds(conn->cmd_cnt);
-	}
+	target_stop_cmd_counter(conn->cmd_cnt);
+	target_wait_for_cmds(conn->cmd_cnt);
 }
 
 static void iscsit_stop_timers_for_cmds(
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index 26dc8ed3045b..b0d7d6c73a1c 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -753,7 +753,13 @@ void iscsit_free_cmd(struct iscsit_cmd *cmd, bool shutdown)
 	if (se_cmd) {
 		rc = transport_generic_free_cmd(se_cmd, shutdown);
 		if (!rc && shutdown && se_cmd->se_sess) {
-			__iscsit_free_cmd(cmd, shutdown);
+			struct iscsit_conn *conn = cmd->conn;
+			/*
+			 * The command wasn't aborted via ABORT_TASK but didn't
+			 * reach the driver so allow it to cleanup resources
+			 * now.
+			 */
+			conn->conn_transport->iscsit_aborted_task(conn, cmd);
 			target_put_sess_cmd(se_cmd);
 		}
 	} else {
-- 
2.25.1


  parent reply	other threads:[~2023-01-29 23:45 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-29 23:44 [PATCH v3 00/14] target: TMF and recovery fixes Mike Christie
2023-01-29 23:44 ` [PATCH v3 01/14] scsi: target: Move sess cmd counter to new struct Mike Christie
2023-03-07 10:53   ` Maurizio Lombardi
2023-01-29 23:44 ` [PATCH v3 02/14] scsi: target: Move cmd counter allocation Mike Christie
2023-01-29 23:44 ` [PATCH v3 03/14] scsi: target: Pass in cmd counter to use during cmd setup Mike Christie
2023-01-29 23:44 ` [PATCH v3 04/14] scsi: target: iscsit/isert: Alloc per conn cmd counter Mike Christie
2023-01-29 23:44 ` [PATCH v3 05/14] scsi: target: iscsit: stop/wait on cmds during conn close Mike Christie
2023-03-01 11:57   ` Maurizio Lombardi
2023-01-29 23:44 ` [PATCH v3 06/14] scsi: target: iscsit: Fix TAS handling during conn cleanup Mike Christie
2023-02-14 14:12   ` Maurizio Lombardi
2023-01-29 23:44 ` [PATCH v3 07/14] scsi: target: Fix multiple LUN_RESET handling Mike Christie
2023-02-11  8:59   ` Dmitry Bogdanov
2023-03-02  9:43     ` Dmitry Bogdanov
2023-03-02 17:29       ` Mike Christie
2023-03-03  9:26         ` Dmitry Bogdanov
2023-03-03 16:52           ` Mike Christie
2023-01-29 23:44 ` [PATCH v3 08/14] scsi: target: Drop t_state_lock use in compare_and_write_post Mike Christie
2023-01-29 23:44 ` [PATCH v3 09/14] scsi: target: Treat CMD_T_FABRIC_STOP like CMD_T_STOP Mike Christie
2023-01-29 23:44 ` [PATCH v3 10/14] scsi: target: iscsit: Add helper to check when cmd has failed Mike Christie
2023-02-27 15:12   ` Maurizio Lombardi
2023-01-29 23:44 ` Mike Christie [this message]
2023-01-29 23:44 ` [PATCH v3 12/14] IB/isert: Fix hang in target_wait_for_cmds Mike Christie
2023-01-29 23:44 ` [PATCH v3 13/14] IB/isert: Fix use after free during conn cleanup Mike Christie
2023-02-27 15:10   ` Maurizio Lombardi
2023-01-29 23:44 ` [PATCH v3 14/14] scsi: target: iscsit: free cmds before session free Mike Christie
2023-02-22 12:26   ` Maurizio Lombardi

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=20230129234441.116310-12-michael.christie@oracle.com \
    --to=michael.christie@oracle.com \
    --cc=d.bogdanov@yadro.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mgurtovoy@nvidia.com \
    --cc=mlombard@redhat.com \
    --cc=sagi@grimberg.me \
    --cc=target-devel@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).