Commit bc3f02a7 authored by Dan Williams's avatar Dan Williams Committed by James Bottomley

[SCSI] scsi_remove_target: fix softlockup regression on hot remove

John reports:
 BUG: soft lockup - CPU#2 stuck for 23s! [kworker/u:8:2202]
 [..]
 Call Trace:
  [<ffffffff8141782a>] scsi_remove_target+0xda/0x1f0
  [<ffffffff81421de5>] sas_rphy_remove+0x55/0x60
  [<ffffffff81421e01>] sas_rphy_delete+0x11/0x20
  [<ffffffff81421e35>] sas_port_delete+0x25/0x160
  [<ffffffff814549a3>] mptsas_del_end_device+0x183/0x270

...introduced by commit 3b661a92 "[SCSI] fix hot unplug vs async scan race".

Don't restart lookup of more stargets in the multi-target case, just
arrange to traverse the list once, on the assumption that new targets
are always added at the end.  There is no guarantee that the target will
change state in scsi_target_reap() so we can end up spinning if we
restart.

Cc: <stable@vger.kernel.org>
Acked-by: default avatarJack Wang <jack_wang@usish.com>
LKML-Reference: <CAEhu1-6wq1YsNiscGMwP4ud0Q+MrViRzv=kcWCQSBNc8c68N5Q@mail.gmail.com>
Reported-by: default avatarJohn Drescher <drescherjm@gmail.com>
Tested-by: default avatarJohn Drescher <drescherjm@gmail.com>
Signed-off-by: default avatarDan Williams <djbw@fb.com>
Signed-off-by: default avatarJames Bottomley <JBottomley@Parallels.com>
parent 225c5696
...@@ -1031,33 +1031,31 @@ static void __scsi_remove_target(struct scsi_target *starget) ...@@ -1031,33 +1031,31 @@ static void __scsi_remove_target(struct scsi_target *starget)
void scsi_remove_target(struct device *dev) void scsi_remove_target(struct device *dev)
{ {
struct Scsi_Host *shost = dev_to_shost(dev->parent); struct Scsi_Host *shost = dev_to_shost(dev->parent);
struct scsi_target *starget, *found; struct scsi_target *starget, *last = NULL;
unsigned long flags; unsigned long flags;
restart: /* remove targets being careful to lookup next entry before
found = NULL; * deleting the last
*/
spin_lock_irqsave(shost->host_lock, flags); spin_lock_irqsave(shost->host_lock, flags);
list_for_each_entry(starget, &shost->__targets, siblings) { list_for_each_entry(starget, &shost->__targets, siblings) {
if (starget->state == STARGET_DEL) if (starget->state == STARGET_DEL)
continue; continue;
if (starget->dev.parent == dev || &starget->dev == dev) { if (starget->dev.parent == dev || &starget->dev == dev) {
found = starget; /* assuming new targets arrive at the end */
found->reap_ref++; starget->reap_ref++;
break; spin_unlock_irqrestore(shost->host_lock, flags);
if (last)
scsi_target_reap(last);
last = starget;
__scsi_remove_target(starget);
spin_lock_irqsave(shost->host_lock, flags);
} }
} }
spin_unlock_irqrestore(shost->host_lock, flags); spin_unlock_irqrestore(shost->host_lock, flags);
if (found) { if (last)
__scsi_remove_target(found); scsi_target_reap(last);
scsi_target_reap(found);
/* in the case where @dev has multiple starget children,
* continue removing.
*
* FIXME: does such a case exist?
*/
goto restart;
}
} }
EXPORT_SYMBOL(scsi_remove_target); EXPORT_SYMBOL(scsi_remove_target);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment