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@xxxxxxxxxxxx>
---
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
