[PATCH 1/1] scsi: scsi_forget_host() shuffle

[Douglas Gilbert <dgilbert@interlog.com>]

This patch leaves me a bit uneasy but it does cure the crash
that occurs in this function. xarray iterators are pretty safe
_unless_ something deletes the parent node holding the
collection. The problem seems to be these nested loops do not
_explicitly_ remove the starget object. That is done magically
at the sdev level on the removal of the last sdev in a starget.
And that is half an iteration too soon! Hence the shuffle which
isn't a great solution. The magical starget removal is wrong IMO
and this will burn us elsewhere, I suspect.

Thes patch is on top of Hannes Reinecke's "[PATCHv4 0/5] scsi:
use xarray for devices and targets" patchset.

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
 drivers/scsi/scsi_scan.c | 47 +++++++++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 0a344653487d..e378f03d0297 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1858,25 +1858,52 @@ void scsi_scan_host(struct Scsi_Host *shost)
 }
 EXPORT_SYMBOL(scsi_scan_host);
 
+static void scsi_forget_host_inner(struct Scsi_Host *shost,
+				   struct scsi_target *starg,
+				   unsigned long *flagsp)
+{
+	struct scsi_device *sdev;
+	struct scsi_device *prev_sdev = NULL;
+	unsigned long lun_id;
+
+	xa_for_each(&starg->__devices, lun_id, sdev) {
+		if (sdev->sdev_state == SDEV_DEL)
+			continue;
+		if (!prev_sdev) {
+			prev_sdev = sdev;
+			continue;
+		}
+		spin_unlock_irqrestore(shost->host_lock, *flagsp);
+		__scsi_remove_device(prev_sdev);
+		spin_lock_irqsave(shost->host_lock, *flagsp);
+		prev_sdev = sdev;
+	}
+	if (prev_sdev) {
+		spin_unlock_irqrestore(shost->host_lock, *flagsp);
+		__scsi_remove_device(prev_sdev);
+		spin_lock_irqsave(shost->host_lock, *flagsp);
+	}
+}
+
+/* N.B. Keeping iteration one step ahead of destruction point */
 void scsi_forget_host(struct Scsi_Host *shost)
 {
 	struct scsi_target *starget;
-	struct scsi_device *sdev;
+	struct scsi_target *prev_starget = NULL;
 	unsigned long flags;
-	unsigned long tid = 0;
+	unsigned long tid;
 
 	spin_lock_irqsave(shost->host_lock, flags);
 	xa_for_each(&shost->__targets, tid, starget) {
-		unsigned long lun_id = 0;
-
-		xa_for_each(&starget->__devices, lun_id, sdev) {
-			if (sdev->sdev_state == SDEV_DEL)
-				continue;
-			spin_unlock_irqrestore(shost->host_lock, flags);
-			__scsi_remove_device(sdev);
-			spin_lock_irqsave(shost->host_lock, flags);
+		if (!prev_starget) {
+			prev_starget = starget;
+			continue;
 		}
+		scsi_forget_host_inner(shost, prev_starget, &flags);
+		prev_starget = starget;
 	}
+	if (prev_starget)
+		scsi_forget_host_inner(shost, prev_starget, &flags);
 	spin_unlock_irqrestore(shost->host_lock, flags);
 }
 
-- 
2.25.1