Skip to content

Commit

Permalink
scsi: restart list search after unlock in scsi_remove_target
Browse files Browse the repository at this point in the history
When dropping a lock while iterating a list we must restart the search
as other threads could have manipulated the list under us.  Without this
we can get stuck in an endless loop.  This bug was introduced by

commit bc3f02a
Author: Dan Williams <[email protected]>
Date:   Tue Aug 28 22:12:10 2012 -0700

    [SCSI] scsi_remove_target: fix softlockup regression on hot remove

Which was itself trying to fix a reported soft lockup issue

http://thread.gmane.org/gmane.linux.kernel/1348679

However, we believe even with this revert of the original patch, the soft
lockup problem has been fixed by

commit f2495e2
Author: James Bottomley <[email protected]>
Date:   Tue Jan 21 07:01:41 2014 -0800

    [SCSI] dual scan thread bug fix

Thanks go to Dan Williams <[email protected]> for tracking all this
prior history down.

Reported-by: Johannes Thumshirn <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
Tested-by: Johannes Thumshirn <[email protected]>
Reviewed-by: Johannes Thumshirn <[email protected]>
Fixes: bc3f02a
Cc: [email protected]
Signed-off-by: James Bottomley <[email protected]>
  • Loading branch information
Christoph Hellwig authored and James Bottomley committed Nov 5, 2015
1 parent a82544c commit 4099819
Showing 1 changed file with 4 additions and 12 deletions.
16 changes: 4 additions & 12 deletions drivers/scsi/scsi_sysfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1158,31 +1158,23 @@ static void __scsi_remove_target(struct scsi_target *starget)
void scsi_remove_target(struct device *dev)
{
struct Scsi_Host *shost = dev_to_shost(dev->parent);
struct scsi_target *starget, *last = NULL;
struct scsi_target *starget;
unsigned long flags;

/* remove targets being careful to lookup next entry before
* deleting the last
*/
restart:
spin_lock_irqsave(shost->host_lock, flags);
list_for_each_entry(starget, &shost->__targets, siblings) {
if (starget->state == STARGET_DEL)
continue;
if (starget->dev.parent == dev || &starget->dev == dev) {
/* assuming new targets arrive at the end */
kref_get(&starget->reap_ref);
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);
scsi_target_reap(starget);
goto restart;
}
}
spin_unlock_irqrestore(shost->host_lock, flags);

if (last)
scsi_target_reap(last);
}
EXPORT_SYMBOL(scsi_remove_target);

Expand Down

0 comments on commit 4099819

Please sign in to comment.