Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753483Ab2H1QMh (ORCPT ); Tue, 28 Aug 2012 12:12:37 -0400 Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:58914 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753022Ab2H1QMg (ORCPT ); Tue, 28 Aug 2012 12:12:36 -0400 Message-ID: <1346170350.12384.9.camel@localhost.localdomain> Subject: Re: Possible mptsas regression post 3.5.0 From: Dan Williams To: John Drescher CC: LKML , , Date: Tue, 28 Aug 2012 09:12:30 -0700 In-Reply-To: References: <1346132253.12384.6.camel@localhost.localdomain> Content-Type: multipart/mixed; boundary="=-JLjCog8p08IZVNonnz3z" X-Mailer: Evolution 3.4.3 (3.4.3-2.fc17) MIME-Version: 1.0 X-Originating-IP: [192.168.18.252] X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.7.7855,1.0.431,0.0.0000 definitions=2012-08-28_06:2012-08-28,2012-08-28,1970-01-01 signatures=0 X-Proofpoint-Spam-Reason: safe Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2972 Lines: 97 --=-JLjCog8p08IZVNonnz3z Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit On Tue, 2012-08-28 at 10:03 -0400, John Drescher wrote: > > I wonder if we are preventing scsi_device_dev_release_usercontext() from > > making forward progress? > > > > ...the attached patch should confirm this or give more info otherwise. > > > > [ 174.758218] scsi_remove_target[0]: reap 0:0 state: 2 reap: 1 dev_del: 1 Thanks! Does the attached patch fix the issue for you? -- Dan --=-JLjCog8p08IZVNonnz3z Content-Disposition: attachment; filename="test-scsi-remove-target-fix.patch" Content-Type: text/x-patch; name="test-scsi-remove-target-fix.patch"; charset="UTF-8" Content-Transfer-Encoding: 7bit scsi_remove_target: fix softlockup condition From: Dan Williams Don't restart lookup of 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. --- drivers/scsi/scsi_sysfs.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 093d4f6..ce5224c 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -1031,33 +1031,31 @@ 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, *found; + struct scsi_target *starget, *last = NULL; unsigned long flags; - restart: - found = NULL; + /* remove targets being careful to lookup next entry before + * deleting the last + */ 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) { - found = starget; - found->reap_ref++; - break; + /* assuming new targets arrive at the end */ + 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); } } spin_unlock_irqrestore(shost->host_lock, flags); - if (found) { - __scsi_remove_target(found); - scsi_target_reap(found); - /* in the case where @dev has multiple starget children, - * continue removing. - * - * FIXME: does such a case exist? - */ - goto restart; - } + if (last) + scsi_target_reap(last); } EXPORT_SYMBOL(scsi_remove_target); --=-JLjCog8p08IZVNonnz3z-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/