| Message ID | CADJHv_uHL0=ebUio3j5ysnaU9gDus+HQi4=pQznTy0B=6+NWAg@mail.gmail.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
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 <jencce.kernel@gmail.com> > --- > 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). > > @@ -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.
On Wed, Apr 13, 2016 at 4:19 PM, Johannes Thumshirn <jthumshirn@suse.de> 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 <jencce.kernel@gmail.com> >> --- >> 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 > -- 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
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); } @@ -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;
How about this? drivers/scsi/scsi_scan: mark STARGET_REMOVE state before destroy Signed-off-by: Xiong Zhou <jencce.kernel@gmail.com> --- drivers/scsi/scsi_scan.c | 3 +++ 1 file changed, 3 insertions(+) }