diff options
author | Alan Stern <stern@rowland.harvard.edu> | 2005-07-26 10:27:10 -0400 |
---|---|---|
committer | James Bottomley <jejb@titanic.(none)> | 2005-09-18 16:22:06 -0400 |
commit | a64358db1253b35d508a411e80a3ad23b859ec88 (patch) | |
tree | e222f3f17d6962a84d966620485d19f67d7fafa7 | |
parent | b95be99d52ce4f9db9ff0bd5f10e9e2066da6d2e (diff) |
[SCSI] SCSI scanning and removal fixes
This patch (as545) fixes the list traversals in __scsi_remove_target and
scsi_forget_host. In each case the existing code list_for_each_entry_safe
in an _unsafe_ manner, because the list was not protected from outside
modification while the iteration was running.
The new scsi_forget_host routine takes the moderately controversial step
of iterating over devices for removal rather than iterating over targets.
This makes more sense to me because the current scheme treats targets as
second-class citizens, created and removed on demand, rather than as
objects corresponding to actual hardware. (Also I couldn't figure out any
safe way to iterate over the target list, since it's not so easy to tell
when a target has already been removed.)
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
-rw-r--r-- | drivers/scsi/scsi_scan.c | 20 | ||||
-rw-r--r-- | drivers/scsi/scsi_sysfs.c | 9 |
2 files changed, 13 insertions, 16 deletions
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index b86f170fa8ed..fcf9f6cbb142 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c | |||
@@ -1466,23 +1466,17 @@ EXPORT_SYMBOL(scsi_scan_single_target); | |||
1466 | 1466 | ||
1467 | void scsi_forget_host(struct Scsi_Host *shost) | 1467 | void scsi_forget_host(struct Scsi_Host *shost) |
1468 | { | 1468 | { |
1469 | struct scsi_target *starget, *tmp; | 1469 | struct scsi_device *sdev; |
1470 | unsigned long flags; | 1470 | unsigned long flags; |
1471 | 1471 | ||
1472 | /* | 1472 | restart: |
1473 | * Ok, this look a bit strange. We always look for the first device | ||
1474 | * on the list as scsi_remove_device removes them from it - thus we | ||
1475 | * also have to release the lock. | ||
1476 | * We don't need to get another reference to the device before | ||
1477 | * releasing the lock as we already own the reference from | ||
1478 | * scsi_register_device that's release in scsi_remove_device. And | ||
1479 | * after that we don't look at sdev anymore. | ||
1480 | */ | ||
1481 | spin_lock_irqsave(shost->host_lock, flags); | 1473 | spin_lock_irqsave(shost->host_lock, flags); |
1482 | list_for_each_entry_safe(starget, tmp, &shost->__targets, siblings) { | 1474 | list_for_each_entry(sdev, &shost->__devices, siblings) { |
1475 | if (sdev->sdev_state == SDEV_DEL) | ||
1476 | continue; | ||
1483 | spin_unlock_irqrestore(shost->host_lock, flags); | 1477 | spin_unlock_irqrestore(shost->host_lock, flags); |
1484 | scsi_remove_target(&starget->dev); | 1478 | __scsi_remove_device(sdev); |
1485 | spin_lock_irqsave(shost->host_lock, flags); | 1479 | goto restart; |
1486 | } | 1480 | } |
1487 | spin_unlock_irqrestore(shost->host_lock, flags); | 1481 | spin_unlock_irqrestore(shost->host_lock, flags); |
1488 | } | 1482 | } |
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 83f87c41b18d..1e47b7eddef4 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c | |||
@@ -719,17 +719,20 @@ void __scsi_remove_target(struct scsi_target *starget) | |||
719 | { | 719 | { |
720 | struct Scsi_Host *shost = dev_to_shost(starget->dev.parent); | 720 | struct Scsi_Host *shost = dev_to_shost(starget->dev.parent); |
721 | unsigned long flags; | 721 | unsigned long flags; |
722 | struct scsi_device *sdev, *tmp; | 722 | struct scsi_device *sdev; |
723 | 723 | ||
724 | spin_lock_irqsave(shost->host_lock, flags); | 724 | spin_lock_irqsave(shost->host_lock, flags); |
725 | starget->reap_ref++; | 725 | starget->reap_ref++; |
726 | list_for_each_entry_safe(sdev, tmp, &shost->__devices, siblings) { | 726 | restart: |
727 | list_for_each_entry(sdev, &shost->__devices, siblings) { | ||
727 | if (sdev->channel != starget->channel || | 728 | if (sdev->channel != starget->channel || |
728 | sdev->id != starget->id) | 729 | sdev->id != starget->id || |
730 | sdev->sdev_state == SDEV_DEL) | ||
729 | continue; | 731 | continue; |
730 | spin_unlock_irqrestore(shost->host_lock, flags); | 732 | spin_unlock_irqrestore(shost->host_lock, flags); |
731 | scsi_remove_device(sdev); | 733 | scsi_remove_device(sdev); |
732 | spin_lock_irqsave(shost->host_lock, flags); | 734 | spin_lock_irqsave(shost->host_lock, flags); |
735 | goto restart; | ||
733 | } | 736 | } |
734 | spin_unlock_irqrestore(shost->host_lock, flags); | 737 | spin_unlock_irqrestore(shost->host_lock, flags); |
735 | scsi_target_reap(starget); | 738 | scsi_target_reap(starget); |