Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753264AbcDODYH (ORCPT ); Thu, 14 Apr 2016 23:24:07 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:33350 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752348AbcDODYF convert rfc822-to-8bit (ORCPT ); Thu, 14 Apr 2016 23:24:05 -0400 MIME-Version: 1.0 In-Reply-To: <1550969.mvAXU2aY35@c203> References: <95ae7ee32dca23bb7f3ab432046fb7016b341049.1459848136.git.jthumshirn@suse.de> <1550969.mvAXU2aY35@c203> Date: Fri, 15 Apr 2016 11:24:02 +0800 Message-ID: Subject: Re: [PATCH v4 1/2] scsi: Add intermediate STARGET_REMOVE state to scsi_target_state From: Xiong Zhou To: Johannes Thumshirn Cc: "Martin K. Petersen" , "James E.J. Bottomley" , "Ewan D. Milne" , Hannes Reinecke , Christoph Hellwig , linux-scsi@vger.kernel.org, "linux-kernel@vger.kernel.org" , stable@vger.kernel.org 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 Content-Length: 2202 Lines: 67 On Wed, Apr 13, 2016 at 4:19 PM, Johannes Thumshirn wrote: > Hi Xiong > Sorry for the late reply > > On Dienstag, 12. April 2016 21:01:53 CEST Xiong Zhou wrote: >> How about this? >> >> drivers/scsi/scsi_scan: mark STARGET_REMOVE state before destroy >> >> Signed-off-by: Xiong Zhou >> --- >> drivers/scsi/scsi_scan.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c >> index 27df7e7..21092e5 100644 >> --- a/drivers/scsi/scsi_scan.c >> +++ b/drivers/scsi/scsi_scan.c >> @@ -395,6 +395,8 @@ static void scsi_target_reap_ref_release(struct kref *kref) >> transport_remove_device(&starget->dev); >> device_del(&starget->dev); >> } >> + >> + starget->state = STARGET_REMOVE; >> scsi_target_destroy(starget); >> } > > The only scsi_target_reap_ref_release() caller is scsi_target_reap_ref_put(), > which in turn is only called by scsi_target_reap(). scsi_target_reap()'s only > callers are scsi_remove_target()/__scsi_remove_target(), which set the > STARGET_REMOVE state and > > __scsi_add_device() > scsi_get_host_dev() > __scsi_scan_target() > > Which I'm currently investiganting (in parallel to reproducing the bug). Thanks ! -- Xiong > >> >> @@ -465,6 +467,7 @@ static struct scsi_target >> *scsi_alloc_target(struct device *parent, >> dev_printk(KERN_ERR, dev, "target allocation failed, error %d\n", error); >> /* don't want scsi_target_reap to do the final >> * put because it will be under the host lock */ >> + starget->state = STARGET_REMOVE; >> scsi_target_destroy(starget); >> return NULL; >> } > > Here starget->state is STARGET_CREATED and the assertion is already aware of > this state transitoin. IOTW this /shouldn't/ be needed. > > > -- > 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 >