[PATCH 0/4] iscsi fixes/cleanups

[PATCH 0/4] iscsi fixes/cleanups

[Mike Christie]

The following patches were made over Martin's 5.8 fixes branches.

The first patch:

[PATCH 1/4] iscsi class: make sure the block/recovery work are done

fixes a regression that was just added in 5.8-rc1 with commit:

commit 3ce419662dd4c9cf8db7869c4972ad51ccdf2ee3
Author: Bob Liu <bob.liu@oracle.com>
Date:   Tue May 5 09:19:08 2020 +0800

    scsi: iscsi: Register sysfs for iscsi workqueue

so it should go into 5.8-rc. With that patch we allow multple works
to be run in parallel so it broke some assumtions the iscsi class
was making.

The other patches can just go into next. They are not critical.

[PATCH 1/4] iscsi class: make sure the block/recovery work are done

[Mike Christie]

When max_active=1 we knew the block work was always before the
unblock and the recovery work had either run or was pending behind the
unblock. With the patch to enable max_active=2:

commit 3ce419662dd4c9cf8db7869c4972ad51ccdf2ee3
Author: Bob Liu <bob.liu@oracle.com>
Date:   Tue May 5 09:19:08 2020 +0800

    scsi: iscsi: Register sysfs for iscsi workqueue

for the iscsi_eh_timer_workq we could have the block or recovery work
works on different threads than the unblock. __iscsi_unblock_session
only tries to cancel the recovery work and so the block work could be
running still, or the recovery work could be running (non pending state)
or we could race and the unblock work could run cancel_delayed_work then
the block work could queue the recovery work.

This patches fixes this by making sure the block and recovery works are
done before updating the session state and its devices.

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

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index f4cc08e..bbf2eb7 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -1947,10 +1947,11 @@ static void __iscsi_unblock_session(struct work_struct *work)
 
 	ISCSI_DBG_TRANS_SESSION(session, "Unblocking session\n");
 	/*
-	 * The recovery and unblock work get run from the same workqueue,
-	 * so try to cancel it if it was going to run after this unblock.
+	 * Make sure we do not race with the block or recovery work, so
+	 * they can't overwrite our state update here.
 	 */
-	cancel_delayed_work(&session->recovery_work);
+	flush_work(&session->block_work);
+	cancel_delayed_work_sync(&session->recovery_work);
 	spin_lock_irqsave(&session->lock, flags);
 	session->state = ISCSI_SESSION_LOGGED_IN;
 	spin_unlock_irqrestore(&session->lock, flags);
-- 
1.8.3.1

[PATCH 2/4] iscsi class: delay freeing target_id

[Mike Christie]

If we are doing async removal of the session, we could be doing a
scsi_remove_target from the removal workqueue, and for the offload
case we could be doing a new session addition and scan to the same
host. The add/scan might then end up trying to use the target_id
of the target we are removing.

This patch just has a delay the freeing of the target_id until after
the scsi_remove_target has completed, so we know it's no longer in
use.

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

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index bbf2eb7..2be28d8 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -2037,11 +2037,11 @@ static void __iscsi_unbind_session(struct work_struct *work)
 	spin_unlock_irqrestore(&session->lock, flags);
 	mutex_unlock(&ihost->mutex);
 
+	scsi_remove_target(&session->dev);
+
 	if (session->ida_used)
 		ida_simple_remove(&iscsi_sess_ida, target_id);
 
-	scsi_remove_target(&session->dev);
-
 unbind_session_exit:
 	iscsi_session_event(session, ISCSI_KEVENT_UNBIND_SESSION);
 	ISCSI_DBG_TRANS_SESSION(session, "Completed target removal\n");
-- 
1.8.3.1

[PATCH 3/4] iscsi class: optimize work queue flush use

[Mike Christie]

There is no need for one session to flush the entire
iscsi_eh_timer_workq when removing/unblocking a session. During removal
we need to make sure our works are not running anymore. And
iscsi_unblock_session only needs to make sure it's work is done. The
unblock work function will flush/cancel the works it has conflicts with.

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

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 2be28d8..2b3af43 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -1979,10 +1979,11 @@ void iscsi_unblock_session(struct iscsi_cls_session *session)
 {
 	queue_work(iscsi_eh_timer_workq, &session->unblock_work);
 	/*
-	 * make sure all the events have completed before tell the driver
-	 * it is safe
+	 * Blocking the session can be done from any context so we only
+	 * queue the block work. Make sure the unblock work has completed
+	 * because it flushes/cancels the other works and updates the state.
 	 */
-	flush_workqueue(iscsi_eh_timer_workq);
+	flush_work(&session->unblock_work);
 }
 EXPORT_SYMBOL_GPL(iscsi_unblock_session);
 
@@ -2206,11 +2207,9 @@ void iscsi_remove_session(struct iscsi_cls_session *session)
 	list_del(&session->sess_list);
 	spin_unlock_irqrestore(&sesslock, flags);
 
-	/* make sure there are no blocks/unblocks queued */
-	flush_workqueue(iscsi_eh_timer_workq);
-	/* make sure the timedout callout is not running */
-	if (!cancel_delayed_work(&session->recovery_work))
-		flush_workqueue(iscsi_eh_timer_workq);
+	flush_work(&session->block_work);
+	flush_work(&session->unblock_work);
+	cancel_delayed_work_sync(&session->recovery_work);
 	/*
 	 * If we are blocked let commands flow again. The lld or iscsi
 	 * layer should set up the queuecommand to fail commands.
-- 
1.8.3.1

[PATCH 4/4] iscsi class: remove sessdestroylist

[Mike Christie]

Just delete the sess from the session list instead of adding it to some
list we never use.

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

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 2b3af43..85a68cb 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -1623,7 +1623,6 @@ static DECLARE_TRANSPORT_CLASS(iscsi_connection_class,
 static DEFINE_MUTEX(conn_mutex);
 
 static LIST_HEAD(sesslist);
-static LIST_HEAD(sessdestroylist);
 static DEFINE_SPINLOCK(sesslock);
 static LIST_HEAD(connlist);
 static LIST_HEAD(connlist_err);
@@ -2204,7 +2203,8 @@ void iscsi_remove_session(struct iscsi_cls_session *session)
 	ISCSI_DBG_TRANS_SESSION(session, "Removing session\n");
 
 	spin_lock_irqsave(&sesslock, flags);
-	list_del(&session->sess_list);
+	if (!list_empty(&session->sess_list))
+		list_del(&session->sess_list);
 	spin_unlock_irqrestore(&sesslock, flags);
 
 	flush_work(&session->block_work);
@@ -3679,7 +3679,7 @@ static int iscsi_logout_flashnode_sid(struct iscsi_transport *transport,
 
 			/* Prevent this session from being found again */
 			spin_lock_irqsave(&sesslock, flags);
-			list_move(&session->sess_list, &sessdestroylist);
+			list_del_init(&session->sess_list);
 			spin_unlock_irqrestore(&sesslock, flags);
 
 			queue_work(iscsi_destroy_workq, &session->destroy_work);
-- 
1.8.3.1