From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752569AbbJBNJh (ORCPT ); Fri, 2 Oct 2015 09:09:37 -0400 Received: from mx2.suse.de ([195.135.220.15]:38424 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751841AbbJBNJf convert rfc822-to-8bit (ORCPT ); Fri, 2 Oct 2015 09:09:35 -0400 From: Johannes Thumshirn To: Christoph Hellwig Cc: James Bottomley , Hannes Reinecke , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] SCSI: Fix hard lockup in scsi_remove_target() References: <1443774062-15638-1-git-send-email-jthumshirn@suse.de> <20151002124903.GA11987@infradead.org> Date: Fri, 02 Oct 2015 15:09:33 +0200 In-Reply-To: <20151002124903.GA11987@infradead.org> (Christoph Hellwig's message of "Fri, 2 Oct 2015 05:49:03 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Christoph Hellwig writes: >> - list_for_each_entry(starget, &shost->__targets, siblings) { >> + list_for_each_entry_safe(starget, tmp, &shost->__targets, siblings) { >> if (starget->state == STARGET_DEL) >> continue; >> if (starget->dev.parent == dev || &starget->dev == dev) { >> /* assuming new targets arrive at the end */ > > Now that the last variable is gone this comments isn't needed. Yep, you're right. I'll remove it. > >> kref_get(&starget->reap_ref); >> spin_unlock_irqrestore(shost->host_lock, flags); >> - if (last) >> - scsi_target_reap(last); >> - last = starget; >> + >> __scsi_remove_target(starget); >> + list_move_tail(&starget->siblings, &reap_list); >> spin_lock_irqsave(shost->host_lock, flags); >> } > > What makes the list_move save after dropping host_lock? I think this > needs to be changed to not drop the host_lock and change > __scsi_remove_target to expect host_lock held to be safe. Having the list_move() outside of the host_lock was purely by accident. Interestingly the stressing didn't mind it. But yes you're right, __scsi_remove_target() should be made host_lock() save for being called under the host_lock. Regarding the list move, does it look OK for you (i.e. do we still need it after reworking __scsi_remove_target())? IMHO yes, but I only have half a year of experience in this area). > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Johannes Thumshirn Storage jthumshirn@suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850