diff mbox series

[v4,05/17] scsi: iscsi: wait on cmds before freeing conn

Message ID 20210424220603.123703-6-michael.christie@oracle.com
State New
Headers show
Series libiscsi and qedi TMF fixes | expand

Commit Message

Mike Christie April 24, 2021, 10:05 p.m. UTC
If we haven't done an unbind target call, we can race during conn
destruction where iscsi_conn_teardown wakes up the eh/abort thread and its
still accessing a task while iscsi_conn_teardown is freeing the conn. This
patch has us wait for all threads to drop their refs to outstanding tasks
during conn destruction.

There is also an issue where we could be accessing the conn directly via
fields like conn->ehwait in the eh callbacks. The next patch will fix
those.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/scsi/libiscsi.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Lee Duncan April 25, 2021, 11:55 p.m. UTC | #1
On 4/24/21 3:05 PM, Mike Christie wrote:
> If we haven't done an unbind target call, we can race during conn

> destruction where iscsi_conn_teardown wakes up the eh/abort thread and its

> still accessing a task while iscsi_conn_teardown is freeing the conn. This

> patch has us wait for all threads to drop their refs to outstanding tasks

> during conn destruction.

> 

> There is also an issue where we could be accessing the conn directly via

> fields like conn->ehwait in the eh callbacks. The next patch will fix

> those.

> 

> Signed-off-by: Mike Christie <michael.christie@oracle.com>

> ---

>  drivers/scsi/libiscsi.c | 28 ++++++++++++++++++++++++++++

>  1 file changed, 28 insertions(+)

> 

> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c

> index 0f2b5f8718ca..e340a67c6764 100644

> --- a/drivers/scsi/libiscsi.c

> +++ b/drivers/scsi/libiscsi.c

> @@ -3124,6 +3124,24 @@ iscsi_conn_setup(struct iscsi_cls_session *cls_session, int dd_size,

>  }

>  EXPORT_SYMBOL_GPL(iscsi_conn_setup);

>  

> +static bool iscsi_session_has_tasks(struct iscsi_session *session)

> +{

> +	struct iscsi_task *task;

> +	int i;

> +

> +	spin_lock_bh(&session->back_lock);

> +	for (i = 0; i < session->cmds_max; i++) {

> +		task = session->cmds[i];

> +

> +		if (task->sc) {

> +			spin_unlock_bh(&session->back_lock);

> +			return true;

> +		}

> +	}

> +	spin_unlock_bh(&session->back_lock);

> +	return false;

> +}

> +

>  /**

>   * iscsi_conn_teardown - teardown iscsi connection

>   * @cls_conn: iscsi class connection

> @@ -3148,7 +3166,17 @@ void iscsi_conn_teardown(struct iscsi_cls_conn *cls_conn)

>  		session->state = ISCSI_STATE_TERMINATE;

>  		wake_up(&conn->ehwait);

>  	}

> +

>  	spin_unlock_bh(&session->frwd_lock);

> +	mutex_unlock(&session->eh_mutex);

> +	/*

> +	 * If the caller didn't do a target unbind we could be exiting a

> +	 * scsi-ml entry point that had a task ref. Wait on them here.

> +	 */

> +	while (iscsi_session_has_tasks(session))

> +		msleep(50);

> +

> +	mutex_lock(&session->eh_mutex);

>  

>  	/* flush queued up work because we free the connection below */

>  	iscsi_suspend_tx(conn);

> 


Reviewed-by: Lee Duncan <lduncan@suse.com>
diff mbox series

Patch

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 0f2b5f8718ca..e340a67c6764 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -3124,6 +3124,24 @@  iscsi_conn_setup(struct iscsi_cls_session *cls_session, int dd_size,
 }
 EXPORT_SYMBOL_GPL(iscsi_conn_setup);
 
+static bool iscsi_session_has_tasks(struct iscsi_session *session)
+{
+	struct iscsi_task *task;
+	int i;
+
+	spin_lock_bh(&session->back_lock);
+	for (i = 0; i < session->cmds_max; i++) {
+		task = session->cmds[i];
+
+		if (task->sc) {
+			spin_unlock_bh(&session->back_lock);
+			return true;
+		}
+	}
+	spin_unlock_bh(&session->back_lock);
+	return false;
+}
+
 /**
  * iscsi_conn_teardown - teardown iscsi connection
  * @cls_conn: iscsi class connection
@@ -3148,7 +3166,17 @@  void iscsi_conn_teardown(struct iscsi_cls_conn *cls_conn)
 		session->state = ISCSI_STATE_TERMINATE;
 		wake_up(&conn->ehwait);
 	}
+
 	spin_unlock_bh(&session->frwd_lock);
+	mutex_unlock(&session->eh_mutex);
+	/*
+	 * If the caller didn't do a target unbind we could be exiting a
+	 * scsi-ml entry point that had a task ref. Wait on them here.
+	 */
+	while (iscsi_session_has_tasks(session))
+		msleep(50);
+
+	mutex_lock(&session->eh_mutex);
 
 	/* flush queued up work because we free the connection below */
 	iscsi_suspend_tx(conn);