Re: [PATCH v2] scsi_sysfs: fix hang when removing scsi device

[Bart Van Assche <Bart.VanAssche@sandisk.com>]

On Mon, 2017-03-13 at 12:23 -0700, James Bottomley wrote:
> On Mon, 2017-03-13 at 18:49 +0000, Bart Van Assche wrote:
> > diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> > index 7bfbcfa7af40..b3bb49d06943 100644
> > --- a/drivers/scsi/scsi.c
> > +++ b/drivers/scsi/scsi.c
> > @@ -602,7 +602,7 @@ EXPORT_SYMBOL(scsi_device_get);
> >   */
> >  void scsi_device_put(struct scsi_device *sdev)
> >  {
> > -       module_put(sdev->host->hostt->module);
> > +       module_put(sdev->hostt->module);
> >         put_device(&sdev->sdev_gendev);
> >  }
> >  EXPORT_SYMBOL(scsi_device_put);
> > diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> > index 6f7128f49c30..7134487abbb1 100644
> > --- a/drivers/scsi/scsi_scan.c
> > +++ b/drivers/scsi/scsi_scan.c
> > @@ -227,6 +227,7 @@ static struct scsi_device *scsi_alloc_sdev(struct
> > scsi_target *starget,
> >         sdev->model = scsi_null_device_strs;
> >         sdev->rev = scsi_null_device_strs;
> >         sdev->host = shost;
> > +       sdev->hostt = shost->hostt;
> >         sdev->queue_ramp_up_period = SCSI_DEFAULT_RAMP_UP_PERIOD;
> >         sdev->id = starget->id;
> >         sdev->lun = lun;
> > diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> > index 6f22b39f1b0c..cda620ed5922 100644
> > --- a/include/scsi/scsi_device.h
> > +++ b/include/scsi/scsi_device.h
> > @@ -82,6 +82,7 @@ struct scsi_event {
> >  
> >  struct scsi_device {
> >         struct Scsi_Host *host;
> > +       struct scsi_host_template *hostt;
> >         struct request_queue *request_queue;
> >  
> 
> The apparent assumption behind this patch is that sdev->host can be
> freed but the sdev will still exist?  That shouldn't be correct: the
> rule for struct devices is that the child always holds the parent and
> the host is parented (albeit not necessarily directly) to the sdev, so
> it looks like something has gone wrong if the host had been freed
> before the sdev.

Hello James,

scsi_remove_host() decreases the sdev reference count but does not wait
until the sdev release work has finished. This is why the SCSI host can
already have disappeared before the last scsi_device_put() call occurs.

Bart.